Fixup MIME usage #222

marcoscaceres · 2024-07-04T10:05:16Z

Closes #69

this is a draft... it doesn't actually address all the issues with MIME usage yet... just a start.

Preview | Diff

index.bs

chrisn · 2024-08-09T16:52:44Z

This PR replaces valid media MIME type with algorithmic steps. It could lead to similar changes to valid video configuration all the way up to valid MediaConfiguration. Please comment if you see any issues with the direction this is taking.

@aboba One specific question, as we develop this PR: section 2.1.4.2 RTP currently references RFC4855 and RFC6838, which define registration requirements. This PR proposes changing these to reference https://www.iana.org/assignments/rtp-parameters/rtp-parameters.xhtml instead. Is this OK?

chrisn · 2024-08-13T07:53:16Z

This is ready for review. PTAL @mfoltzgoogle, @aboba, @alastor0325, @marcoscaceres

The PR makes two main changes:

Replace "valid media MIME type"
Changes other algorithms to use the abstract-op dfn type

If needed we can tackle "valid video configuration" etc in follow-up PRs.

markafoltz · 2024-08-19T22:35:29Z

LGTM with one side comment, nice PR @marcoscaceres

I just did a first read of the spec and wondered if a long term possibility is to decouple from mime types along the lines of the WebCodecs registry. However, the registry is missing a bunch of codecs though that may never be supported in WebCodecs, and container parsing support seems to be out of scope for WebCodecs as well.

chrisn · 2024-08-20T11:03:00Z

There's a similar discussion in this EME issue: w3c/encrypted-media#559. I think a registry could help, but also recognise Joey's concerns in w3c/encrypted-media#559 (comment). This might be one to talk about at TPAC.

markafoltz · 2024-09-17T20:16:29Z

@adoba, bumping the question above in #222 (comment):

One specific question, as we develop this PR: section 2.1.4.2 RTP currently references RFC4855 and RFC6838, which define registration requirements. This PR proposes changing these to reference https://www.iana.org/assignments/rtp-parameters/rtp-parameters.xhtml instead. Is this OK?

index.bs

markafoltz · 2024-10-07T18:50:14Z

@chrisn per https://www.w3.org/2024/09/26-mediawg-minutes.html#a01:

ACTION: cpn to update the Media Capabilities PR to refer to the main MIME type registry

After that is done, this should be ready to merge.

chrisn · 2024-10-08T12:06:52Z

I've added the link to the IANA media type registry, as well as bringing back the references to the RTP specific RFCs.

Helps readers understand the algorithm is specified in EME.

chrisn · 2024-10-08T23:18:17Z

@markafoltz I have removed the RFC4855 reference as we discussed in today's meeting (minutes). The PR is ready for your review. Many thanks.

index.bs

markafoltz · 2024-10-09T16:23:11Z

index.bs

+            and {{MediaEncodingType}} or {{MediaDecodingType}} |encodingOrDecodingType|.
+          </li>
+          <li>
+            If |result| is <code>failure</code> or <code>unsupported</code>,


Two problems here?

The Check MIME Type Support steps don't return unsupported, but see my proposed change below to fix the steps.

If the result is unsupported, IIUC, we shouldn't reject the configuration as invalid. We should pass through the result to Create a MediaCapabilitiesDecodingInfo to set the supported property of the MediaCapabilitiesDecodingInfo return value.

You're right ... we should only reject if parsing the MIME type fails or the type something other than video or application. I can try to adjust the algorithms.

The difficulty I see is that we currently call Check MIME Type Support from valid video configuration and valid audio configuration (via valid MediaDecodingConfiguration and valid MediaConfiguration) but we need to use the result from Check MIME Type Support in Create a MediaCapabilitiesDecodingInfo.

What I'd like to propose is we write a complete set of algorithm steps for decodingInfo() and in doing so remove the valid MediaDecodingConfiguration, valid MediaConfiguration, valid audio configuration, valid video configuration, etc. concepts (and similar for encodingInfo()).

This would make this PR a much bigger rewrite, but I think it would takes us in the right direction. The alternative would be to keep valid video configuration etc but then we'd need to split Check MIME Type Support into two algorithms: one to check basic validity (does it parse, is its type audio, video or application, etc), the other to check for UA support.

Or am I missing an easier way to solve this?

Can we take Check MIME Type Support out of the valid video configuration steps and instead call it in the Create a MediaCapabilitiesDecodingInfo algorithms? (Similar for audio, encoding, etc.)

It would be okay to reject early with a TypeError because the input is invalid (i.e. correct properties not set) and later also reject with a TypeError because the contentType could not be parsed.

Sorry for the delay, I'm looking into this...

I don't see any reason why it's not valid. Internal slots are intended to be part of the implementation and not visible to user scripts. I could call in a webidl expert if we need to.

I will defer to @xhwang-chromium and EME experts to handle that :)

