From daf1e81041b94381c4e344d522ffdaa3612bebd3 Mon Sep 17 00:00:00 2001 From: Simon McLoughlin Date: Tue, 23 Jan 2024 10:21:58 +0000 Subject: [PATCH] - move collectibles details code into a completion block to avoid race condition - remove an unnecessary extra call to load - add error handling to video player --- .../Detail/CollectibleDetailAVCell.swift | 28 ++++++++++ .../Detail/CollectibleDetailImageCell.swift | 4 +- .../CollectiblesDetailsViewModel.swift | 55 +++++++++---------- 3 files changed, 58 insertions(+), 29 deletions(-) diff --git a/Kukai Mobile/Modules/Collectibles/Cells/Detail/CollectibleDetailAVCell.swift b/Kukai Mobile/Modules/Collectibles/Cells/Detail/CollectibleDetailAVCell.swift index 28fe82b8..ca3128a9 100644 --- a/Kukai Mobile/Modules/Collectibles/Cells/Detail/CollectibleDetailAVCell.swift +++ b/Kukai Mobile/Modules/Collectibles/Cells/Detail/CollectibleDetailAVCell.swift @@ -9,6 +9,7 @@ import UIKit import AVKit import KukaiCoreSwift import MediaPlayer +import OSLog class CollectibleDetailAVCell: UICollectionViewCell { @@ -30,6 +31,7 @@ class CollectibleDetailAVCell: UICollectionViewCell { private var playbackWillKeepUpObserver: NSKeyValueObservation? = nil private var rateObserver: NSKeyValueObservation? = nil + private var errorObserver: NSKeyValueObservation? = nil private var commandCentreTargetStop: Any? = nil private var commandCentreTargetToggle: Any? = nil private var commandCentreTargetPlay: Any? = nil @@ -87,6 +89,29 @@ class CollectibleDetailAVCell: UICollectionViewCell { } }) + self.errorObserver = avplayerController.player?.currentItem?.observe(\.status, changeHandler: { [weak self] item, change in + switch item.status { + case .readyToPlay: + // Handled elsewhere + break + + case .failed: + self?.mediaActivityView.stopAnimating() + self?.mediaActivityView.isHidden = true + Logger.app.error("AVPlayer - Error: \(String(describing: item.error)), Message: \(String(describing: item.error?.localizedDescription))") + + case .unknown: + self?.mediaActivityView.stopAnimating() + self?.mediaActivityView.isHidden = true + Logger.app.error("AVPlayer - unknown: \(String(describing: item.error)), Message: \(String(describing: item.error?.localizedDescription))") + + @unknown default: + self?.mediaActivityView.stopAnimating() + self?.mediaActivityView.isHidden = true + Logger.app.error("AVPlayer - default/unknown: \(String(describing: item.error)), Message: \(String(describing: item.error?.localizedDescription))") + } + }) + // if allowsExternalPlayback set to false, during airplay via the command centre, iOS correctly picks up that its a song and shows the album artwork + title + album // With this setup, videos however do not cast to the external device @@ -137,6 +162,9 @@ class CollectibleDetailAVCell: UICollectionViewCell { rateObserver?.invalidate() rateObserver = nil + errorObserver?.invalidate() + errorObserver = nil + clearCommandCenterCommands() } diff --git a/Kukai Mobile/Modules/Collectibles/Cells/Detail/CollectibleDetailImageCell.swift b/Kukai Mobile/Modules/Collectibles/Cells/Detail/CollectibleDetailImageCell.swift index 8e6838b9..664053bb 100644 --- a/Kukai Mobile/Modules/Collectibles/Cells/Detail/CollectibleDetailImageCell.swift +++ b/Kukai Mobile/Modules/Collectibles/Cells/Detail/CollectibleDetailImageCell.swift @@ -33,7 +33,9 @@ class CollectibleDetailImageCell: UICollectionViewCell { // Load image if not only perfroming collectionview layout logic if !layoutOnly { - MediaProxyService.load(url: mediaContent.mediaURL, to: imageView, withCacheType: .temporary, fallback: UIImage.unknownThumb()) + MediaProxyService.load(url: mediaContent.mediaURL, to: imageView, withCacheType: .temporary, fallback: UIImage.unknownThumb()) { [weak self] _ in + self?.activityIndicator.isHidden = true + } } setup = true diff --git a/Kukai Mobile/Modules/Collectibles/CollectiblesDetailsViewModel.swift b/Kukai Mobile/Modules/Collectibles/CollectiblesDetailsViewModel.swift index 10e34a16..7cb00865 100644 --- a/Kukai Mobile/Modules/Collectibles/CollectiblesDetailsViewModel.swift +++ b/Kukai Mobile/Modules/Collectibles/CollectiblesDetailsViewModel.swift @@ -220,42 +220,41 @@ class CollectiblesDetailsViewModel: ViewModel, UICollectionViewDiffableDataSourc section1Content.append(self.descriptionData) self.currentSnapshot.appendItems(section1Content, toSection: 0) - ds.apply(self.currentSnapshot, animatingDifferences: animate) - self.state = .success(nil) - - - // If unbale to determine contentn type, we need to do a network request to find it - if response.needsMediaTypeVerification { - self.mediaContentForFailedOfflineFetch(forNFT: self.nft) { [weak self] mediaContent in + ds.apply(self.currentSnapshot, animatingDifferences: animate) { + + // If unbale to determine content type, we need to do a network request to find it + if response.needsMediaTypeVerification { + self.mediaContentForFailedOfflineFetch(forNFT: self.nft) { [weak self] mediaContent in + + if let newMediaContent = mediaContent { + self?.replace(existingMediaContent: response.mediaContent, with: newMediaContent) + } else { + // Unbale to determine type and unable to locate URL, or fetch packet from URL. Default to missing image palceholder + let blankMediaContent = MediaContent(isImage: true, isThumbnail: false, mediaURL: nil, mediaURL2: nil, width: 100, height: 100) + self?.replace(existingMediaContent: response.mediaContent, with: blankMediaContent) + } + } + } + + // If we don't have the full image cached, download it and replace the thumbnail with the real thing + else if response.needsToDownloadFullImage { + let newURL = MediaProxyService.url(fromUri: self.nft?.displayURI, ofFormat: MediaProxyService.Format.large.rawFormat()) + let isDualURL = (response.mediaContent.mediaURL2 != nil) - if let newMediaContent = mediaContent { + MediaProxyService.cacheImage(url: newURL) { [weak self] size in + let mediaURL1 = isDualURL ? response.mediaContent.mediaURL : newURL + let mediaURL2 = isDualURL ? newURL : nil + let width = Double(size?.width ?? 300) + let height = Double(size?.height ?? 300) + let newMediaContent = MediaContent(isImage: response.mediaContent.isImage, isThumbnail: false, mediaURL: mediaURL1, mediaURL2: mediaURL2, width: width, height: height) self?.replace(existingMediaContent: response.mediaContent, with: newMediaContent) - } else { - // Unbale to determine type and unable to locate URL, or fetch packet from URL. Default to missing image palceholder - let blankMediaContent = MediaContent(isImage: true, isThumbnail: false, mediaURL: nil, mediaURL2: nil, width: 100, height: 100) - self?.replace(existingMediaContent: response.mediaContent, with: blankMediaContent) } } } - // If we don't have the full image cached, download it and replace the thumbnail with the real thing - else if response.needsToDownloadFullImage { - let newURL = MediaProxyService.url(fromUri: self.nft?.displayURI, ofFormat: MediaProxyService.Format.large.rawFormat()) - let isDualURL = (response.mediaContent.mediaURL2 != nil) - - MediaProxyService.cacheImage(url: newURL) { [weak self] size in - let mediaURL1 = isDualURL ? response.mediaContent.mediaURL : newURL - let mediaURL2 = isDualURL ? newURL : nil - let width = Double(size?.width ?? 300) - let height = Double(size?.height ?? 300) - let newMediaContent = MediaContent(isImage: response.mediaContent.isImage, isThumbnail: false, mediaURL: mediaURL1, mediaURL2: mediaURL2, width: width, height: height) - self?.replace(existingMediaContent: response.mediaContent, with: newMediaContent) - } - } + self.state = .success(nil) } - ds.apply(self.currentSnapshot, animatingDifferences: animate) - // Load remote data after UI let address = DependencyManager.shared.selectedWalletAddress ?? ""