w3c / webcodecs

WebCodecs is a flexible web API for encoding and decoding audio and video.
https://w3c.github.io/webcodecs/
Other
978 stars 136 forks source link

Remove ImageTrack.onchange event. #497

Closed dalecurtis closed 2 years ago

dalecurtis commented 2 years ago

We decided not to do this, but forgot to remove it from the spec. The fact that decode() calls stall until the next frame is available and the availability of the completed promise make it largely unnecessary.

Clients which don't want to wait for for completed can instead handle a RangeError exception during decode() if no more frames remain. While not ideal it's inefficient to implement something like onchange since common formats like GIF would require us to attempt decode in order to update the frame count.


:boom: Error: 400 Bad Request :boom:

PR Preview failed to build. (Last tried on Jun 6, 2022, 11:07 PM UTC).

More PR Preview relies on a number of web services to run. There seems to be an issue with the following one: :rotating_light: [CSS Spec Preprocessor](https://api.csswg.org/bikeshed/) - CSS Spec Preprocessor is the web service used to build Bikeshed specs. :link: [Related URL](https://api.csswg.org/bikeshed/?url=https%3A%2F%2Fraw.githubusercontent.com%2Fw3c%2Fwebcodecs%2F99fe3034e5feed0e7708510fe96670d5112093f7%2Findex.src.html&md-warning=not%20ready) ``` Error running preprocessor, returned code: 2. WARNING: IGNORED LEGACY IDL LINE: 5 - "," WARNING: IGNORED LEGACY IDL LINE: 4 - "," WARNING: IGNORED LEGACY IDL LINE: 22 - "," WARNING: IGNORED LEGACY IDL LINE: 10 - "," WARNING: IGNORED LEGACY IDL LINE: 20 - "," WARNING: IGNORED LEGACY IDL LINE: 6 - "," FATAL ERROR: Obsolete biblio ref: [rfc7231] is replaced by [rfc9110]. Either update the reference, or use [rfc7231 obsolete] if this is an intentionally-obsolete reference. ✘ Did not generate, due to fatal errors ``` _If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please [file an issue](https://github.com/tobie/pr-preview/issues/new?title=Error%20not%20surfaced%20properly&body=See%20w3c/webcodecs%23497.)._
dalecurtis commented 2 years ago

Since Chris is OOO, requesting review from Paul and Bernard. Thanks for taking a look!

dalecurtis commented 2 years ago

Actually I misremembered the context here. GIF doesn't require us to decode, just scan the byte stream -- which Chromium implements today on any new bytes received. So while this event is unimplemented in Chromium, it is possible to do efficiently (at least for the common formats I know of).

So we can leave this in for folks who don't want to catch a RangeError or wait for completed. However, it's pretty unwieldy, so I'm not sure why someone would want to: The decode loop would be to wait for ImageTrackList.ready then subscribe to ImageTrack.onchange and decode up until you reach whatever frameCount value you cached during the event.

As such, I'm still inclined to remove it since the RangeError or completed property is way easier to handle.

dalecurtis commented 2 years ago

Bump @padenot @aboba for thoughts on this.

chcunningham commented 2 years ago

I'm worried about the use case of rendering a streaming gif. Say you fetch a gif and you're able to decode the first 10 frames until you see a RangeError. But say there are 100 frames after that point still being downloaded. You can wait until "completed", but maybe you'd like to render or decode incrementally while the download continues. With the event, you'll know that frameCount grew and get a trigger to decode more. Without the event I think you have to resort to setTimeout polling. Sound right?

dalecurtis commented 2 years ago

No, just calling decode({frameIndex: 11}) will stall pending arrival of the data for that frame. It won't throw a RangeError until completed (assuming we only got 10 frames).

chcunningham commented 2 years ago

Ah, right! LGTM

chcunningham commented 2 years ago

@padenot FYI - doing TBR merge while you're OOO.