I would need to talk to folks more directly involved with the implementation, but from what I can tell, Chrome checks the two values independently. If we really needed to we could amend the steps to be something like:

If audioMimeSupport is supported, videoMimeSupport is supported, and the user agent can decode the media represented by configuration, set supported to true and false otherwise.

Also in "valid video configuration" I don't think we now have enough checks. To be consistent with the logic before this PR, it would have to check for video or application type and the parameters.

Yes, it looks like the checks in 2.1.4. MIME types got dropped.

I pushed a commit with most of my suggested changes, feel free to comment/amend as you like.

I wasn't able to resolve this issue though, because the steps in Check MIME Type Support need to be updated. They can't return failure because the steps that call it don't expect that result; with the current PR, failure would result in a supported return value.

And its not clear to me yet whether the checks in 2.1.4.1 and 2.1.4.2 (in the current spec) belong in Check MIME Type Support (and take the unsupported path) or check for valid {video,audio} configuration (and take the failure path). Might have to check the WPTs to see what implementations do.

The checks for audio, video, application etc. and a "single codec" all take the failure path in WPTs.

https://github.com/web-platform-tests/wpt/blob/f54cd920024da0857fdc5b036f16a7d1fd8792fd/media-capabilities/decodingInfo.any.js#L124

I found one WPT case that checks whether a valid mime type is also valid for WebRTC, and it takes the unsupported path:

https://github.com/web-platform-tests/wpt/blob/f54cd920024da0857fdc5b036f16a7d1fd8792fd/media-capabilities/decodingInfo-webrtc.any.js#L137

I'm doing some tests, and finding some inconsistencies with the "imply a codec" checks:

Chrome rejects if there are parameters other than codecs, Firefox resolves. The spec requires implementations to reject

Chrome resolves on audio/webm for audio contentType, Firefox rejects. A codecs parameter should be required

Chrome resolves on video/webm for video contentType, Firefox rejects. A codecs parameter should be required

I also found a difference in parsing behaviour:

Chrome rejects on audio/mpeg; (note the semicolon), Firefox resolves

index.bs

Co-authored-by: Mark Foltz <[email protected]>

This adds an internal slot to |AudioConfiguration| and |VideoConfiguration| to store the parsed MIME type

markafoltz

This looks good, with a couple of spots I noted where steps could be simplified or the order changed to be more logical. Nothing that blocks merging IMO.

index.bs

marcoscaceres · 2024-11-07T09:21:00Z

index.bs

+             <code>false</code> and abort these steps.
+          </li>
+          <li>
+            Create an internal slot on |configuration|, \[[mimeType]], and set


As above... at the same time, this duplicates the other algorithm... can we merge these two algorithms and switch on the media type?

The validation steps for audio and video configurations are slightly different so I don't see a way to merge them completely.

index.bs

chrisn · 2024-11-07T10:32:27Z

index.bs

+          </li>
+          <li>
+            If the combined <code>type</code> and <code>subtype</code> members
+            of |mimeType| imply a single media codec:


"imply a single media codec" is vague. Making it more precise would involve listing the specific MIME types where a codecs parameter isn't also needed. Related: #235

marcoscaceres commented Jul 18, 2024

View reviewed changes

index.bs Outdated Show resolved Hide resolved

marcoscaceres commented Jul 18, 2024

View reviewed changes

index.bs Outdated Show resolved Hide resolved

marcoscaceres commented Jul 18, 2024

View reviewed changes

index.bs Outdated Show resolved Hide resolved

marcoscaceres commented Jul 18, 2024

View reviewed changes

index.bs Outdated Show resolved Hide resolved

marcoscaceres commented Jul 18, 2024

View reviewed changes

index.bs Outdated Show resolved Hide resolved

marcoscaceres commented Jul 18, 2024

View reviewed changes

index.bs Outdated Show resolved Hide resolved

chrisn force-pushed the mime branch 2 times, most recently from 305ab3d to b7f3138 Compare July 18, 2024 11:01

tidoust reviewed Jul 22, 2024

View reviewed changes

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

chrisn force-pushed the mime branch from 042414b to 3518987 Compare August 8, 2024 13:34

chrisn changed the title ~~WIP: Fixup MIME usage~~ Fixup MIME usage Aug 13, 2024

chrisn mentioned this pull request Sep 13, 2024

Use "parse a MIME type" to check validity. #105

Closed

chrisn added this to the V1 milestone Sep 13, 2024

chrisn added the TPAC2024 Topic for discussion at TPAC 2024 label Sep 13, 2024

chrisn approved these changes Sep 16, 2024

View reviewed changes

chrisn requested review from tidoust and aboba September 16, 2024 18:23

tidoust approved these changes Sep 17, 2024

View reviewed changes

chrisn mentioned this pull request Sep 17, 2024

