From 12d7dd6d8b578c3049cbd461882406472af34059 Mon Sep 17 00:00:00 2001 From: Cedric Guinoiseau Date: Thu, 6 Jul 2023 09:33:05 +0200 Subject: [PATCH 1/5] fix: issue 2744, call replaceCurrentItem in dispatch thread --- ios/Video/RCTVideo.swift | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ios/Video/RCTVideo.swift b/ios/Video/RCTVideo.swift index 264b1de3..82a604f2 100644 --- a/ios/Video/RCTVideo.swift +++ b/ios/Video/RCTVideo.swift @@ -310,7 +310,10 @@ class RCTVideo: UIView, RCTVideoPlayerViewControllerDelegate, RCTPlayerObserverH } self._player = self._player ?? AVPlayer() - self._player?.replaceCurrentItem(with: playerItem) + // https://github.com/react-native-video/react-native-video/issues/2744#issuecomment-1237459473 + DispatchQueue.global(qos: .default).async { [weak self] in + self?._player?.replaceCurrentItem(with: playerItem) + } self._playerObserver.player = self._player self.applyModifiers() self._player?.actionAtItemEnd = .none From d526479fe0983cbd1aa1a4a6ab1dd1ba82e85bb7 Mon Sep 17 00:00:00 2001 From: Cedric Guinoiseau Date: Thu, 6 Jul 2023 09:37:02 +0200 Subject: [PATCH 2/5] fix: issue 3040, prevent crash --- ios/Video/RCTVideo.swift | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ios/Video/RCTVideo.swift b/ios/Video/RCTVideo.swift index 82a604f2..fbfd635b 100644 --- a/ios/Video/RCTVideo.swift +++ b/ios/Video/RCTVideo.swift @@ -665,6 +665,11 @@ class RCTVideo: UIView, RCTVideoPlayerViewControllerDelegate, RCTPlayerObserverH self.onVideoFullscreenPlayerWillPresent?(["target": reactTag as Any]) if let playerViewController = _playerViewController { + if(_controls) { + // prevents crash https://github.com/react-native-video/react-native-video/issues/3040 + self._playerViewController?.removeFromParent() + } + viewController.present(playerViewController, animated:true, completion:{ self._playerViewController?.showsPlaybackControls = self._controls self._fullscreenPlayerPresented = fullscreen From 50b3650e2fe5a2624559f50bc81f4481be2475ee Mon Sep 17 00:00:00 2001 From: Cedric Guinoiseau Date: Thu, 6 Jul 2023 09:52:33 +0200 Subject: [PATCH 3/5] fix: memory leak due to [weak self] and delegate not being weak --- ios/Video/Features/RCTIMAAdsManager.swift | 17 ++++++++++------- ios/Video/Features/RCTPlayerObserver.swift | 19 +++++++++++++------ ios/Video/RCTVideo.swift | 10 ++++++---- ios/Video/RCTVideoPlayerViewController.swift | 10 ++++------ 4 files changed, 33 insertions(+), 23 deletions(-) diff --git a/ios/Video/Features/RCTIMAAdsManager.swift b/ios/Video/Features/RCTIMAAdsManager.swift index 44aff4be..d069af72 100644 --- a/ios/Video/Features/RCTIMAAdsManager.swift +++ b/ios/Video/Features/RCTIMAAdsManager.swift @@ -4,7 +4,7 @@ import GoogleInteractiveMediaAds class RCTIMAAdsManager: NSObject, IMAAdsLoaderDelegate, IMAAdsManagerDelegate { - private var _video:RCTVideo + private weak var _video: RCTVideo? /* Entry point for the SDK. Used to make ad requests. */ private var adsLoader: IMAAdsLoader! @@ -23,6 +23,7 @@ class RCTIMAAdsManager: NSObject, IMAAdsLoaderDelegate, IMAAdsManagerDelegate { } func requestAds() { + guard let _video = _video else {return} // Create ad display container for ad rendering. let adDisplayContainer = IMAAdDisplayContainer(adContainer: _video, viewController: _video.reactViewController()) @@ -54,6 +55,7 @@ class RCTIMAAdsManager: NSObject, IMAAdsLoaderDelegate, IMAAdsManagerDelegate { // MARK: - IMAAdsLoaderDelegate func adsLoader(_ loader: IMAAdsLoader, adsLoadedWith adsLoadedData: IMAAdsLoadedData) { + guard let _video = _video else {return} // Grab the instance of the IMAAdsManager and set yourself as the delegate. adsManager = adsLoadedData.adsManager adsManager?.delegate = self @@ -71,12 +73,13 @@ class RCTIMAAdsManager: NSObject, IMAAdsLoaderDelegate, IMAAdsManagerDelegate { print("Error loading ads: " + adErrorData.adError.message!) } - _video.setPaused(false) + _video?.setPaused(false) } // MARK: - IMAAdsManagerDelegate func adsManager(_ adsManager: IMAAdsManager, didReceive event: IMAAdEvent) { + guard let _video = _video else {return} // Mute ad if the main player is muted if (_video.isMuted()) { adsManager.volume = 0; @@ -102,19 +105,19 @@ class RCTIMAAdsManager: NSObject, IMAAdsLoaderDelegate, IMAAdsManagerDelegate { } // Fall back to playing content - _video.setPaused(false) + _video?.setPaused(false) } func adsManagerDidRequestContentPause(_ adsManager: IMAAdsManager) { // Pause the content for the SDK to play ads. - _video.setPaused(true) - _video.setAdPlaying(true) + _video?.setPaused(true) + _video?.setAdPlaying(true) } func adsManagerDidRequestContentResume(_ adsManager: IMAAdsManager) { // Resume the content since the SDK is done playing ads (at least for now). - _video.setAdPlaying(false) - _video.setPaused(false) + _video?.setAdPlaying(false) + _video?.setPaused(false) } // MARK: - Helpers diff --git a/ios/Video/Features/RCTPlayerObserver.swift b/ios/Video/Features/RCTPlayerObserver.swift index 68fc56dd..9e798dc3 100644 --- a/ios/Video/Features/RCTPlayerObserver.swift +++ b/ios/Video/Features/RCTPlayerObserver.swift @@ -25,7 +25,7 @@ protocol RCTPlayerObserverHandler: RCTPlayerObserverHandlerObjc { class RCTPlayerObserver: NSObject { - var _handlers: RCTPlayerObserverHandler! + weak var _handlers: RCTPlayerObserverHandler? var player:AVPlayer? { willSet { @@ -84,11 +84,13 @@ class RCTPlayerObserver: NSObject { private var _playerViewControllerOverlayFrameObserver:NSKeyValueObservation? deinit { - NotificationCenter.default.removeObserver(_handlers) + if let _handlers = _handlers { + NotificationCenter.default.removeObserver(_handlers) + } } func addPlayerObservers() { - guard let player = player else { + guard let player = player, let _handlers = _handlers else { return } @@ -102,7 +104,7 @@ class RCTPlayerObserver: NSObject { } func addPlayerItemObservers() { - guard let playerItem = playerItem else { return } + guard let playerItem = playerItem, let _handlers = _handlers else { return } _playerItemStatusObserver = playerItem.observe(\.status, options: [.new, .old], changeHandler: _handlers.handlePlayerItemStatusChange) _playerPlaybackBufferEmptyObserver = playerItem.observe(\.isPlaybackBufferEmpty, options: [.new, .old], changeHandler: _handlers.handlePlaybackBufferKeyEmpty) @@ -118,7 +120,7 @@ class RCTPlayerObserver: NSObject { } func addPlayerViewControllerObservers() { - guard let playerViewController = playerViewController else { return } + guard let playerViewController = playerViewController, let _handlers = _handlers else { return } _playerViewControllerReadyForDisplayObserver = playerViewController.observe(\.isReadyForDisplay, options: [.new], changeHandler: _handlers.handleReadyForDisplay) @@ -131,6 +133,7 @@ class RCTPlayerObserver: NSObject { } func addPlayerLayerObserver() { + guard let _handlers = _handlers else {return} _playerLayerReadyForDisplayObserver = playerLayer?.observe(\.isReadyForDisplay, options: [.new], changeHandler: _handlers.handleReadyForDisplay) } @@ -139,6 +142,7 @@ class RCTPlayerObserver: NSObject { } func addPlayerTimeObserver() { + guard let _handlers = _handlers else {return} removePlayerTimeObserver() let progressUpdateIntervalMS:Float64 = _progressUpdateInterval / 1000 // @see endScrubbing in AVPlayerDemoPlaybackViewController.m @@ -174,6 +178,7 @@ class RCTPlayerObserver: NSObject { } func attachPlayerEventListeners() { + guard let _handlers = _handlers else {return} NotificationCenter.default.removeObserver(_handlers, name:NSNotification.Name.AVPlayerItemDidPlayToEndTime, @@ -202,6 +207,8 @@ class RCTPlayerObserver: NSObject { func clearPlayer() { player = nil playerItem = nil - NotificationCenter.default.removeObserver(_handlers) + if let _handlers = _handlers { + NotificationCenter.default.removeObserver(_handlers) + } } } diff --git a/ios/Video/RCTVideo.swift b/ios/Video/RCTVideo.swift index fbfd635b..4afeb851 100644 --- a/ios/Video/RCTVideo.swift +++ b/ios/Video/RCTVideo.swift @@ -246,7 +246,8 @@ class RCTVideo: UIView, RCTVideoPlayerViewControllerDelegate, RCTPlayerObserverH // MARK: - Player and source @objc func setSrc(_ source:NSDictionary!) { - DispatchQueue.global(qos: .default).async { + DispatchQueue.global(qos: .default).async { [weak self] in + guard let self = self else {return} self._source = VideoSource(source) if (self._source?.uri == nil || self._source?.uri == "") { self._player?.replaceCurrentItem(with: nil) @@ -670,7 +671,8 @@ class RCTVideo: UIView, RCTVideoPlayerViewControllerDelegate, RCTPlayerObserverH self._playerViewController?.removeFromParent() } - viewController.present(playerViewController, animated:true, completion:{ + viewController.present(playerViewController, animated:true, completion:{ [weak self] in + guard let self = self else {return} self._playerViewController?.showsPlaybackControls = self._controls self._fullscreenPlayerPresented = fullscreen self._playerViewController?.autorotate = self._fullscreenAutorotate @@ -682,8 +684,8 @@ class RCTVideo: UIView, RCTVideoPlayerViewControllerDelegate, RCTPlayerObserverH } } else if !fullscreen && _fullscreenPlayerPresented, let _playerViewController = _playerViewController { self.videoPlayerViewControllerWillDismiss(playerViewController: _playerViewController) - _presentingViewController?.dismiss(animated: true, completion:{ - self.videoPlayerViewControllerDidDismiss(playerViewController: _playerViewController) + _presentingViewController?.dismiss(animated: true, completion:{[weak self] in + self?.videoPlayerViewControllerDidDismiss(playerViewController: _playerViewController) }) } } diff --git a/ios/Video/RCTVideoPlayerViewController.swift b/ios/Video/RCTVideoPlayerViewController.swift index e398e62f..1abbc384 100644 --- a/ios/Video/RCTVideoPlayerViewController.swift +++ b/ios/Video/RCTVideoPlayerViewController.swift @@ -2,7 +2,7 @@ import AVKit class RCTVideoPlayerViewController: AVPlayerViewController { - var rctDelegate:RCTVideoPlayerViewControllerDelegate! + weak var rctDelegate: RCTVideoPlayerViewControllerDelegate? // Optional paramters var preferredOrientation:String? @@ -19,11 +19,9 @@ class RCTVideoPlayerViewController: AVPlayerViewController { override func viewDidDisappear(_ animated: Bool) { super.viewDidDisappear(animated) - - if rctDelegate != nil { - rctDelegate.videoPlayerViewControllerWillDismiss(playerViewController: self) - rctDelegate.videoPlayerViewControllerDidDismiss(playerViewController: self) - } + + rctDelegate?.videoPlayerViewControllerWillDismiss(playerViewController: self) + rctDelegate?.videoPlayerViewControllerDidDismiss(playerViewController: self) } #if !TARGET_OS_TV From 238daf8720ab6c1a4a53004b3dbd93c0be47f3ba Mon Sep 17 00:00:00 2001 From: Cedric Guinoiseau Date: Thu, 6 Jul 2023 11:16:49 +0200 Subject: [PATCH 4/5] fix: issue 3085, onFullscreen call backs are never fired --- ios/Video/RCTVideo.swift | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/ios/Video/RCTVideo.swift b/ios/Video/RCTVideo.swift index 4afeb851..f33f5859 100644 --- a/ios/Video/RCTVideo.swift +++ b/ios/Video/RCTVideo.swift @@ -59,6 +59,7 @@ class RCTVideo: UIView, RCTVideoPlayerViewControllerDelegate, RCTPlayerObserverH private var _fullscreenAutorotate:Bool = true private var _fullscreenOrientation:String! = "all" private var _fullscreenPlayerPresented:Bool = false + private var _fullscreenUncontrolPlayerPresented:Bool = false // to call events switching full screen mode from player controls private var _filterName:String! private var _filterEnabled:Bool = false private var _presentingViewController:UIViewController? @@ -1113,12 +1114,27 @@ class RCTVideo: UIView, RCTVideoPlayerViewControllerDelegate, RCTPlayerObserverH let oldRect = change.oldValue let newRect = change.newValue if !oldRect!.equalTo(newRect!) { + // https://github.com/react-native-video/react-native-video/issues/3085#issuecomment-1557293391 if newRect!.equalTo(UIScreen.main.bounds) { RCTLog("in fullscreen") + if (!_fullscreenUncontrolPlayerPresented) { + _fullscreenUncontrolPlayerPresented = true; - self.reactViewController().view.frame = UIScreen.main.bounds - self.reactViewController().view.setNeedsLayout() - } else {NSLog("not fullscreen")} + self.onVideoFullscreenPlayerWillPresent?(["target": self.reactTag as Any]) + self.onVideoFullscreenPlayerDidPresent?(["target": self.reactTag as Any]) + } + } else { + NSLog("not fullscreen") + if (_fullscreenUncontrolPlayerPresented) { + _fullscreenUncontrolPlayerPresented = false; + + self.onVideoFullscreenPlayerWillDismiss?(["target": self.reactTag as Any]) + self.onVideoFullscreenPlayerDidDismiss?(["target": self.reactTag as Any]) + } + } + + self.reactViewController().view.frame = UIScreen.main.bounds + self.reactViewController().view.setNeedsLayout() } } From 71c3c5c9403a3057f3791689f1482189b39f5eca Mon Sep 17 00:00:00 2001 From: Cedric Guinoiseau Date: Sun, 9 Jul 2023 20:44:58 +0200 Subject: [PATCH 5/5] Revert "fix: issue 2744, call replaceCurrentItem in dispatch thread" This reverts commit 12d7dd6d8b578c3049cbd461882406472af34059. --- ios/Video/RCTVideo.swift | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/ios/Video/RCTVideo.swift b/ios/Video/RCTVideo.swift index f33f5859..baeac2bf 100644 --- a/ios/Video/RCTVideo.swift +++ b/ios/Video/RCTVideo.swift @@ -312,10 +312,7 @@ class RCTVideo: UIView, RCTVideoPlayerViewControllerDelegate, RCTPlayerObserverH } self._player = self._player ?? AVPlayer() - // https://github.com/react-native-video/react-native-video/issues/2744#issuecomment-1237459473 - DispatchQueue.global(qos: .default).async { [weak self] in - self?._player?.replaceCurrentItem(with: playerItem) - } + self._player?.replaceCurrentItem(with: playerItem) self._playerObserver.player = self._player self.applyModifiers() self._player?.actionAtItemEnd = .none