w3c / media-source

Media Source Extensions
https://w3c.github.io/media-source/
Other
268 stars 57 forks source link

Have appendBuffer and remove return promise. #100

Open jyavenard opened 8 years ago

jyavenard commented 8 years ago

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 commented 8 years ago

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

wolenetz commented 8 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 6 years ago

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 commented 3 years ago

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

joeyparrish commented 1 year ago

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 commented 1 year ago

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 commented 10 months ago

@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.