Replace video file get with actual implementation #64
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cjpillsbury
reviewed
Jul 13, 2023
| public var videoFile: URL? { | ||
| #warning("This isn't the actual implementation, should return either the exported asset file URL or the standardized file URL depending on the inspection result") | ||
| return (input.sourceAsset as? AVURLAsset)?.url | ||
| return fileWorker?.inputFileURL |
Contributor
There was a problem hiding this comment.
question(blocking): The original warning called out an "or", but that isn't obviously captured in this implementation change, particularly given the naming of fileWorker.inputFileUrl. Mind explaining what happened there?
Contributor
Author
There was a problem hiding this comment.
Good question, fileWorker is initialized with the correct file URL when the upload is done preparing.
Contributor
There was a problem hiding this comment.
cool that's what I was hoping/assuming. So inputFileURL may be slightly confusing since it may actually be the "standardized file URL". Not requesting a change here, but food for thought.
Contributor
Author
There was a problem hiding this comment.
input is a reference to the Mux Video defined term for what we actually send to the direct upload.
I'd prefer to keep internal SDK terminology consistent with Mux Video to the extent possible so it's easy for those new to the codebase to navigate it.
Contributor
Author
There was a problem hiding this comment.
Additionally, whether the file is standardized or not is irrelevant to ChunkFileUploader.
cjpillsbury
approved these changes
Jul 13, 2023
andrewjl-mux
added a commit
that referenced
this pull request
Jul 13, 2023
andrewjl-mux
added a commit
that referenced
this pull request
Jul 18, 2023
andrewjl-mux
added a commit
that referenced
this pull request
Jul 19, 2023
andrewjl-mux
added a commit
that referenced
this pull request
Jul 20, 2023
Upload Input Inspection and Standardization 1 (#17) Add scaffolding Add initializers Add internal and external state mapping Remove duplicate status enum and add inline docs to external status Add inline API docs to PHAsset-based MuxUpload constructor Consolidate all `MuxUpload` options into a single struct `UploadOptions` Declare asynchronous MuxUpload constructor in PHAsset extension Place extension methods into dedicated directories Polish inline API documentation Add new API documentation and note the placeholder implementation Add option variants as static members: defaults, disabled inputStandardization Deprecate existing initializer, normally this API should be removed prior to GA, but since it was the only initializer exposed up to this point removing it would break everybody. Instead deprecate and remove at a later date. Store all MuxUpload-related options in UploadInfo Use correct starting byte parameter when restarting upload Hardcode request options when exporting AVAsset from PhotosKit chunkSize -> chunkSizeInBytes Input standardization 2 (#41) Maintain handle to canonical input asset inside UploadInput If input standardized, standardized input URL is passed to UploadInfo instead of the original input URL used for initializer Note: SDK probably needs to re-export a high quality asset anyway so possibly need a bridging status Add dedicated internal initializer for MuxUpload error with unknown error code Disable input standardization when running tests Add inspection logic Request local and remote assets Status -> TransportStatus Rely on input status in MuxUpload computed property getters Expose progress from internal upload state if available Track transport status inside of UploadInput Transport status start time optional Add TransportStatus docs Inspect and check video track frame rate Migrate tests for MuxUpload input result handler Input standardization 3 (#46) Standardize via export session Back out outputURL construction into MuxUpload Expose hook for client to cancel upload if standardization failed Call cancellation hook if inspection fails. We're not sure if the input is standard or not so better to be safe and confirm Export based on maximum resolution set by client Cleaner non standard input handler invocation (#57) Add CustomStringConvertible conformance to maximum resolution (#56) Update input state when upload is finished (#63) Update input state when the upload succeeds or fails Tweak input state definition, keep an error in the failed case and the success struct in the success case NFC: remove some if let checks for the handler closure properties in MuxUpload and replace with ? operator. Keeps the existing logic. Replace video file get with actual implementation (#64) Reporting updates (#49) Only mark upload as started if its ready Add upload failed and input standardization events Add standardization failure event handler to reporter Add standardization success event handler to reporter Rename method for upload success Rename upload event to upload succeeded Support dispatching multiple events at the same time Naive non-thread safe implementation reporter networking Change event type casing, individual files for events Fix reporter test Actually report upload failure Read off file size Use Date in events and serialize to ISO8601 string Finangle duration and start time Set correct version in correct place Keep a session UUID Route every request through one chokepoint Upload failed test Shared test encoder Input standardization failed test Input standardization succeeded test Better organize expected json strings Use a shared instance Remove dupe file Thread asset duration through inspection and standardization Pass export error back Include non standard input reasons Workaround duration load pause Avoid mutating while iterating when UploadManager makes delegate calls Set custom HTTP request header for reporting (#67) Remove Equatable and Hashable conformance from MuxUpload (#68) Safe storage for MuxUpload (#71) Intended to prevent a crash if MuxUpload is extended by the SDK client to conform to Equatable or Hashable protocols
cjpillsbury
pushed a commit
that referenced
this pull request
Jul 20, 2023
Upload Input Inspection and Standardization 1 (#17) Add scaffolding Add initializers Add internal and external state mapping Remove duplicate status enum and add inline docs to external status Add inline API docs to PHAsset-based MuxUpload constructor Consolidate all `MuxUpload` options into a single struct `UploadOptions` Declare asynchronous MuxUpload constructor in PHAsset extension Place extension methods into dedicated directories Polish inline API documentation Add new API documentation and note the placeholder implementation Add option variants as static members: defaults, disabled inputStandardization Deprecate existing initializer, normally this API should be removed prior to GA, but since it was the only initializer exposed up to this point removing it would break everybody. Instead deprecate and remove at a later date. Store all MuxUpload-related options in UploadInfo Use correct starting byte parameter when restarting upload Hardcode request options when exporting AVAsset from PhotosKit chunkSize -> chunkSizeInBytes Input standardization 2 (#41) Maintain handle to canonical input asset inside UploadInput If input standardized, standardized input URL is passed to UploadInfo instead of the original input URL used for initializer Note: SDK probably needs to re-export a high quality asset anyway so possibly need a bridging status Add dedicated internal initializer for MuxUpload error with unknown error code Disable input standardization when running tests Add inspection logic Request local and remote assets Status -> TransportStatus Rely on input status in MuxUpload computed property getters Expose progress from internal upload state if available Track transport status inside of UploadInput Transport status start time optional Add TransportStatus docs Inspect and check video track frame rate Migrate tests for MuxUpload input result handler Input standardization 3 (#46) Standardize via export session Back out outputURL construction into MuxUpload Expose hook for client to cancel upload if standardization failed Call cancellation hook if inspection fails. We're not sure if the input is standard or not so better to be safe and confirm Export based on maximum resolution set by client Cleaner non standard input handler invocation (#57) Add CustomStringConvertible conformance to maximum resolution (#56) Update input state when upload is finished (#63) Update input state when the upload succeeds or fails Tweak input state definition, keep an error in the failed case and the success struct in the success case NFC: remove some if let checks for the handler closure properties in MuxUpload and replace with ? operator. Keeps the existing logic. Replace video file get with actual implementation (#64) Reporting updates (#49) Only mark upload as started if its ready Add upload failed and input standardization events Add standardization failure event handler to reporter Add standardization success event handler to reporter Rename method for upload success Rename upload event to upload succeeded Support dispatching multiple events at the same time Naive non-thread safe implementation reporter networking Change event type casing, individual files for events Fix reporter test Actually report upload failure Read off file size Use Date in events and serialize to ISO8601 string Finangle duration and start time Set correct version in correct place Keep a session UUID Route every request through one chokepoint Upload failed test Shared test encoder Input standardization failed test Input standardization succeeded test Better organize expected json strings Use a shared instance Remove dupe file Thread asset duration through inspection and standardization Pass export error back Include non standard input reasons Workaround duration load pause Avoid mutating while iterating when UploadManager makes delegate calls Set custom HTTP request header for reporting (#67) Remove Equatable and Hashable conformance from MuxUpload (#68) Safe storage for MuxUpload (#71) Intended to prevent a crash if MuxUpload is extended by the SDK client to conform to Equatable or Hashable protocols
andrewjl-mux
added a commit
that referenced
this pull request
Jul 20, 2023
Upload Input Inspection and Standardization 1 (#17) Add scaffolding Add initializers Add internal and external state mapping Remove duplicate status enum and add inline docs to external status Add inline API docs to PHAsset-based MuxUpload constructor Consolidate all `MuxUpload` options into a single struct `UploadOptions` Declare asynchronous MuxUpload constructor in PHAsset extension Place extension methods into dedicated directories Polish inline API documentation Add new API documentation and note the placeholder implementation Add option variants as static members: defaults, disabled inputStandardization Deprecate existing initializer, normally this API should be removed prior to GA, but since it was the only initializer exposed up to this point removing it would break everybody. Instead deprecate and remove at a later date. Store all MuxUpload-related options in UploadInfo Use correct starting byte parameter when restarting upload Hardcode request options when exporting AVAsset from PhotosKit chunkSize -> chunkSizeInBytes Input standardization 2 (#41) Maintain handle to canonical input asset inside UploadInput If input standardized, standardized input URL is passed to UploadInfo instead of the original input URL used for initializer Note: SDK probably needs to re-export a high quality asset anyway so possibly need a bridging status Add dedicated internal initializer for MuxUpload error with unknown error code Disable input standardization when running tests Add inspection logic Request local and remote assets Status -> TransportStatus Rely on input status in MuxUpload computed property getters Expose progress from internal upload state if available Track transport status inside of UploadInput Transport status start time optional Add TransportStatus docs Inspect and check video track frame rate Migrate tests for MuxUpload input result handler Input standardization 3 (#46) Standardize via export session Back out outputURL construction into MuxUpload Expose hook for client to cancel upload if standardization failed Call cancellation hook if inspection fails. We're not sure if the input is standard or not so better to be safe and confirm Export based on maximum resolution set by client Cleaner non standard input handler invocation (#57) Add CustomStringConvertible conformance to maximum resolution (#56) Update input state when upload is finished (#63) Update input state when the upload succeeds or fails Tweak input state definition, keep an error in the failed case and the success struct in the success case NFC: remove some if let checks for the handler closure properties in MuxUpload and replace with ? operator. Keeps the existing logic. Replace video file get with actual implementation (#64) Reporting updates (#49) Only mark upload as started if its ready Add upload failed and input standardization events Add standardization failure event handler to reporter Add standardization success event handler to reporter Rename method for upload success Rename upload event to upload succeeded Support dispatching multiple events at the same time Naive non-thread safe implementation reporter networking Change event type casing, individual files for events Fix reporter test Actually report upload failure Read off file size Use Date in events and serialize to ISO8601 string Finangle duration and start time Set correct version in correct place Keep a session UUID Route every request through one chokepoint Upload failed test Shared test encoder Input standardization failed test Input standardization succeeded test Better organize expected json strings Use a shared instance Remove dupe file Thread asset duration through inspection and standardization Pass export error back Include non standard input reasons Workaround duration load pause Avoid mutating while iterating when UploadManager makes delegate calls Set custom HTTP request header for reporting (#67) Remove Equatable and Hashable conformance from MuxUpload (#68) Safe storage for MuxUpload (#71) Intended to prevent a crash if MuxUpload is extended by the SDK client to conform to Equatable or Hashable protocols
andrewjl-mux
added a commit
that referenced
this pull request
Jul 20, 2023
Upload Input Inspection and Standardization 1 (#17) Add scaffolding Add initializers Add internal and external state mapping Remove duplicate status enum and add inline docs to external status Add inline API docs to PHAsset-based MuxUpload constructor Consolidate all `MuxUpload` options into a single struct `UploadOptions` Declare asynchronous MuxUpload constructor in PHAsset extension Place extension methods into dedicated directories Polish inline API documentation Add new API documentation and note the placeholder implementation Add option variants as static members: defaults, disabled inputStandardization Deprecate existing initializer, normally this API should be removed prior to GA, but since it was the only initializer exposed up to this point removing it would break everybody. Instead deprecate and remove at a later date. Store all MuxUpload-related options in UploadInfo Use correct starting byte parameter when restarting upload Hardcode request options when exporting AVAsset from PhotosKit chunkSize -> chunkSizeInBytes Input standardization 2 (#41) Maintain handle to canonical input asset inside UploadInput If input standardized, standardized input URL is passed to UploadInfo instead of the original input URL used for initializer Note: SDK probably needs to re-export a high quality asset anyway so possibly need a bridging status Add dedicated internal initializer for MuxUpload error with unknown error code Disable input standardization when running tests Add inspection logic Request local and remote assets Status -> TransportStatus Rely on input status in MuxUpload computed property getters Expose progress from internal upload state if available Track transport status inside of UploadInput Transport status start time optional Add TransportStatus docs Inspect and check video track frame rate Migrate tests for MuxUpload input result handler Input standardization 3 (#46) Standardize via export session Back out outputURL construction into MuxUpload Expose hook for client to cancel upload if standardization failed Call cancellation hook if inspection fails. We're not sure if the input is standard or not so better to be safe and confirm Export based on maximum resolution set by client Cleaner non standard input handler invocation (#57) Add CustomStringConvertible conformance to maximum resolution (#56) Update input state when upload is finished (#63) Update input state when the upload succeeds or fails Tweak input state definition, keep an error in the failed case and the success struct in the success case NFC: remove some if let checks for the handler closure properties in MuxUpload and replace with ? operator. Keeps the existing logic. Replace video file get with actual implementation (#64) Reporting updates (#49) Only mark upload as started if its ready Add upload failed and input standardization events Add standardization failure event handler to reporter Add standardization success event handler to reporter Rename method for upload success Rename upload event to upload succeeded Support dispatching multiple events at the same time Naive non-thread safe implementation reporter networking Change event type casing, individual files for events Fix reporter test Actually report upload failure Read off file size Use Date in events and serialize to ISO8601 string Finangle duration and start time Set correct version in correct place Keep a session UUID Route every request through one chokepoint Upload failed test Shared test encoder Input standardization failed test Input standardization succeeded test Better organize expected json strings Use a shared instance Remove dupe file Thread asset duration through inspection and standardization Pass export error back Include non standard input reasons Workaround duration load pause Avoid mutating while iterating when UploadManager makes delegate calls Set custom HTTP request header for reporting (#67) Remove Equatable and Hashable conformance from MuxUpload (#68) Safe storage for MuxUpload (#71) Intended to prevent a crash if MuxUpload is extended by the SDK client to conform to Equatable or Hashable protocols
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.