-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MuxUpload storage indirection #71
Merged
andrewjl-mux
merged 1 commit into
project/input-standardization
from
ajlb/muxupload-storage-indirection
Jul 20, 2023
Merged
MuxUpload storage indirection #71
andrewjl-mux
merged 1 commit into
project/input-standardization
from
ajlb/muxupload-storage-indirection
Jul 20, 2023
Conversation
This file contains 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
Intended to prevent a crash if MuxUpload is extended by the SDK client to conform to Equatable or Hashable protocols
cjpillsbury
approved these changes
Jul 20, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@@ -27,8 +27,24 @@ import Foundation | |||
/// ``` | |||
/// | |||
public final class UploadManager { | |||
|
|||
private struct UploadStorage: Equatable, Hashable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
praise: 😎
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
andrewjl-mux
added a commit
that referenced
this pull request
Jul 21, 2023
* Project Input standardization (#17) (#41) (#46) (#48) (#57) (#77) Add AVFoundation and PhotosKit 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 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 Request local and remote assets Standardize via AVFoundation asset export session 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 Add CustomStringConvertible conformance to maximum resolution (#56) Only mark upload as started if its ready 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 Switch order of operations to avoid long pause on fetching duration AVAsset sometimes hangs when asked to asynchronously fetch duration when there aren't audio or video tracks present. To avoid this after starting the upload, the inspection step will get the video tracks first and get the duration afterwards. --------- Co-authored-by: Emily Dixon <[email protected]> * Minor example app renaming (#29) * Use a UUID string as MuxUpload internal identifier (#30) * Display a more specific error message when the direct upload POST request fails (#32) * Use MuxUpload id instead if the input URL when looking up or writing state in the SDK (#33) * Change upload creation example app method to use discardableResult (#34) * Add dedicated internal initializer for MuxUpload error with unknown error code (#35) * Rename enum and adjust to camel casing (#36) * Adhere to Swift formatting guidelines, remove snake casing (#37) * Fix potential crash in ChunkedFile (#38) * Include Cloud shared asset sources when requesting assets (#40) * Prevent arithmentic overflow when setting chunk content range value (#45) * Remove force unwrap that can cause a crash (#47) * Make internal class methods internal (#51)
tomkordic
pushed a commit
that referenced
this pull request
Mar 1, 2024
* Project Input standardization (#17) (#41) (#46) (#48) (#57) (#77) Add AVFoundation and PhotosKit 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 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 Request local and remote assets Standardize via AVFoundation asset export session 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 Add CustomStringConvertible conformance to maximum resolution (#56) Only mark upload as started if its ready 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 Switch order of operations to avoid long pause on fetching duration AVAsset sometimes hangs when asked to asynchronously fetch duration when there aren't audio or video tracks present. To avoid this after starting the upload, the inspection step will get the video tracks first and get the duration afterwards. --------- Co-authored-by: Emily Dixon <[email protected]> * Minor example app renaming (#29) * Use a UUID string as MuxUpload internal identifier (#30) * Display a more specific error message when the direct upload POST request fails (#32) * Use MuxUpload id instead if the input URL when looking up or writing state in the SDK (#33) * Change upload creation example app method to use discardableResult (#34) * Add dedicated internal initializer for MuxUpload error with unknown error code (#35) * Rename enum and adjust to camel casing (#36) * Adhere to Swift formatting guidelines, remove snake casing (#37) * Fix potential crash in ChunkedFile (#38) * Include Cloud shared asset sources when requesting assets (#40) * Prevent arithmentic overflow when setting chunk content range value (#45) * Remove force unwrap that can cause a crash (#47) * Make internal class methods internal (#51)
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.
Intended to prevent a crash if MuxUpload is extended by the SDK client to conform to Equatable or Hashable protocols.
MuxUpload was originally intended to be a facade-like type, so longer term this solution should be replaced in favor of not handling MuxUpload directly.