-
Notifications
You must be signed in to change notification settings - Fork 24
Deprecate MediaRecorder.isTypeSupported() by limiting it to fixed list of identifiers w/wildcards #226
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Pehrsons
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
|
@youennf does this LGTY without "mp4a"? Full disclosure https://jsfiddle.net/jib1/sw1mu6Lv/ in Safari produces: |
|
I suppose for browsers with only one audio choice (like Safari and Firefox), this might matter less since you get the only audio codec available by default. |
|
FWIW, Chrome (on Windows) produces the following output with the same script: |
|
Some thoughts:
|
|
It might be good to check whether we need |
|
We should try to only include |
|
cc @handellm |
|
With regards to |
|
Safari is also supporting vp08 and vp09, but not Chrome. I'll check internally whether we can remove those checks in Safari. |
|
Good news is this updated fiddle shows browsers for the most part ignore profile levels. I.e. both Chrome (mostly) and Safari accept anything after the period. E.g.: "video/webm;codecs=av01",
"video/webm;codecs=av01.",
"video/webm;codecs=av01.*",
"video/webm;codecs=av01,opus",
"video/webm;codecs=av01,opus.",
"video/webm;codecs=av01,opus.*",
...Safari is even more flexible, not even requiring the period (except for hvc1 and hev1). E.g.: "video/mp4;codecs=avc1*",
"video/mp4;codecs=avc1,mp4a*",
"video/mp4;codecs=hvc1.",
"video/mp4;codecs=hvc1.*",
"video/mp4;codecs=hev1.",
"video/mp4;codecs=hev1.*"But there are notable exceptions in Chrome (note lack of wildcard): "video/mp4",
"video/mp4;codecs=vp9",
"video/mp4;codecs=vp9,opus",
"video/mp4;codecs=vp9,mp4a.40.2",
"video/mp4;codecs=av01",
"video/mp4;codecs=av01.0.19M.08",
"video/mp4;codecs=av01,opus",
"video/mp4;codecs=av01,mp4a.40.2",
"video/mp4;codecs=avc1",
"video/mp4;codecs=avc1.64003E",
"video/mp4;codecs=avc1,opus",
"video/mp4;codecs=avc1,mp4a.40.2",
"video/mp4;codecs=avc3",
"video/mp4;codecs=avc3.64003E",
"video/mp4;codecs=avc3,opus",
"video/mp4;codecs=avc3,mp4a.40.2",
"video/mp4;codecs=hvc1.1.6.L186.B0",
"video/mp4;codecs=hev1.1.6.L186.B0",But updating these to a wildcard should be largely backwards compatible (insofar as it won't break any recording that used to work — and there doesn't appear to be significant choice between profiles here yet) So we should be able to standardize on the wildcard version (with or without a period — slight preference for requiring the period). |
| while recording, the User Agent has to be prepared to re-encode it to | ||
| avoid interruptions. | ||
| </div> | ||
| <li>If the User Agent does not support the specified combination of media |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In our opinion this is a good addition. However the current Chromium implementation will actually synch-check for support already in the ctor. Can we allow "either a ctor exception, or an event on start"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do. This is covered by is type supported called from the constructor with deferNewerCodecsCheck = true:¹
- If type contains a standalone or comma-separated codec specifier string that in its entirety is not a case-insensitive exact match for any item in the list of synchronously exposed codec specifiers and deferNewerCodecsCheck is true, then return true.
- If the MediaRecorder does not support the specified combination of media type/subtype, codecs and container then return false.
So the rules are:
- Only old codecs → throws in constructor
- any new codec mentioned → event on start instead
1. Note I'm in the process of updating this text to allow for wildcard, so this was the text at time of writing this, ahead of that change, for reference. I'll preserve its behaviors relevant here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean I think this spec text says "is type supported" with codecs=h264.bla from the ctor will return true in step 8, whereas currently the Chromium ctor will throw an exception instead. Allowing either exception in the ctor or error event on start() seems reasonable and allows for relaxed rules on UA implementation. Maybe 8 + 9 could be merged in the spec text somehow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes the "exact match" language in step 8 was from a simpler era (when the sync list was quite small). This should be better now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Allowing either exception in the ctor or error event on start() seems reasonable and allows for relaxed rules on UA implementation
Unless we're willing to commit to changing shipped behavior, I don't think we should entertain variance across implementations. Concerns over compat across versions mirror those across implementations.
It might hurt web compat if some browsers throw and others fire an event — in a way that some codecs throw and others fire an event, would not, provided it was consistent across browsers.
The latest text mandates forever throwing in the ctor for codec descriptors on the list (which has now grown quite a bit). This perhaps hurts the original premise that we wanted to get away from sync detection. But if shipped is shippped, then I don't see a good way here to have it both ways.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This perhaps hurts the original premise that we wanted to get away from sync detection. But if shipped is shippped, then I don't see a good way here to have it both ways.
But I think the text now in |is type supported| ignores the part after leading dot, so Chromium's ctor behavior would not be compliant (since it can return true or false depending on the string after first dot)?
The isTypeSupported algorithm seems acceptable as it stands now from Chromium. We will need to do changes (allow wildcarding for the hevc types) but I think these changes are for the better.
MediaRecorder.bs
Outdated
| </li> | ||
| <li>If the MediaRecorder does not support the specified combination of media | ||
| type/subtype, and container then return false.</li> | ||
| <li>If |type| contains a standalone or comma-separated codec specifier string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make L580 bullet behavior optional and allow returning false from synch execution here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is already conditional, and step 9 below returns false. See previous comment. LMK if that solves your concern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's continue this discussion in the thread above :)
|
Hi @jan-ivar, when adding HEVC support we had some internal discussion on wether we should allow wildcardish operation and the conclusion at the time was to not allow it. We support allowing user agents that implement the wildcard strategy to fire an event asynchronously, but we want to allow Chrome to continue to do what it is doing (i.e. throw exception at construction or have isTypeSupported return false for codec.* types) for backwards-compat reason. We think it would be more ideal to feature freeze rather than change. |
Understood, but doesn't this leave it inconsistent within Chrome? Above I listed only 18 "notable exceptions" out of 158. The full list below shows Chrome supporting wildcard for all the other codecs: Full Chrome list (showing inconsistent support for wildcards across codec types): Supported mimeTypes (158) = [
"audio/webm",
"audio/webm;codecs=opus",
"audio/webm;codecs=opus.",
"audio/webm;codecs=opus.*",
"audio/webm;codecs=pcm",
"audio/webm;codecs=pcm.",
"audio/webm;codecs=pcm.*",
"audio/mp4",
"audio/mp4;codecs=opus",
"audio/mp4;codecs=mp4a.40.2",
"video/webm",
"video/webm;codecs=vp8",
"video/webm;codecs=vp8.",
"video/webm;codecs=vp8.*",
"video/webm;codecs=vp8,opus",
"video/webm;codecs=vp8,opus.",
"video/webm;codecs=vp8,opus.*",
"video/webm;codecs=vp8,pcm",
"video/webm;codecs=vp8,pcm.",
"video/webm;codecs=vp8,pcm.*",
"video/webm;codecs=vp9",
"video/webm;codecs=vp9.",
"video/webm;codecs=vp9.*",
"video/webm;codecs=vp9,opus",
"video/webm;codecs=vp9,opus.",
"video/webm;codecs=vp9,opus.*",
"video/webm;codecs=vp9,pcm",
"video/webm;codecs=vp9,pcm.",
"video/webm;codecs=vp9,pcm.*",
"video/webm;codecs=av1",
"video/webm;codecs=av1.",
"video/webm;codecs=av1.*",
"video/webm;codecs=av1,opus",
"video/webm;codecs=av1,opus.",
"video/webm;codecs=av1,opus.*",
"video/webm;codecs=av1,pcm",
"video/webm;codecs=av1,pcm.",
"video/webm;codecs=av1,pcm.*",
"video/webm;codecs=av01",
"video/webm;codecs=av01.",
"video/webm;codecs=av01.*",
"video/webm;codecs=av01,opus",
"video/webm;codecs=av01,opus.",
"video/webm;codecs=av01,opus.*",
"video/webm;codecs=av01,pcm",
"video/webm;codecs=av01,pcm.",
"video/webm;codecs=av01,pcm.*",
"video/webm;codecs=h264",
"video/webm;codecs=h264.",
"video/webm;codecs=h264.*",
"video/webm;codecs=h264,opus",
"video/webm;codecs=h264,opus.",
"video/webm;codecs=h264,opus.*",
"video/webm;codecs=h264,pcm",
"video/webm;codecs=h264,pcm.",
"video/webm;codecs=h264,pcm.*",
"video/webm;codecs=avc1",
"video/webm;codecs=avc1.",
"video/webm;codecs=avc1.*",
"video/webm;codecs=avc1,opus",
"video/webm;codecs=avc1,opus.",
"video/webm;codecs=avc1,opus.*",
"video/webm;codecs=avc1,pcm",
"video/webm;codecs=avc1,pcm.",
"video/webm;codecs=avc1,pcm.*",
"video/webm;codecs=avc3",
"video/webm;codecs=avc3.",
"video/webm;codecs=avc3.*",
"video/webm;codecs=avc3,opus",
"video/webm;codecs=avc3,opus.",
"video/webm;codecs=avc3,opus.*",
"video/webm;codecs=avc3,pcm",
"video/webm;codecs=avc3,pcm.",
"video/webm;codecs=avc3,pcm.*",
"video/mp4",
"video/mp4;codecs=vp9",
"video/mp4;codecs=vp9,opus",
"video/mp4;codecs=vp9,mp4a.40.2",
"video/mp4;codecs=av01",
"video/mp4;codecs=av01.0.19M.08",
"video/mp4;codecs=av01,opus",
"video/mp4;codecs=av01,mp4a.40.2",
"video/mp4;codecs=avc1",
"video/mp4;codecs=avc1.64003E",
"video/mp4;codecs=avc1,opus",
"video/mp4;codecs=avc1,mp4a.40.2",
"video/mp4;codecs=avc3",
"video/mp4;codecs=avc3.64003E",
"video/mp4;codecs=avc3,opus",
"video/mp4;codecs=avc3,mp4a.40.2",
"video/mp4;codecs=hvc1.1.6.L186.B0",
"video/mp4;codecs=hev1.1.6.L186.B0",
"video/x-matroska",
"video/x-matroska;codecs=vp8",
"video/x-matroska;codecs=vp8.",
"video/x-matroska;codecs=vp8.*",
"video/x-matroska;codecs=vp8,opus",
"video/x-matroska;codecs=vp8,opus.",
"video/x-matroska;codecs=vp8,opus.*",
"video/x-matroska;codecs=vp8,pcm",
"video/x-matroska;codecs=vp8,pcm.",
"video/x-matroska;codecs=vp8,pcm.*",
"video/x-matroska;codecs=vp9",
"video/x-matroska;codecs=vp9.",
"video/x-matroska;codecs=vp9.*",
"video/x-matroska;codecs=vp9,opus",
"video/x-matroska;codecs=vp9,opus.",
"video/x-matroska;codecs=vp9,opus.*",
"video/x-matroska;codecs=vp9,pcm",
"video/x-matroska;codecs=vp9,pcm.",
"video/x-matroska;codecs=vp9,pcm.*",
"video/x-matroska;codecs=av1",
"video/x-matroska;codecs=av1.",
"video/x-matroska;codecs=av1.*",
"video/x-matroska;codecs=av1,opus",
"video/x-matroska;codecs=av1,opus.",
"video/x-matroska;codecs=av1,opus.*",
"video/x-matroska;codecs=av1,pcm",
"video/x-matroska;codecs=av1,pcm.",
"video/x-matroska;codecs=av1,pcm.*",
"video/x-matroska;codecs=av01",
"video/x-matroska;codecs=av01.",
"video/x-matroska;codecs=av01.*",
"video/x-matroska;codecs=av01,opus",
"video/x-matroska;codecs=av01,opus.",
"video/x-matroska;codecs=av01,opus.*",
"video/x-matroska;codecs=av01,pcm",
"video/x-matroska;codecs=av01,pcm.",
"video/x-matroska;codecs=av01,pcm.*",
"video/x-matroska;codecs=h264",
"video/x-matroska;codecs=h264.",
"video/x-matroska;codecs=h264.*",
"video/x-matroska;codecs=h264,opus",
"video/x-matroska;codecs=h264,opus.",
"video/x-matroska;codecs=h264,opus.*",
"video/x-matroska;codecs=h264,pcm",
"video/x-matroska;codecs=h264,pcm.",
"video/x-matroska;codecs=h264,pcm.*",
"video/x-matroska;codecs=avc1",
"video/x-matroska;codecs=avc1.",
"video/x-matroska;codecs=avc1.*",
"video/x-matroska;codecs=avc1,opus",
"video/x-matroska;codecs=avc1,opus.",
"video/x-matroska;codecs=avc1,opus.*",
"video/x-matroska;codecs=avc1,pcm",
"video/x-matroska;codecs=avc1,pcm.",
"video/x-matroska;codecs=avc1,pcm.*",
"video/x-matroska;codecs=avc3",
"video/x-matroska;codecs=avc3.",
"video/x-matroska;codecs=avc3.*",
"video/x-matroska;codecs=avc3,opus",
"video/x-matroska;codecs=avc3,opus.",
"video/x-matroska;codecs=avc3,opus.*",
"video/x-matroska;codecs=avc3,pcm",
"video/x-matroska;codecs=avc3,pcm.",
"video/x-matroska;codecs=avc3,pcm.*",
"video/x-matroska;codecs=hvc1.1.6.L186.B0",
"video/x-matroska;codecs=hev1.1.6.L186.B0"
] |
I plan to update the PR based on #226 (comment) to include The compromise is to limit detection to wildcards, to reduce the fingerprinting surface. In my experience, web compat issues usually comes in the form of web devs forgetting to test their input with Safari or Firefox, not Chrome. So Chrome having a stricter test on input than spec (for a subset of its codecs) shouldn't break any web devs here. It seems Chrome should be able to unify detection of its latest codecs with its other codecs here without breaking anyone. |
Yes it is (which isn't optimal). I will await the wildcard update :) |
|
Now updated. @handellm PTAL. |
MediaRecorder.bs
Outdated
| The <dfn>list of synchronously exposed codec identifiers</dfn> is: | ||
| <code>"vp8"</code>, <code>"vp9"</code>, <code>"h264"</code> aka | ||
| <code>"avc1"</code>, <code>"av1"</code> aka <code>"av01"</code>, | ||
| <code>hvc1</code> aka <code>hev1</code>, <code>avc1</code> aka <code>avc3</code>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo? avc1 != avc3.
| <dd> | ||
| This method is deprecated; it exists for legacy compatibility reasons only. | ||
| The <dfn>list of synchronously exposed codec identifiers</dfn> is: | ||
| <code>"vp8"</code>, <code>"vp9"</code>, <code>"h264"</code> aka |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps also add vp08/vp09 weaving in @youennf 's comment above?
|
The isTypeSupported algorithm seems acceptable as it stands now from Chromium. We will need to do changes (allow wildcarding for the hevc types) but I think these changes are for the better. Still some clarifications to do regarding Chromium ctor behavior, spec text and intentions. |
Can you clarify, are you talking about adding an informative note or changing one of the steps of the constructor algorithm? |
I mean, this thread isn't reconciled yet. |
|
Just a progress update, this is still being circled internally. I'll update once we have a clear view on consequences. |
|
Hi @jan-ivar |
|
@handellm, it seems the main change for Chrome to align with this PR is to relax the checks for HEVC. |
|
One downside is the PR essentially defers establishing the non-throwing code-path to whenever a new codec is built in the future. Editor's meeting suggestion: We could add something like "User agent MAY attempt to shorten the list of synchronously exposed codec identifiers" to allow UAs a head start on establishing this second code path if they're comfortable with the web compat effect. |
|
I've added some histograms to Chrome to try to figure out how iTS and the MR ctor are used in production, we'll need another week to get initial data from Chrome Canary. See crbug.com/450757339 |
Fixes #202.
Preview | Diff