From 38a3e3173932239460f7f3e7005d39c104c6cbba Mon Sep 17 00:00:00 2001 From: AJ Lauer Barinov Date: Mon, 10 Jun 2024 12:03:17 -0700 Subject: [PATCH] fix: upload and source asset state handling (#109) * fix: use the file URL for source asset instead of the upload URL, which is the wrong URL * fix: upload cancelled while standardizing shouldn't proceed to the transport stage not needed because if automatically synthesized will default to non-public visibility * refactor: represent source internally as AVURLAsset * update docc plugin --- .../xcshareddata/swiftpm/Package.resolved | 4 +- .../UploadInputStandardizationWorker.swift | 4 +- .../UploadInputStandardizer.swift | 4 +- .../InternalUtilities/UploadInput.swift | 72 +++++------ .../DirectUpload+AVFoundation.swift | 9 +- .../MuxUploadSDK/PublicAPI/DirectUpload.swift | 120 +++++++++++++----- .../PublicAPI/DirectUploadManager.swift | 18 ++- Sources/MuxUploadSDK/Upload/UploadInfo.swift | 8 -- .../Helpers/UploadInput+Fixtures.swift | 4 +- .../UploadManagerTests.swift | 4 +- 10 files changed, 154 insertions(+), 93 deletions(-) diff --git a/Example/SwiftUploadSDKExample/SwiftUploadSDKExample.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/Example/SwiftUploadSDKExample/SwiftUploadSDKExample.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index bb40ae73..e65252d0 100644 --- a/Example/SwiftUploadSDKExample/SwiftUploadSDKExample.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/Example/SwiftUploadSDKExample/SwiftUploadSDKExample.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -5,8 +5,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/apple/swift-docc-plugin", "state" : { - "revision" : "9b1258905c21fc1b97bf03d1b4ca12c4ec4e5fda", - "version" : "1.2.0" + "revision" : "26ac5758409154cc448d7ab82389c520fa8a8247", + "version" : "1.3.0" } }, { diff --git a/Sources/MuxUploadSDK/InputStandardization/UploadInputStandardizationWorker.swift b/Sources/MuxUploadSDK/InputStandardization/UploadInputStandardizationWorker.swift index 13d8f02d..2fa17b26 100644 --- a/Sources/MuxUploadSDK/InputStandardization/UploadInputStandardizationWorker.swift +++ b/Sources/MuxUploadSDK/InputStandardization/UploadInputStandardizationWorker.swift @@ -42,10 +42,10 @@ class UploadInputStandardizationWorker { var standardizedInput: AVAsset? func standardize( - sourceAsset: AVAsset, + sourceAsset: AVURLAsset, rescalingDetails: UploadInputFormatInspectionResult.RescalingDetails, outputURL: URL, - completion: @escaping (AVAsset, AVAsset?, Error?) -> () + completion: @escaping (AVURLAsset, AVAsset?, Error?) -> () ) { let availableExportPresets = AVAssetExportSession.allExportPresets() diff --git a/Sources/MuxUploadSDK/InputStandardization/UploadInputStandardizer.swift b/Sources/MuxUploadSDK/InputStandardization/UploadInputStandardizer.swift index b3f4dc7d..56e56cc5 100644 --- a/Sources/MuxUploadSDK/InputStandardization/UploadInputStandardizer.swift +++ b/Sources/MuxUploadSDK/InputStandardization/UploadInputStandardizer.swift @@ -10,10 +10,10 @@ class UploadInputStandardizer { func standardize( id: String, - sourceAsset: AVAsset, + sourceAsset: AVURLAsset, rescalingDetails: UploadInputFormatInspectionResult.RescalingDetails, outputURL: URL, - completion: @escaping (AVAsset, AVAsset?, Error?) -> () + completion: @escaping (AVURLAsset, AVAsset?, Error?) -> () ) { let worker = UploadInputStandardizationWorker() diff --git a/Sources/MuxUploadSDK/InternalUtilities/UploadInput.swift b/Sources/MuxUploadSDK/InternalUtilities/UploadInput.swift index 97100c1a..2b3e2092 100644 --- a/Sources/MuxUploadSDK/InternalUtilities/UploadInput.swift +++ b/Sources/MuxUploadSDK/InternalUtilities/UploadInput.swift @@ -10,26 +10,26 @@ import Foundation struct UploadInput { internal enum Status { - case ready(AVAsset, UploadInfo) - case started(AVAsset, UploadInfo) - case underInspection(AVAsset, UploadInfo) - case standardizing(AVAsset, UploadInfo) + case ready(AVURLAsset, UploadInfo) + case started(AVURLAsset, UploadInfo) + case underInspection(AVURLAsset, UploadInfo) + case standardizing(AVURLAsset, UploadInfo) case standardizationSucceeded( - source: AVAsset, - standardized: AVAsset?, + source: AVURLAsset, + standardized: AVURLAsset?, uploadInfo: UploadInfo ) - case standardizationFailed(AVAsset, UploadInfo) - case awaitingUploadConfirmation(UploadInfo) - case uploadInProgress(UploadInfo, DirectUpload.TransportStatus) - case uploadPaused(UploadInfo, DirectUpload.TransportStatus) - case uploadSucceeded(UploadInfo, DirectUpload.SuccessDetails) - case uploadFailed(UploadInfo, DirectUploadError) + case standardizationFailed(AVURLAsset, UploadInfo) + case awaitingUploadConfirmation(AVURLAsset,UploadInfo) + case uploadInProgress(AVURLAsset, UploadInfo, DirectUpload.TransportStatus) + case uploadPaused(AVURLAsset, UploadInfo, DirectUpload.TransportStatus) + case uploadSucceeded(AVURLAsset, UploadInfo, DirectUpload.SuccessDetails) + case uploadFailed(AVURLAsset, UploadInfo, DirectUploadError) } var status: Status - var sourceAsset: AVAsset { + var sourceAsset: AVURLAsset { switch status { case .ready(let sourceAsset, _): return sourceAsset @@ -43,16 +43,16 @@ struct UploadInput { return sourceAsset case .standardizationFailed(let sourceAsset, _): return sourceAsset - case .awaitingUploadConfirmation(let uploadInfo): - return uploadInfo.sourceAsset() - case .uploadInProgress(let uploadInfo, _): - return uploadInfo.sourceAsset() - case .uploadSucceeded(let uploadInfo, _): - return uploadInfo.sourceAsset() - case .uploadFailed(let uploadInfo, _): - return uploadInfo.sourceAsset() - case .uploadPaused(let uploadInfo, _): - return uploadInfo.sourceAsset() + case .awaitingUploadConfirmation(let sourceAsset, _): + return sourceAsset + case .uploadInProgress(let sourceAsset, _, _): + return sourceAsset + case .uploadSucceeded(let sourceAsset, _, _): + return sourceAsset + case .uploadFailed(let sourceAsset, _, _): + return sourceAsset + case .uploadPaused(let sourceAsset, _, _): + return sourceAsset } } @@ -70,15 +70,15 @@ struct UploadInput { return uploadInfo case .standardizationFailed(_, let uploadInfo): return uploadInfo - case .awaitingUploadConfirmation(let uploadInfo): + case .awaitingUploadConfirmation(_, let uploadInfo): return uploadInfo - case .uploadInProgress(let uploadInfo, _): + case .uploadInProgress(_, let uploadInfo, _): return uploadInfo - case .uploadPaused(let uploadInfo, _): + case .uploadPaused(_, let uploadInfo, _): return uploadInfo - case .uploadSucceeded(let uploadInfo, _): + case .uploadSucceeded(_, let uploadInfo, _): return uploadInfo - case .uploadFailed(let uploadInfo, _): + case .uploadFailed(_, let uploadInfo, _): return uploadInfo } } @@ -99,11 +99,11 @@ struct UploadInput { return nil case .awaitingUploadConfirmation: return nil - case .uploadInProgress(_, let transportStatus): + case .uploadInProgress(_, _, let transportStatus): return transportStatus - case .uploadPaused(_, let transportStatus): + case .uploadPaused(_, _, let transportStatus): return transportStatus - case .uploadSucceeded(_, let successDetails): + case .uploadSucceeded(_, _, let successDetails): return successDetails.finalState case .uploadFailed: return nil @@ -125,7 +125,7 @@ extension UploadInput { startingTransportStatus: DirectUpload.TransportStatus ) { if case UploadInput.Status.underInspection = status { - status = .uploadInProgress(uploadInfo, startingTransportStatus) + status = .uploadInProgress(sourceAsset, uploadInfo, startingTransportStatus) } else { return } @@ -134,15 +134,15 @@ extension UploadInput { mutating func processUploadSuccess( transportStatus: DirectUpload.TransportStatus ) { - if case UploadInput.Status.uploadInProgress(let info, _) = status { - status = .uploadSucceeded(info, DirectUpload.SuccessDetails(finalState: transportStatus)) + if case UploadInput.Status.uploadInProgress(let asset, let info, _) = status { + status = .uploadSucceeded(asset, info, DirectUpload.SuccessDetails(finalState: transportStatus)) } else { return } } mutating func processUploadFailure(error: DirectUploadError) { - status = .uploadFailed(uploadInfo, error) + status = .uploadFailed(sourceAsset, uploadInfo, error) } } @@ -151,7 +151,7 @@ extension UploadInput.Status: Equatable { } extension UploadInput { init( - asset: AVAsset, + asset: AVURLAsset, info: UploadInfo ) { self.status = .ready(asset, info) diff --git a/Sources/MuxUploadSDK/PublicAPI/AVFoundation+DirectUpload/DirectUpload+AVFoundation.swift b/Sources/MuxUploadSDK/PublicAPI/AVFoundation+DirectUpload/DirectUpload+AVFoundation.swift index 12ff1b41..9b6115f8 100644 --- a/Sources/MuxUploadSDK/PublicAPI/AVFoundation+DirectUpload/DirectUpload+AVFoundation.swift +++ b/Sources/MuxUploadSDK/PublicAPI/AVFoundation+DirectUpload/DirectUpload+AVFoundation.swift @@ -22,9 +22,16 @@ extension DirectUpload { inputAsset: AVAsset, options: DirectUploadOptions ) { + guard let urlAsset = inputAsset as? AVURLAsset else { + precondition( + false, + "Only assets with URLs can be uploaded" + ) + } + self.init( input: UploadInput( - asset: inputAsset, + asset: urlAsset, info: UploadInfo( uploadURL: uploadURL, options: options diff --git a/Sources/MuxUploadSDK/PublicAPI/DirectUpload.swift b/Sources/MuxUploadSDK/PublicAPI/DirectUpload.swift index 10903c0c..e064330a 100644 --- a/Sources/MuxUploadSDK/PublicAPI/DirectUpload.swift +++ b/Sources/MuxUploadSDK/PublicAPI/DirectUpload.swift @@ -107,33 +107,62 @@ public final class DirectUpload { return InputStatus.preparing(sourceAsset) case .standardizationFailed(let sourceAsset, _): return InputStatus.preparing(sourceAsset) - case .awaitingUploadConfirmation(let uploadInfo): + case .awaitingUploadConfirmation(let sourceAsset, _): return InputStatus.awaitingConfirmation( - uploadInfo.sourceAsset() + sourceAsset ) - case .uploadInProgress(let uploadInfo, let transportStatus): + case .uploadInProgress(let sourceAsset, _, let transportStatus): return InputStatus.transportInProgress( - uploadInfo.sourceAsset(), + sourceAsset, transportStatus ) - case .uploadPaused(let uploadInfo, let transportStatus): + case .uploadPaused(let sourceAsset, _, let transportStatus): return InputStatus.paused( - uploadInfo.sourceAsset(), + sourceAsset, transportStatus ) - case .uploadSucceeded(let uploadInfo, let success): + case .uploadSucceeded(let sourceAsset, _, let success): return InputStatus.finished( - uploadInfo.sourceAsset(), + sourceAsset, .success(success) ) - case .uploadFailed(let uploadInfo, let error): + case .uploadFailed(let sourceAsset, _, let error): return InputStatus.finished( - uploadInfo.sourceAsset(), + sourceAsset, .failure(error) ) } } + + /// AVAsset containing the input source + public var inputAsset: AVAsset { + switch input.status { + case .ready(let sourceAsset, _): + return sourceAsset + case .started(let sourceAsset, _): + return sourceAsset + case .underInspection(let sourceAsset, _): + return sourceAsset + case .standardizing(let sourceAsset, _): + return sourceAsset + case .standardizationSucceeded(let sourceAsset, _, _): + return sourceAsset + case .standardizationFailed(let sourceAsset, _): + return sourceAsset + case .awaitingUploadConfirmation(let sourceAsset, _): + return sourceAsset + case .uploadInProgress(let sourceAsset, _, _): + return sourceAsset + case .uploadPaused(let sourceAsset, _, _): + return sourceAsset + case .uploadSucceeded(let sourceAsset, _, _): + return sourceAsset + case .uploadFailed(let sourceAsset, _, _): + return sourceAsset + } + } + /// Handles a change in the input status of the upload public typealias InputStatusHandler = (InputStatus) -> () @@ -199,7 +228,7 @@ public final class DirectUpload { inputStandardization: DirectUploadOptions.InputStandardization = .default, eventTracking: DirectUploadOptions.EventTracking = .default ) { - let asset = AVAsset(url: videoFileURL) + let asset = AVURLAsset(url: videoFileURL) self.init( input: UploadInput( asset: asset, @@ -236,7 +265,7 @@ public final class DirectUpload { inputFileURL: URL, options: DirectUploadOptions = .default ) { - let asset = AVAsset( + let asset = AVURLAsset( url: inputFileURL ) self.init( @@ -286,6 +315,7 @@ public final class DirectUpload { self.init( input: UploadInput( status: .uploadInProgress( + AVURLAsset(url: uploader.inputFileURL), uploader.uploadInfo, TransportStatus( progress: uploader.currentState.progress ?? Progress(), @@ -380,13 +410,10 @@ public final class DirectUpload { /// restarted. If false the upload will resume from where /// it left off if paused, otherwise the upload will change. public func start(forceRestart: Bool = false) { - - let videoFile = (input.sourceAsset as! AVURLAsset).url - if self.manageBySDK { // See if there's anything in progress already fileWorker = uploadManager.findChunkedFileUploader( - inputFileURL: videoFile + inputFileURL: input.sourceAsset.url ) } if fileWorker != nil && !forceRestart { @@ -405,17 +432,17 @@ public final class DirectUpload { if case UploadInput.Status.ready = input.status { input.status = .started(input.sourceAsset, uploadInfo) - startInspection(videoFile: videoFile) + startInspection(sourceAsset: input.sourceAsset) } else if forceRestart { cancel() } } func startInspection( - videoFile: URL + sourceAsset: AVURLAsset ) { if !uploadInfo.options.inputStandardization.isRequested { - startNetworkTransport(videoFile: videoFile) + startNetworkTransport(videoFile: sourceAsset.url) } else { let inputStandardizationStartTime = Date() let reporter = Reporter.shared @@ -427,7 +454,7 @@ public final class DirectUpload { // instead throw an error since upload // will likely fail let inputSize = (try? FileManager.default.fileSizeOfItem( - atPath: videoFile.path + atPath: input.sourceAsset.url.absoluteString )) ?? 0 input.status = .underInspection(input.sourceAsset, uploadInfo) @@ -445,7 +472,7 @@ public final class DirectUpload { inputDuration: inputDuration, inputSize: inputSize, inputStandardizationStartTime: inputStandardizationStartTime, - videoFile: videoFile + sourceAsset: sourceAsset ) case (.none, .some(let error)): self.handleInspectionFailure( @@ -453,7 +480,7 @@ public final class DirectUpload { inputDuration: inputDuration, inputSize: inputSize, inputStandardizationStartTime: inputStandardizationStartTime, - videoFile: videoFile + sourceAsset: sourceAsset ) case (.some(let result), .none): if result.isStandardInput { @@ -474,12 +501,12 @@ public final class DirectUpload { self.inputStandardizer.standardize( id: self.id, - sourceAsset: AVAsset(url: videoFile), + sourceAsset: sourceAsset, rescalingDetails: result.rescalingDetails, outputURL: outputURL ) { sourceAsset, standardizedAsset, error in - if let error { + if let _ = error { // Request upload confirmation // before proceeding. If handler unset, // by default do not cancel upload if @@ -488,7 +515,7 @@ public final class DirectUpload { if !shouldCancelUpload { self.startNetworkTransport( - videoFile: videoFile + videoFile: sourceAsset.url ) } else { self.fileWorker?.cancel() @@ -507,7 +534,7 @@ public final class DirectUpload { } else { self.startNetworkTransport( - videoFile: videoFile + videoFile: sourceAsset.url ) } } else { @@ -531,7 +558,7 @@ public final class DirectUpload { self.inputStandardizer.standardize( id: self.id, - sourceAsset: AVAsset(url: videoFile), + sourceAsset: sourceAsset, rescalingDetails: result.rescalingDetails, outputURL: outputURL ) { sourceAsset, standardizedAsset, error in @@ -557,7 +584,7 @@ public final class DirectUpload { if !shouldCancelUpload { self.startNetworkTransport( - videoFile: videoFile + videoFile: sourceAsset.url ) } else { self.fileWorker?.cancel() @@ -584,13 +611,13 @@ public final class DirectUpload { self.inputStandardizer.acknowledgeCompletion(id: self.id) } } - case (.some(let result), .some(let error)): + case (.some(_), .some(let error)): self.handleInspectionFailure( inspectionError: error, inputDuration: inputDuration, inputSize: inputSize, inputStandardizationStartTime: inputStandardizationStartTime, - videoFile: videoFile + sourceAsset: sourceAsset ) } } @@ -602,7 +629,7 @@ public final class DirectUpload { inputDuration: CMTime, inputSize: UInt64, inputStandardizationStartTime: Date, - videoFile: URL + sourceAsset: AVURLAsset ) { let reporter = Reporter.shared // Request upload confirmation @@ -625,7 +652,7 @@ public final class DirectUpload { if !shouldCancelUpload { self.startNetworkTransport( - videoFile: videoFile + videoFile: sourceAsset.url ) } else { self.fileWorker?.cancel() @@ -634,9 +661,32 @@ public final class DirectUpload { } } + func readyForTransport() -> Bool { + switch inputStatus { + case .ready: + return false + case .started: + return true + case .preparing: + return true + case .awaitingConfirmation: + return true + case .transportInProgress: + return false + case .paused: + return false + case .finished: + return false + } + } + func startNetworkTransport( videoFile: URL ) { + guard readyForTransport() else { + return + } + let completedUnitCount = UInt64(uploadStatus?.progress?.completedUnitCount ?? 0) let fileWorker = ChunkedFileUploader( @@ -668,6 +718,11 @@ public final class DirectUpload { videoFile: URL, duration: CMTime ) { + + guard readyForTransport() else { + return + } + let completedUnitCount = UInt64(uploadStatus?.progress?.completedUnitCount ?? 0) let fileWorker = ChunkedFileUploader( @@ -774,6 +829,7 @@ public final class DirectUpload { ) } else { input.status = .uploadInProgress( + input.sourceAsset, input.uploadInfo, status ) diff --git a/Sources/MuxUploadSDK/PublicAPI/DirectUploadManager.swift b/Sources/MuxUploadSDK/PublicAPI/DirectUploadManager.swift index 5a36afc0..c92f8e78 100644 --- a/Sources/MuxUploadSDK/PublicAPI/DirectUploadManager.swift +++ b/Sources/MuxUploadSDK/PublicAPI/DirectUploadManager.swift @@ -145,13 +145,13 @@ public final class DirectUploadManager { uploadsByID.updateValue(UploadStorage(upload: upload), forKey: upload.id) fileWorker.addDelegate(withToken: UUID().uuidString, uploaderDelegate) + self.notifyDelegates() Task.detached { await self.uploadActor.updateUpload( fileWorker.uploadInfo, fileInputURL: fileWorker.inputFileURL, withUpdate: fileWorker.currentState ) - self.notifyDelegates() } } @@ -170,13 +170,15 @@ public final class DirectUploadManager { /// The shared instance of this object that should be used public static let shared = DirectUploadManager() - internal init() { } private struct FileUploaderDelegate : ChunkedFileUploaderDelegate { let manager: DirectUploadManager - func chunkedFileUploader(_ uploader: ChunkedFileUploader, stateUpdated state: ChunkedFileUploader.InternalUploadState) { - Task.detached { + func chunkedFileUploader( + _ uploader: ChunkedFileUploader, + stateUpdated state: ChunkedFileUploader.InternalUploadState + ) { + let _ = Task.detached { await manager.uploadActor.updateUpload( uploader.uploadInfo, fileInputURL: uploader.inputFileURL, @@ -185,8 +187,12 @@ public final class DirectUploadManager { manager.notifyDelegates() } switch state { - case .success(_), .canceled: manager.acknowledgeUpload(id: uploader.uploadInfo.id) - default: do { } + case .canceled: + manager.acknowledgeUpload( + id: uploader.uploadInfo.id + ) + default: + break } } } diff --git a/Sources/MuxUploadSDK/Upload/UploadInfo.swift b/Sources/MuxUploadSDK/Upload/UploadInfo.swift index 8c7661dc..020f6120 100644 --- a/Sources/MuxUploadSDK/Upload/UploadInfo.swift +++ b/Sources/MuxUploadSDK/Upload/UploadInfo.swift @@ -29,11 +29,3 @@ struct UploadInfo : Codable { } extension UploadInfo: Equatable { } - -extension UploadInfo { - func sourceAsset() -> AVAsset { - AVAsset( - url: uploadURL - ) - } -} diff --git a/Tests/MuxUploadSDKTests/Helpers/UploadInput+Fixtures.swift b/Tests/MuxUploadSDKTests/Helpers/UploadInput+Fixtures.swift index 0fec1649..a261b3b4 100644 --- a/Tests/MuxUploadSDKTests/Helpers/UploadInput+Fixtures.swift +++ b/Tests/MuxUploadSDKTests/Helpers/UploadInput+Fixtures.swift @@ -19,7 +19,7 @@ extension UploadInput { URL(string: "file://path/to/dummy/file/") ) - let uploadInputAsset = AVAsset( + let uploadInputAsset = AVURLAsset( url: videoInputURL ) @@ -43,7 +43,7 @@ extension UploadInput { URL(string: "file://path/to/dummy/file/") ) - let uploadInputAsset = AVAsset( + let uploadInputAsset = AVURLAsset( url: videoInputURL ) diff --git a/Tests/MuxUploadSDKTests/UploadManagerTests/UploadManagerTests.swift b/Tests/MuxUploadSDKTests/UploadManagerTests/UploadManagerTests.swift index 01b4eea3..47c4e7ab 100644 --- a/Tests/MuxUploadSDKTests/UploadManagerTests/UploadManagerTests.swift +++ b/Tests/MuxUploadSDKTests/UploadManagerTests/UploadManagerTests.swift @@ -34,7 +34,7 @@ class UploadManagerTests: XCTestCase { let upload = DirectUpload( input: UploadInput( - asset: AVAsset(url: videoInputURL), + asset: AVURLAsset(url: videoInputURL), info: UploadInfo( uploadURL: uploadURL, options: .inputStandardizationSkipped @@ -45,7 +45,7 @@ class UploadManagerTests: XCTestCase { let duplicateUpload = DirectUpload( input: UploadInput( - asset: AVAsset(url: videoInputURL), + asset: AVURLAsset(url: videoInputURL), info: UploadInfo( uploadURL: uploadURL, options: .inputStandardizationSkipped