Intent-to-Ship: MediaRecorder keyframe configurability

[Markus Handell]

Contact emails:

hand...@google.com

Explainer:

• https://github.com/w3c/mediacapture-record/pull/216

• crbug.com/1371449

TAG review:

N/A, small incremental change

Summary:

The `MediaRecorder` spec was recently updated to allow users to configure video keyframe generation parameters of the encoder. Previously there was no such support, and no spec requirements related to keyframe generation existed, leading to recorded streams that sometimes contain a single keyframe at the beginning of the stream. When played back those streams are inefficient to seek in, and it’s impossible to excise segments from them without resorting to transcoding. These problems present significant hurdles to use the MR API for some applications.

Link to “Intent to Prototype”

N/A, small incremental change and going directly for shipping.

Risks

Interoperability and Compatibility

This is now in the editors draft https://w3c.github.io/mediacapture-record/#mediarecorder-api

Gecko: No signal

WebKit: No signal

Web developers: No signals

The pull request adding this feature was reviewed and approved by the WebRTC WG containing representatives from Apple, Microsoft, and Mozilla. Feedback was addressed in the spec PR. But we have not reached out for comments on signals.

Will this feature be supported on all six Blink platforms (Windows, Mac, Linux, Chrome OS, Android, and Android WebView)?

Yes.

Is this feature fully tested by web-platform-tests?

Testability requires muxer parsers which MediaRecorder WPTs are currently lacking. https://github.com/web-platform-tests/wpt/issues/39642 has been filed to ensure WPTs are added when this is fixed.

Tracking bug

crbug.com/1371449

Link to entry on the Chrome Platform Status

https://chromestatus.com/feature/5117302963437568

[Henrik Boström]

I was present when the WG discussed this pull request and can confirm that representatives of Apple, Microsoft, Firefox and Google were present when the PR was merged.

This API solves problems relating to variable frame rate tracks and being able to ensure key frames in the streams. Allowing implementing this seems like a good thing as it plugs an existing hole, so to speak.

[Philip Jägenstedt]

Thanks for filing
https://github.com/web-platform-tests/wpt/issues/39642, I've asked
there for more details on what it would take to test this.

The spec change added two dictionary members to
https://w3c.github.io/mediacapture-record/#mediarecorderoptions-section
which means that it would at least be possible to add a test for
videoKeyFrameIntervalDuration and videoKeyFrameIntervalCount being
present. Can you write such a test?

Also, I think vendor signals here aren't needed given the reviews on
https://github.com/w3c/mediacapture-record/pull/216, but can you file
bugs for Gecko and WebKit?

> --
> 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/e104767c-a31e-4553-ad18-1a101e33d0een%40chromium.org.

[Yoav Weiss]

On Fri, Apr 21, 2023 at 6:48 PM 'Markus Handell' via blink-dev <blin...@chromium.org> wrote:

Contact emails:

hand...@google.com

Explainer:

• https://github.com/w3c/mediacapture-record/pull/216

• crbug.com/1371449

I think that a real explainer that outlines the API shape and how developers would be using it would really help here with reviewing this.

Developer facing documentation could be a good replacement for that, if y'all have any.

 

--
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/CAJjiFfHABg0DrRviiT5HXAidwynN5ti%2BTyE7Go4ZWUViQg-shg%40mail.gmail.com.

[Philip Jägenstedt]

On Wed, Apr 26, 2023 at 12:42 PM Yoav Weiss <yoav...@chromium.org> wrote:

On Fri, Apr 21, 2023 at 6:48 PM 'Markus Handell' via blink-dev <blin...@chromium.org> wrote:

Contact emails:

hand...@google.com

Explainer:

• https://github.com/w3c/mediacapture-record/pull/216

• crbug.com/1371449

I think that a real explainer that outlines the API shape and how developers would be using it would really help here with reviewing this.

I would suggest searching for "keyframeinterval" in https://w3c.github.io/mediacapture-record/, I found that to be enough to understand the API surface and what it does.

[Yoav Weiss]

https://w3c.github.io/mediacapture-record/#mediarecorderoptions-section indeed helps clarify what this is.  

[Markus Handell]

Thanks for filing
https://github.com/web-platform-tests/wpt/issues/39642, I've asked
there for more details on what it would take to test this.

I added some more detail there.

 

The spec change added two dictionary members to
https://w3c.github.io/mediacapture-record/#mediarecorderoptions-section
which means that it would at least be possible to add a test for
videoKeyFrameIntervalDuration and videoKeyFrameIntervalCount being
present. Can you write such a test?

Yes!

 

Also, I think vendor signals here aren't needed given the reviews on
https://github.com/w3c/mediacapture-record/pull/216, but can you file
bugs for Gecko and WebKit?

Bugs filed:

https://bugs.webkit.org/show_bug.cgi?id=256031

https://bugzilla.mozilla.org/show_bug.cgi?id=1830236

[Philip Jägenstedt]

On Thu, Apr 27, 2023 at 9:24 AM Markus Handell <hand...@google.com> wrote:

Thanks for filing
https://github.com/web-platform-tests/wpt/issues/39642, I've asked
there for more details on what it would take to test this.

I added some more detail there.

 

The spec change added two dictionary members to
https://w3c.github.io/mediacapture-record/#mediarecorderoptions-section
which means that it would at least be possible to add a test for
videoKeyFrameIntervalDuration and videoKeyFrameIntervalCount being
present. Can you write such a test?

Yes!

Thanks! It won't test the behavior, but will at least let the tests show if there's some support or not, and catch typos in IDL, which happens.

 

Also, I think vendor signals here aren't needed given the reviews on
https://github.com/w3c/mediacapture-record/pull/216, but can you file
bugs for Gecko and WebKit?

