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

Have appendBuffer and remove return promise. #100

Open
jyavenard opened this issue Jun 17, 2016 · 9 comments
Open

Have appendBuffer and remove return promise. #100

jyavenard opened this issue Jun 17, 2016 · 9 comments
Labels
TPAC-2022-discussion Marked for discussion at TPAC 2022 Media WG meeting Sep 16
Milestone

Comments

@jyavenard
Copy link
Member

I realise this is late in the lifecycle ; but I believe this would be a great improvement to the current spec and be fully backward compatible with the existing spec.

SourceBuffer::appendBuffer and SourceBuffer::remove should now return a promise.

Such promise will be resolved when either of those calls will complete.
Promise will be rejected with the error code whenever an error occurred and optionally extra details explaining the error (this would make troubleshooting much easier, like explaining what was wrong in the data being added).

I intend to provide further implementation details in a following post.

However, the general guideline would be that whenever in the current text we read:
"Queue a task to fire a simple event named update at this SourceBuffer object.
Queue a task to fire a simple event named updateend at this SourceBuffer object."

would now read:
"Queue a task to fire a simple event named update at this SourceBuffer object.
Queue a task to fire a simple event named updateend at this SourceBuffer object.
Resolve the current pending promise"

where we read:
"Queue a task to fire a simple event named abort at sourceBuffer.
Queue a task to fire a simple event named updateend at sourceBuffer."
would now read:
"Queue a task to fire a simple event named abort at sourceBuffer.
Queue a task to fire a simple event named updateend at sourceBuffer.
Reject the current pending promise with MEDIA_ERR_ABORTED"

where we read:
"Queue a task to fire a simple event named error at this SourceBuffer object.
Queue a task to fire a simple event named updateend at this SourceBuffer object."
would now read:
Queue a task to fire a simple event named error at this SourceBuffer object.
Queue a task to fire a simple event named updateend at this SourceBuffer object.
Reject the current pending promise with decode error"

the decode error could be augmented such as it contains the actual error code that originally caused the error.

Where in appendBuffer and remove we read:
"If the updating attribute equals true, then throw an InvalidStateError exception and abort these steps."

would now read:
"If the updating attribute equals true, then throw an InvalidStateError exception, return a rejected promise with InvalidStateError error and abort these steps."

Those changes are fully backward compatible with existing implementation and use, but it will make things much simpler to use for the clients than having to deal with the various "update*" events being fired.

Firefox/Gecko can implement those changes very quickly (a few days)

@jyavenard
Copy link
Member Author

gecko related issue will be tracked in https://bugzilla.mozilla.org/show_bug.cgi?id=1280613

@wolenetz wolenetz added this to the VNext milestone Jun 17, 2016
@wolenetz
Copy link
Member

I agree promise support would be a good thing for MSE, but this is a substantive enough spec change that would necessitate it be in VNext. It's something that was considered when EME was adding promise support a while back, and perhaps should have been done then for MSE. We've closed the substantive scope for MSE v1. Thanks for filing this -- it's something definitely we'll consider early in VNext.

@silviapfeiffer
Copy link
Member

The more you delay it, the harder it gets to add, and since it's fully backwards compatible, it seems not that hard to add.

@paulbrucecotton
Copy link

I want to suggest that if the community wants to extend the MSE Recommendation with support for ServiceWorkers and/or change to use Promises that these kind of proposals be incubated in the WICG.

@jyavenard
Copy link
Member Author

Upon investigating further, it would be difficult to have something 100% backward compatible with the existing spec.
currently appendBuffer() / remove() are defined to throw if an error condition is detected.
However, when using promises, the typical use is to return a rejected promise with the error code instead. Including if the arguments type is invalid.

Returning a promise and throwing would be different to any w3c specs making use of promises.

So a way forward would be to accept that appendBuffer/Remove continue to throw as now, or defined new methods, such as appendBufferAsync / RemoveAsync.
Likely not the best name as appendBuffer is already partially asynchronous.

@wolenetz wolenetz removed this from the VNext milestone Jun 9, 2020
@wolenetz wolenetz added this to the V2 milestone Sep 21, 2020
@wolenetz wolenetz added the agenda Topic should be discussed in a group call label Sep 21, 2020
@wolenetz
Copy link
Member

This seems like a good modernization to include in scope of V2 to me.

@mwatson2 mwatson2 removed the agenda Topic should be discussed in a group call label Sep 21, 2020
@wolenetz wolenetz added the TPAC-2022-discussion Marked for discussion at TPAC 2022 Media WG meeting Sep 16 label Sep 16, 2022
@joeyparrish
Copy link
Member

I am enthusiastic about Promise-based methods for MSE. I'd like to clarify some details, though.

appendBuffer() and remove() are both asynchronous today and event-based, but there are other places where Promises would be useful. For example, you can't do even synchronous operations while the SourceBuffers are busy. Setting mediaSource.duration requires you to wait for all SourceBuffers to be idle. A Promise-based method to set duration could encompass waiting for idle, too.

Another issue with setting duration is that if you reduce the duration, it can implicitly trigger the removal algorithm, which then results in an 'updateend' event that you must listen for to adequately track the idle state of your SourceBuffers. Having a Promise-based setter would help with this, too.

So I'd like to plan out the full list of what could/should be Promise-based, because I believe it would be useful to add Promises to everything that could possibly generate an 'updateend' or that requires an idle state today.

@dalecurtis
Copy link

I finally got annoyed enough at this to write a polyfill / playground for what a modern promise based API could look like:
https://github.com/dalecurtis/promised-media-source

As @joeyparrish notes, I think it's not quite as simple as just making append/remove async and providing a ready promise. I think it'd be a pretty large undertaking to promisify everything that we probably should.

@wolenetz
Copy link
Member

@joeyparrish :

Another issue with setting duration is that if you reduce the duration, it can implicitly trigger the removal algorithm, which then results in an 'updateend' event that you must listen for to adequately track the idle state of your SourceBuffers. Having a Promise-based setter would help with this, too.

Though Chrome may still allow duration reductions to trigger async removal if truncating buffered media, that is not the REC MSE behavior and it has deprecation message and metrics tracking how often this occurs. Hopefully this particular non-compliant support will be fully deprecated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TPAC-2022-discussion Marked for discussion at TPAC 2022 Media WG meeting Sep 16
Projects
None yet
Development

No branches or pull requests

7 participants