Skip to content
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

api: convert standardization enabled to standardization requested #77

Merged
merged 5 commits into from
Jul 20, 2023

Conversation

andrewjl-mux
Copy link
Contributor

@andrewjl-mux andrewjl-mux commented Jul 20, 2023

Referring to the option as standardization requested implies a more accurate mental model for how input standardization actually works, it is not necessarily performed if requested, only if the SDK deems necessary. Therefore using enabled may be misleading.

The convenience API for preventing the SDK from performing any input standardization has been respelled as skipped.

Reporting event keys have also been updated to reflect the new spelling.

Thanks to @daytime-em for coming up with this!

A more accurate nomenclature for how input standardization actually
works, it is not necessarily performed if requested, only if the SDK
deems necessary
@andrewjl-mux andrewjl-mux requested a review from a team July 20, 2023 23:27
Copy link
Collaborator

@daytime-em daytime-em left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrewjl-mux andrewjl-mux merged commit 29fe7cd into releases/v0.4.0 Jul 20, 2023
@andrewjl-mux andrewjl-mux deleted the ajlb/standardization-requested branch July 20, 2023 23:44
@github-actions github-actions bot mentioned this pull request Jul 20, 2023
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants