From 6902dd31e634f3091e894a38e3288995416d8bb6 Mon Sep 17 00:00:00 2001 From: fanyu Date: Tue, 2 Nov 2021 17:13:36 +0800 Subject: [PATCH 1/5] Add log --- .../Chat/GalleryVideoItemViewController.swift | 33 +++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/Mixin/UserInterface/Controllers/Chat/GalleryVideoItemViewController.swift b/Mixin/UserInterface/Controllers/Chat/GalleryVideoItemViewController.swift index 69ad76074a..eecdf520a1 100644 --- a/Mixin/UserInterface/Controllers/Chat/GalleryVideoItemViewController.swift +++ b/Mixin/UserInterface/Controllers/Chat/GalleryVideoItemViewController.swift @@ -219,6 +219,7 @@ final class GalleryVideoItemViewController: GalleryItemViewController, GalleryAn controlView.slider.value = controlView.slider.minimumValue controlView.playedTimeLabel.text = mediaDurationFormatter.string(from: 0) guard let item = item else { + Logger.general.error(category: "GalleryVideoItemViewController", message: "Load item failed, it's nil") return } videoRatio = standardizedRatio(of: item.size) @@ -439,6 +440,24 @@ extension GalleryVideoItemViewController: AVPictureInPictureControllerDelegate { } +extension GalleryVideoItemViewController { + + override func observeValue(forKeyPath keyPath: String?, of object: Any?, change: [NSKeyValueChangeKey : Any]?, context: UnsafeMutableRawPointer?) { + guard keyPath == #keyPath(AVPlayerItem.status) else { + super.observeValue(forKeyPath: keyPath, of: object, change: change, context: context) + return + } + if let statusNumber = change?[.newKey] as? NSNumber, let status = AVPlayerItem.Status(rawValue: statusNumber.intValue) { + if case .failed = status, let error = player.currentItem?.error { + Logger.general.error(category: "GalleryVideoItemViewController", message: "Player item status is failed: \(error)") + } + } else { + Logger.general.error(category: "GalleryVideoItemViewController", message: "Player item status is not yet ready") + } + } + +} + extension GalleryVideoItemViewController { @objc func playAction(_ sender: Any) { @@ -451,6 +470,7 @@ extension GalleryVideoItemViewController { } } guard let item = item else { + Logger.general.error(category: "GalleryVideoItemViewController", message: "Play item failed, it's nil") return } playerDidFailedToPlay = false @@ -462,10 +482,12 @@ extension GalleryVideoItemViewController { try session.setCategory(.playback, mode: .default, options: .defaultToSpeaker) } mute = false - } catch AudioSession.Error.insufficientPriority { + } catch AudioSession.Error.insufficientPriority(let priority) { mute = true + Logger.general.error(category: "GalleryVideoItemViewController", message: "AudioSession activate with insufficient Priority :\(priority)") } catch { mute = false + Logger.general.error(category: "GalleryVideoItemViewController", message: "AudioSession activate error: \(error)") } player.isMuted = mute addTimeObservers() @@ -682,6 +704,10 @@ extension GalleryVideoItemViewController { itemPresentationSizeObserver = item.observe(\.presentationSize) { [weak self] (item, _) in self?.updateVideoViewSize(with: item) } + item.addObserver(self, + forKeyPath: #keyPath(AVPlayerItem.status), + options: [.old, .new], + context: nil) let center = NotificationCenter.default center.addObserver(self, @@ -706,10 +732,12 @@ extension GalleryVideoItemViewController { try session.setCategory(.playback, mode: .default, options: .defaultToSpeaker) } mute = false - } catch AudioSession.Error.insufficientPriority { + } catch AudioSession.Error.insufficientPriority(let priority) { mute = true + Logger.general.error(category: "GalleryVideoItemViewController", message: "AudioSession activate with insufficient Priority :\(priority)") } catch { mute = false + Logger.general.error(category: "GalleryVideoItemViewController", message: "AudioSession activate error: \(error)") } player.isMuted = mute player.play() @@ -837,6 +865,7 @@ extension GalleryVideoItemViewController { itemStatusObserver?.invalidate() itemPresentationSizeObserver?.invalidate() timeControlObserver?.invalidate() + player.currentItem?.removeObserver(self, forKeyPath: #keyPath(AVPlayerItem.status)) NotificationCenter.default.removeObserver(self) } From 9cf1e1133b7ccd6ecb66dab69c5d67b23b364d5c Mon Sep 17 00:00:00 2001 From: fanyu Date: Wed, 3 Nov 2021 10:23:34 +0800 Subject: [PATCH 2/5] Add player status observe --- .../Chat/GalleryVideoItemViewController.swift | 38 ++++++++++++++++--- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/Mixin/UserInterface/Controllers/Chat/GalleryVideoItemViewController.swift b/Mixin/UserInterface/Controllers/Chat/GalleryVideoItemViewController.swift index eecdf520a1..bdc7dfb284 100644 --- a/Mixin/UserInterface/Controllers/Chat/GalleryVideoItemViewController.swift +++ b/Mixin/UserInterface/Controllers/Chat/GalleryVideoItemViewController.swift @@ -443,16 +443,22 @@ extension GalleryVideoItemViewController: AVPictureInPictureControllerDelegate { extension GalleryVideoItemViewController { override func observeValue(forKeyPath keyPath: String?, of object: Any?, change: [NSKeyValueChangeKey : Any]?, context: UnsafeMutableRawPointer?) { - guard keyPath == #keyPath(AVPlayerItem.status) else { + guard keyPath == #keyPath(AVPlayerItem.status) || keyPath == #keyPath(AVPlayer.status) else { super.observeValue(forKeyPath: keyPath, of: object, change: change, context: context) return } - if let statusNumber = change?[.newKey] as? NSNumber, let status = AVPlayerItem.Status(rawValue: statusNumber.intValue) { - if case .failed = status, let error = player.currentItem?.error { - Logger.general.error(category: "GalleryVideoItemViewController", message: "Player item status is failed: \(error)") + if let statusNumber = change?[.newKey] as? NSNumber { + if let player = object as? AVPlayer, case .failed = AVPlayer.Status(rawValue: statusNumber.intValue), let error = player.error { + Logger.general.error(category: "GalleryVideoItemViewController", message: "Player can no longer play AVPlayerItem instances because of an error: \(error)") + } else if let item = object as? AVPlayerItem, case .failed = AVPlayerItem.Status(rawValue: statusNumber.intValue), let error = item.error { + Logger.general.error(category: "GalleryVideoItemViewController", message: "Player item can no longer be played because of an error: \(error)") } } else { - Logger.general.error(category: "GalleryVideoItemViewController", message: "Player item status is not yet ready") + if object is AVPlayer { + Logger.general.error(category: "GalleryVideoItemViewController", message: "Player has not tried to load new media resources for playback") + } else if object is AVPlayerItem { + Logger.general.error(category: "GalleryVideoItemViewController", message: "Player item has not tried to load new media resources for playback") + } } } @@ -587,6 +593,20 @@ extension GalleryVideoItemViewController { updateControlView(playControlsHidden: false, otherControlsHidden: false, animated: true) removeTimeObservers() AudioSession.shared.deactivateAsynchronously(client: self, notifyOthersOnDeactivation: false) + let message: String + if let error = notification.userInfo?[AVPlayerItemFailedToPlayToEndTimeErrorKey] as? Error { + message = error.localizedDescription + } else { + message = "" + } + Logger.general.error(category: "GalleryVideoItemViewController", message: "Player item failed to play to end time: \(message)") + } + + @objc private func playerItemNewErrorLogEntry(_ notification: Notification) { + guard let playerItem = notification.object as? AVPlayerItem, let errorLog = playerItem.errorLog() else { + return + } + Logger.general.error(category: "GalleryVideoItemViewController", message: "Player item new error log entry: \(errorLog)") } @objc private func beginScrubbingAction(_ sender: Any) { @@ -708,6 +728,10 @@ extension GalleryVideoItemViewController { forKeyPath: #keyPath(AVPlayerItem.status), options: [.old, .new], context: nil) + player.addObserver(self, + forKeyPath: #keyPath(AVPlayer.status), + options: [.old, .new], + context: nil) let center = NotificationCenter.default center.addObserver(self, @@ -718,6 +742,10 @@ extension GalleryVideoItemViewController { selector: #selector(playerItemFailedToPlayToEndTime(_:)), name: .AVPlayerItemFailedToPlayToEndTime, object: item) + center.addObserver(self, + selector: #selector(playerItemNewErrorLogEntry(_:)), + name: .AVPlayerItemNewErrorLogEntry, + object: item) timeControlObserver = player.observe(\.timeControlStatus, changeHandler: { [weak self] (player, _) in self?.updateControlView() From 5c4ffe5470c80a538299236a8458d74eb724849c Mon Sep 17 00:00:00 2001 From: fanyu Date: Wed, 3 Nov 2021 10:32:58 +0800 Subject: [PATCH 3/5] Remove observer --- .../Controllers/Chat/GalleryVideoItemViewController.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Mixin/UserInterface/Controllers/Chat/GalleryVideoItemViewController.swift b/Mixin/UserInterface/Controllers/Chat/GalleryVideoItemViewController.swift index bdc7dfb284..9526fc285b 100644 --- a/Mixin/UserInterface/Controllers/Chat/GalleryVideoItemViewController.swift +++ b/Mixin/UserInterface/Controllers/Chat/GalleryVideoItemViewController.swift @@ -894,6 +894,7 @@ extension GalleryVideoItemViewController { itemPresentationSizeObserver?.invalidate() timeControlObserver?.invalidate() player.currentItem?.removeObserver(self, forKeyPath: #keyPath(AVPlayerItem.status)) + player.removeObserver(self, forKeyPath: #keyPath(AVPlayer.status)) NotificationCenter.default.removeObserver(self) } From be79681ef2256329d78a29a9be47646bf61e5629 Mon Sep 17 00:00:00 2001 From: fanyu Date: Wed, 3 Nov 2021 13:43:08 +0800 Subject: [PATCH 4/5] Update kvo --- .../Chat/GalleryVideoItemViewController.swift | 49 ++++++------------- 1 file changed, 15 insertions(+), 34 deletions(-) diff --git a/Mixin/UserInterface/Controllers/Chat/GalleryVideoItemViewController.swift b/Mixin/UserInterface/Controllers/Chat/GalleryVideoItemViewController.swift index 9526fc285b..fb6772687f 100644 --- a/Mixin/UserInterface/Controllers/Chat/GalleryVideoItemViewController.swift +++ b/Mixin/UserInterface/Controllers/Chat/GalleryVideoItemViewController.swift @@ -14,6 +14,7 @@ final class GalleryVideoItemViewController: GalleryItemViewController, GalleryAn private var tapRecognizer: UITapGestureRecognizer! private var itemStatusObserver: NSKeyValueObservation? + private var playerStatusObserver: NSKeyValueObservation? private var timeControlObserver: NSKeyValueObservation? private var itemPresentationSizeObserver: NSKeyValueObservation? private var sliderObserver: Any? @@ -440,30 +441,6 @@ extension GalleryVideoItemViewController: AVPictureInPictureControllerDelegate { } -extension GalleryVideoItemViewController { - - override func observeValue(forKeyPath keyPath: String?, of object: Any?, change: [NSKeyValueChangeKey : Any]?, context: UnsafeMutableRawPointer?) { - guard keyPath == #keyPath(AVPlayerItem.status) || keyPath == #keyPath(AVPlayer.status) else { - super.observeValue(forKeyPath: keyPath, of: object, change: change, context: context) - return - } - if let statusNumber = change?[.newKey] as? NSNumber { - if let player = object as? AVPlayer, case .failed = AVPlayer.Status(rawValue: statusNumber.intValue), let error = player.error { - Logger.general.error(category: "GalleryVideoItemViewController", message: "Player can no longer play AVPlayerItem instances because of an error: \(error)") - } else if let item = object as? AVPlayerItem, case .failed = AVPlayerItem.Status(rawValue: statusNumber.intValue), let error = item.error { - Logger.general.error(category: "GalleryVideoItemViewController", message: "Player item can no longer be played because of an error: \(error)") - } - } else { - if object is AVPlayer { - Logger.general.error(category: "GalleryVideoItemViewController", message: "Player has not tried to load new media resources for playback") - } else if object is AVPlayerItem { - Logger.general.error(category: "GalleryVideoItemViewController", message: "Player item has not tried to load new media resources for playback") - } - } - } - -} - extension GalleryVideoItemViewController { @objc func playAction(_ sender: Any) { @@ -720,18 +697,23 @@ extension GalleryVideoItemViewController { // Known issue: https://bugs.swift.org/browse/SR-5872 // 'change' are always nil here self?.updateControlView() + if case .failed = item.status, let error = item.error { + Logger.general.error(category: "GalleryVideoItemViewController", message: "Player item can no longer be played because of an error: \(error)") + } else if case .unknown = item.status { + Logger.general.error(category: "GalleryVideoItemViewController", message: "Player item has not tried to load new media resources for playback") + } } itemPresentationSizeObserver = item.observe(\.presentationSize) { [weak self] (item, _) in self?.updateVideoViewSize(with: item) } - item.addObserver(self, - forKeyPath: #keyPath(AVPlayerItem.status), - options: [.old, .new], - context: nil) - player.addObserver(self, - forKeyPath: #keyPath(AVPlayer.status), - options: [.old, .new], - context: nil) + + playerStatusObserver = player.observe(\.status, options: [.initial, .new]) { player, _ in + if case .failed = player.status, let error = player.error { + Logger.general.error(category: "GalleryVideoItemViewController", message: "Player can no longer play AVPlayerItem instances because of an error: \(error)") + } else if case .unknown = player.status { + Logger.general.error(category: "GalleryVideoItemViewController", message: "Player has not tried to load new media resources for playback") + } + } let center = NotificationCenter.default center.addObserver(self, @@ -893,8 +875,7 @@ extension GalleryVideoItemViewController { itemStatusObserver?.invalidate() itemPresentationSizeObserver?.invalidate() timeControlObserver?.invalidate() - player.currentItem?.removeObserver(self, forKeyPath: #keyPath(AVPlayerItem.status)) - player.removeObserver(self, forKeyPath: #keyPath(AVPlayer.status)) + playerStatusObserver?.invalidate() NotificationCenter.default.removeObserver(self) } From b4a56441ba8e8aee34f645075aa30e591b1534bb Mon Sep 17 00:00:00 2001 From: wuyueyang Date: Wed, 3 Nov 2021 14:28:07 +0800 Subject: [PATCH 5/5] Apply code style --- .../Chat/GalleryVideoItemViewController.swift | 42 ++++++++++++------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/Mixin/UserInterface/Controllers/Chat/GalleryVideoItemViewController.swift b/Mixin/UserInterface/Controllers/Chat/GalleryVideoItemViewController.swift index fb6772687f..28592bda36 100644 --- a/Mixin/UserInterface/Controllers/Chat/GalleryVideoItemViewController.swift +++ b/Mixin/UserInterface/Controllers/Chat/GalleryVideoItemViewController.swift @@ -478,6 +478,7 @@ extension GalleryVideoItemViewController { playerDidReachEnd = false player.seek(to: .zero) } + Logger.general.info(category: "GalleryVideoItemViewController", message: "Playing video") player.play() } else if let url = item.url { controlView.style.insert(.loading) @@ -570,13 +571,13 @@ extension GalleryVideoItemViewController { updateControlView(playControlsHidden: false, otherControlsHidden: false, animated: true) removeTimeObservers() AudioSession.shared.deactivateAsynchronously(client: self, notifyOthersOnDeactivation: false) - let message: String + let errorDescription: String if let error = notification.userInfo?[AVPlayerItemFailedToPlayToEndTimeErrorKey] as? Error { - message = error.localizedDescription + errorDescription = error.localizedDescription } else { - message = "" + errorDescription = "(null)" } - Logger.general.error(category: "GalleryVideoItemViewController", message: "Player item failed to play to end time: \(message)") + Logger.general.error(category: "GalleryVideoItemViewController", message: "Player item failed to play to end time: \(errorDescription)") } @objc private func playerItemNewErrorLogEntry(_ notification: Notification) { @@ -657,6 +658,7 @@ extension GalleryVideoItemViewController { func showReloadAndReport(error: Error?) { if let error = error { reporter.report(error: error) + Logger.general.error(category: "GalleryVideoItemViewController", message: "Failed to load asset: \(error)") } controlView.style.remove(.loading) controlView.playControlStyle = .reload @@ -688,6 +690,7 @@ extension GalleryVideoItemViewController { private func load(playableAsset asset: AVURLAsset, playAfterLoaded: Bool) { guard asset.isPlayable else { // TODO: UI Update + Logger.general.error(category: "GalleryVideoItemViewController", message: "Asset is not playable") return } removeAllObservers() @@ -695,23 +698,33 @@ extension GalleryVideoItemViewController { let item = AVPlayerItem(asset: asset) itemStatusObserver = item.observe(\.status, options: [.initial, .new]) { [weak self] (item, change) in // Known issue: https://bugs.swift.org/browse/SR-5872 - // 'change' are always nil here + // 'change' is always nil here self?.updateControlView() - if case .failed = item.status, let error = item.error { - Logger.general.error(category: "GalleryVideoItemViewController", message: "Player item can no longer be played because of an error: \(error)") - } else if case .unknown = item.status { - Logger.general.error(category: "GalleryVideoItemViewController", message: "Player item has not tried to load new media resources for playback") + switch item.status { + case .readyToPlay: + break + case .failed: + Logger.general.error(category: "GalleryVideoItemViewController", message: "Player item failed: \(item.error?.localizedDescription ?? "(null)")") + case .unknown: + Logger.general.info(category: "GalleryVideoItemViewController", message: "Player item becomes unknown") + @unknown default: + Logger.general.info(category: "GalleryVideoItemViewController", message: "Player item status: \(item.status.rawValue)") } } itemPresentationSizeObserver = item.observe(\.presentationSize) { [weak self] (item, _) in self?.updateVideoViewSize(with: item) } - + playerStatusObserver = player.observe(\.status, options: [.initial, .new]) { player, _ in - if case .failed = player.status, let error = player.error { - Logger.general.error(category: "GalleryVideoItemViewController", message: "Player can no longer play AVPlayerItem instances because of an error: \(error)") - } else if case .unknown = player.status { - Logger.general.error(category: "GalleryVideoItemViewController", message: "Player has not tried to load new media resources for playback") + switch player.status { + case .readyToPlay: + break + case .failed: + Logger.general.error(category: "GalleryVideoItemViewController", message: "Player failed: \(player.error?.localizedDescription ?? "(null)")") + case .unknown: + Logger.general.info(category: "GalleryVideoItemViewController", message: "Player becomes unknown") + @unknown default: + Logger.general.info(category: "GalleryVideoItemViewController", message: "Player status: \(player.status.rawValue)") } } @@ -750,6 +763,7 @@ extension GalleryVideoItemViewController { Logger.general.error(category: "GalleryVideoItemViewController", message: "AudioSession activate error: \(error)") } player.isMuted = mute + Logger.general.info(category: "GalleryVideoItemViewController", message: "Playing video") player.play() } }