LGTM3 On Mon, Dec 20, 2021 at 9:02 PM 'Chris Cunningham' via blink-dev < blink-dev@chromium.org> wrote:
> > Thanks Mike, > > > LGTM1 if we can change the deprecation message to "is deprecated". > > CL is out for review > <https://chromium-review.googlesource.com/c/chromium/src/+/3350790>. All > of the OWNERS are OOO until the new year, but 98 doesn't promote to Beta > until Jan 6... could still work out. > > Best, > Chris > > On Monday, December 20, 2021 at 11:42:34 AM UTC-8 Chris Harrelson wrote: > >> LGTM2 >> >> On Mon, Dec 20, 2021, 6:41 AM Mike Taylor <mike...@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 <mike...@chromium.org> >>> wrote: >>> >>>> Hi Chris, >>>> >>>> On 12/17/21 3:24 PM, Chris Cunningham wrote: >>>> >>>> Contact emails >>>> >>>> >>>> * chcunn...@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+...@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+...@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/55e14552-4d34-49d6-9dd1-8739b73ad151n%40chromium.org > <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/55e14552-4d34-49d6-9dd1-8739b73ad151n%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/CAL5BFfUddpxLZwdc7zXyOzzpeeroO8v88DON1_q7QRc5u9d%3D4A%40mail.gmail.com.
