LGTM2 On Mon, Dec 20, 2021, 6:41 AM Mike Taylor <miketa...@chromium.org> wrote:
> On 12/19/21 7:03 PM, Chris Cunningham wrote: > > Hi Mike, > > > And the proposed change here is to remove temporalLayderId as a > top-level key on EncodedVideoChunkMetadata, right? > > That's right. > > > The proposed change here is to start throwing without a timestamp key in > the VideoFrameInit dictionary, for all "image" types except VideoFrame and > HTMLVideoElement, correct? > > That's also right. > > > Can you clarify the timing of the proposed removal? Do you intend to > send deprecation messages in M99, and if so, for how long? Or do you intend > to deprecate and remove all at once in M99? > > My ideal timing would be to remove in 99. We've just landed a flag ( > --enable-features=RemoveWebCodecsSpecViolations > <https://chromium-review.googlesource.com/c/chromium/src/+/3347023>) to > simulate the removal, which I should be able to merge back to 98. We landed > a "may deprecate" message for the VideoFrame constructor in 97. I could > merge a change to 98 that hardens language to "is deprecated". I'm not sure > we can add a message for the metadata.temporalLayerId deprecation since > it's just an output dictionary member. > > Happy to be flexible if this timeline is problematic. At this point I > think the usage of the bad paths is actually near zero, so a faster > timeline has advantages too. > > Given that usage is around .00015% right now, I agree that moving faster > on this change is probably smart. *LGTM1* if we can change the > deprecation message to "is deprecated". > > Merging back the flag back to M98 seems useful if we can make developers > aware it exists, perhaps by updating https://web.dev/webcodecs/ with an > "update" blurb up to mentioning the changes and the flag? > > (Before I hit send, I went and searched for `temporalLayerId` in the > httparchive.latest.requests_desktop dataset and got zero results - that > makes me feel better about hitting send). > > Best, > Chris > > > > On Sat, Dec 18, 2021 at 12:30 PM Mike Taylor <miketa...@chromium.org> > wrote: > >> Hi Chris, >> >> On 12/17/21 3:24 PM, Chris Cunningham wrote: >> >> Contact emails >> >> >> * chcunning...@chromium.org <chcunning...@chromium.org> * Explainer >> >> >> * https://github.com/w3c/webcodecs/blob/main/explainer.md >> <https://github.com/w3c/webcodecs/blob/main/explainer.md> * >> Specification >> >> >> * https://w3c.github.io/webcodecs/ <https://w3c.github.io/webcodecs/> * >> Summary >> >> >> * We've identified two areas where our implementation violates the >> specification. We've implemented parallel correct paths for authors to use >> and would like to deprecate the original bad paths. The issues affect >> VideoFrame construction and the EncodedVideoChunkMetadata dictionary. * Blink >> component >> >> >> * Blink>Media>WebCodecs >> <https://bugs.chromium.org/p/chromium/issues/list?q=component:Blink%3EMedia%3EWebCodecs> >> * Motivation >> >> >> >> * We've identified two areas where our implementation of WebCodecs >> violates the specification. We've considered changing the spec, but prefer >> to instead fix the implementation. The specified behavior is cleaner and >> less error prone. The changes are breaking, but the workarounds are trivial >> and WebCodecs usage is currently very low (we just shipped in Chrome 94, >> only engine to ship so far). >> https://chromestatus.com/metrics/feature/timeline/popularity/3464 >> <https://chromestatus.com/metrics/feature/timeline/popularity/3464> >> Details: 1. The spec defines the temporalLayerId attribute as a member of >> the SvcOutputMetadata dictionary which is nested under the >> EncodedVideoChunkMetadata dictionary (metadata.svc.temporalLayerId). But >> Chrome places the temporalLayerId directly on the top level >> EncodedVideoChunkMetadata dictionary (metadata.temporalLayerId). As of >> Chrome 98, either option is available. * >> >> And the proposed change here is to remove temporalLayderId as a top-level >> key on EncodedVideoChunkMetadata, right? >> >> * 2. The spec requires that the VideoFrame(CanvasImageSource, ...) >> constructor include a timestamp argument (VideoFrameInit.timestamp) for >> CanvasImageSource types that don't implicitly have a timestamp (e.g. >> HTMLCanvasElement). Failing to include the timestamp should result in a >> TypeError, but Chrome currently defaults the timestamp to zero. Chrome will >> respect the timestamp if one is given. * >> >> The proposed change here is to start throwing without a timestamp key in >> the VideoFrameInit dictionary, for all "image" types except VideoFrame and >> HTMLVideoElement, correct? >> >> Initial public proposal >> >> >> * >> https://groups.google.com/a/chromium.org/g/blink-dev/c/7UlTzFMbTFs/m/Rib4ca4-BQAJ >> <https://groups.google.com/a/chromium.org/g/blink-dev/c/7UlTzFMbTFs/m/Rib4ca4-BQAJ> >> * TAG review >> >> >> * https://github.com/w3ctag/design-reviews/issues/612 >> <https://github.com/w3ctag/design-reviews/issues/612> * TAG review status >> >> >> * Complete * Risks >> Site breakage >> >> >> >> >> * Both changes can break sites. For temporalLayerId, we're not able to >> add metrics for it's usage (dictionary member), but we have a reasonable >> sense for which sites may be affected and will reach out directly. For the >> VideoFrame constructor, we added UKM metrics to count usage of the bad path >> and a "may deprecate" warning. These metrics landed in M97 (beta). So far, >> no usage of the bad path. * Interoperability and Compatibility >> >> >> >> >> * Gecko: Supportive. Paul Adenot approved the PRs that defined the >> specified behavior. We discussed changing the behavior of the VideoFrame >> constructor but both prefer to fix the implementation if that can be done >> without huge developer pain. WebKit: No signal Web developers: No >> signals. * Debuggability >> >> >> * Fixing the VideoFrame constructor may reduce the need for author >> debugging. The current defaulting behavior (timestamp = 0) may at first >> seem helpful, but is problematic if you then send the VideoFrame to a >> VideoEncoder, where timestamps are used to guide bitrate control. * Is >> this feature fully tested by web-platform-tests >> <https://chromium.googlesource.com/chromium/src/+/master/docs/testing/web_platform_tests.md> >> ? >> >> >> * Yes. https://github.com/web-platform-tests/wpt/tree/master/webcodecs >> <https://github.com/web-platform-tests/wpt/tree/master/webcodecs> * Flag >> name >> >> >> * None yet. We'll implement a flag and announce in a follow up "Ready for >> Trial" thread. * Requires code in //chrome? >> >> >> * False * Estimated milestones >> >> * 99 * >> >> Can you clarify the timing of the proposed removal? Do you intend to send >> deprecation messages in M99, and if so, for how long? Or do you intend to >> deprecate and remove all at once in M99? >> >> >> Link to entry on the Chrome Platform Status >> >> https://chromestatus.com/feature/5667793157488640 >> >> -- >> You received this message because you are subscribed to the Google Groups >> "blink-dev" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to blink-dev+unsubscr...@chromium.org. >> To view this discussion on the web visit >> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CALG6eSpjBUfdEUsQk0ekp9W1dAZHJNoeEFL8tDBR9PR%3DZhbjMQ%40mail.gmail.com >> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CALG6eSpjBUfdEUsQk0ekp9W1dAZHJNoeEFL8tDBR9PR%3DZhbjMQ%40mail.gmail.com?utm_medium=email&utm_source=footer> >> . >> >> >> > -- > You received this message because you are subscribed to the Google Groups > "blink-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to blink-dev+unsubscr...@chromium.org. > To view this discussion on the web visit > https://groups.google.com/a/chromium.org/d/msgid/blink-dev/54b7584d-edb1-d92d-4a84-b499593a1710%40chromium.org > <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/54b7584d-edb1-d92d-4a84-b499593a1710%40chromium.org?utm_medium=email&utm_source=footer> > . > -- You received this message because you are subscribed to the Google Groups "blink-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscr...@chromium.org. To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAOMQ%2Bw9H6bNvtexHGxuKqdWsHgjv-A%3DnFFPq4knsdawDYG1JLw%40mail.gmail.com.