Bugs filed:

https://bugs.webkit.org/show_bug.cgi?id=256031

https://bugzilla.mozilla.org/show_bug.cgi?id=1830236

Thank you!

Skimming https://w3c.github.io/mediacapture-record/#mediarecorderoptions-section I have just one more question about interop here. The spec says "If both videoKeyFrameIntervalDuration and videoKeyFrameIntervalCount are not null, the UA may ignore either." It seems to me an implementation will definitely pick one of them to ignore and it will be deterministic. Is there any reason to leave this up to the UA instead of writing in the spec which takes precedent?

Best regards,

Philip

[Markus Handell]

Yep this topic was discussed on https://github.com/w3c/mediacapture-record/pull/216#discussion_r1159521616 and earlier. In the end I backed out of my proposed semantics due to scepticism so the PR leaves it unspecified.

During the interim that hbos@ speaks to where the PR was merged, it was decided that a separate spec issue be filed on that topic, https://github.com/w3c/mediacapture-record/issues/219.

[Philip Jägenstedt]

Great, with https://github.com/w3c/mediacapture-record/issues/219 filed and the discussion you linked to, I don't think this needs to block.

LGTM1 to ship given a surface-level test in WPT as discussed.

[Chris Harrelson]

LGTM2

--

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/CAARdPYf5CAXSME%2B4nmBT%2B2upB-2-hGh3MMJUn8j1M5%3DV4EYLxQ%40mail.gmail.com.

[Yoav Weiss]

LGTM3

[Markus Handell]

Thanks all! 

[Peter Kasting]

Sorry to necro this thread, but I came across the impl and had a quick question about desired behavior, since it looks buggy to me.

The spec says that both the duration and frame count refer to the interval "between key frames". The Blink implementation currently requests a key frame if the elapsed time is >= the requested duration, or the frame count is > the requested count. Note the discrepancy between ">=" and ">" there; I think this is a bug and the latter should also be ">=", i.e. if an author requests an interval of "24 frames", that means a key frame every 24th frame, not a key frame followed by 24 delta frames and then another key frame. However, the current behavior is tested in the unittest, so I want to be sure before I go changing things.

(N.B. I have not examined other browsers' current behavior.)

PK

[Peter Kasting]

On Thursday, May 30, 2024 at 9:32:50 AM UTC-7 Peter Kasting wrote:

The spec says that both the duration and frame count refer to the interval "between key frames".

More spec text: "If videoKeyFrameIntervalCount is not null and videoKeyFrameIntervalDuration is null, the video encoder produces a keyframe on the first frame arriving after videoKeyFrameIntervalCount frames passed since the last key frame." I think that means Blink's current impl is indeed off-by-one.

PK 

[Markus Handell]

Hi Peter, from the spec text:

"If videoKeyFrameIntervalCount is not null ... the video encoder produces a keyframe on the first frame arriving after videoKeyFrameIntervalCount frames passed since the last key frame"

It sounds to me like the blink impl is correctly implementing the spec meaning. If videoKeyFrameIntervalCount is 0, the spec text implies every frame should be a key frame. If your alternative interpretation were true, what would a value of 0 mean?

[Peter Kasting]

On Fri, May 31, 2024, 5:43 AM Markus Handell <hand...@google.com> wrote:

Hi Peter, from the spec text:

"If videoKeyFrameIntervalCount is not null ... the video encoder produces a keyframe on the first frame arriving after videoKeyFrameIntervalCount frames passed since the last key frame"

It sounds to me like the blink impl is correctly implementing the spec meaning. If videoKeyFrameIntervalCount is 0, the spec text implies every frame should be a key frame. If your alternative interpretation were true, what would a value of 0 mean?

A value of 0 would in practice have the same effect as a value of 1, both meaning, "key frame every frame". Though also in practice I don't think either actually works, given that the blink impl API only requests key frames in response to receiving delta frames.

If "first frame after" is intended to imply "exclusive of", then the count part is correct but the time part is wrong (it would need to switch to > from >=). I think the intent of the wording was to imply "inclusive of", though. But I wasn't in the meetings, so I'm not sure. 

PK

[Markus Handell]

It was a while, but the discussions we had were not revolving around this corner case.

I think it makes sense for the implementation to keep adhering to the "exclusive of" to get rid of the ambiguity of what 0 means in Interval. I also don't see another way to interpret the spec text.

For the duration I don't think it practically matters whether >= or > is used as we're talking microseconds. If you think this is important, please file a MediaRecorder bug.

Thanks!

Markus

[Peter Kasting]

0 is actually the one unambiguous case -- it clearly means you want a key frame every frame. 

Seems strange that someone whose steam has a key frame every 30 frames should specify a count of 29. I dunno though, if that's also what other vendors do then changing is far more trouble than benefit. Do we know what other vendors do? Is there any kind of wpt test for this, or similar?

PK

[Markus Handell]

WPT addition is covered under https://github.com/web-platform-tests/wpt/issues/39642 and I haven't been able to prioritize that yet.

I don't know what other vendors do. You're welcome to file a bug in the mediarecorder - not sure I think the issue raised here is terribly important but I could support the implementation doing > instead of >= when it comes to duration.

[Peter Kasting]

On Mon, Jun 3, 2024 at 2:29 AM Markus Handell <hand...@google.com> wrote:

You're welcome to file a bug in the mediarecorder - not sure I think the issue raised here is terribly important but I could support the implementation doing > instead of >= when it comes to duration.

I filed https://issues.chromium.org/346406089 as P3. The only reason it might be higher priority is that, while trivial, fixing this sooner rather than later is less likely to run into problems with Hyrum's law.

PK