Handle video profiles mismatched with colorGamut and transferFunction. #231

Open

cconcolato reviewed Sep 26, 2024

View reviewed changes

index.bs Outdated Show resolved Hide resolved

markafoltz assigned chrisn Oct 7, 2024

marcoscaceres and others added 2 commits October 8, 2024 13:07

WIP: Fixup MIME usage

a395dfc

Word wrap

d7aa76e

chrisn added 8 commits October 8, 2024 13:11

WIP: use abstract-op links

eb7a1c0

Added EME reference for Get Supported Configuration

80bfb9f

Helps readers understand the algorithm is specified in EME.

Added ignore to fix bikeshed warning

33497a7

Fixed 'media' and 'encodingOrDecodingType' references

fd2e232

Rename algorithm to 'Check MIME Type Support'

324a907

Clarify MIME type algorithm

4b5ed31

Fix whitespace issues in mouse-over prompts

5809216

Add reference to IANA media type registry

17edcd5

chrisn force-pushed the mime branch from e797955 to 17edcd5 Compare October 8, 2024 12:13

chrisn mentioned this pull request Oct 8, 2024

Why does it matter if a single media codec is listed? #235

Open

Remove RFC4855 as the RTP payload registry is closing

59098c9

markafoltz requested changes Oct 9, 2024

View reviewed changes

chrisn and others added 3 commits October 9, 2024 19:59

Use enum-like return value in Check MIME Type Support

d342de5

Co-authored-by: Mark Foltz <[email protected]>

Use enum-like return value in Check MIME Type Support

b1c0ee9

Avoid calling Check MIME Type Support multiple times

73e3176

This adds an internal slot to |AudioConfiguration| and |VideoConfiguration| to store the parsed MIME type

markafoltz reviewed Nov 1, 2024

View reviewed changes

index.bs Show resolved Hide resolved

index.bs Show resolved Hide resolved

index.bs Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

Make changes

21cb439

marcoscaceres commented Nov 7, 2024

View reviewed changes

index.bs Outdated Show resolved Hide resolved

marcoscaceres commented Nov 7, 2024

View reviewed changes

index.bs Outdated Show resolved Hide resolved

marcoscaceres commented Nov 7, 2024

View reviewed changes

index.bs Outdated Show resolved Hide resolved

marcoscaceres commented Nov 7, 2024

View reviewed changes

index.bs Outdated Show resolved Hide resolved

marcoscaceres commented Nov 7, 2024

View reviewed changes

index.bs Outdated Show resolved Hide resolved

marcoscaceres commented Nov 7, 2024

View reviewed changes

index.bs Outdated Show resolved Hide resolved

marcoscaceres commented Nov 7, 2024

View reviewed changes

index.bs Outdated Show resolved Hide resolved

marcoscaceres commented Nov 7, 2024

View reviewed changes

index.bs Outdated Show resolved Hide resolved

chrisn reviewed Nov 7, 2024

View reviewed changes

markafoltz added 2 commits November 8, 2024 13:47

Address comments from Marcos

1d206f4

Fix typo in MediaEncodingInfo steps

b3b53aa

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixup MIME usage #222

Fixup MIME usage #222

marcoscaceres commented Jul 4, 2024 •

edited by pr-preview bot

Loading

chrisn commented Aug 9, 2024

chrisn commented Aug 13, 2024

markafoltz commented Aug 19, 2024

chrisn commented Aug 20, 2024

markafoltz commented Sep 17, 2024

markafoltz commented Oct 7, 2024

chrisn commented Oct 8, 2024

chrisn commented Oct 8, 2024

markafoltz Oct 9, 2024

chrisn Oct 10, 2024

chrisn Oct 14, 2024

markafoltz Oct 14, 2024

chrisn Oct 21, 2024

markafoltz Nov 1, 2024

markafoltz Nov 1, 2024

markafoltz Nov 4, 2024

markafoltz Nov 11, 2024

chrisn Nov 11, 2024

markafoltz left a comment

marcoscaceres Nov 7, 2024 •

edited

Loading

markafoltz Nov 8, 2024

chrisn Nov 7, 2024

Fixup MIME usage #222

Are you sure you want to change the base?

Fixup MIME usage #222

Conversation

marcoscaceres commented Jul 4, 2024 • edited by pr-preview bot Loading

chrisn commented Aug 9, 2024

chrisn commented Aug 13, 2024

markafoltz commented Aug 19, 2024

chrisn commented Aug 20, 2024

markafoltz commented Sep 17, 2024

markafoltz commented Oct 7, 2024

chrisn commented Oct 8, 2024

chrisn commented Oct 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markafoltz left a comment

Choose a reason for hiding this comment

marcoscaceres Nov 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcoscaceres commented Jul 4, 2024 •

edited by pr-preview bot

Loading

marcoscaceres Nov 7, 2024 •

edited

Loading