Skip to content
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

Comparison rules for RTCRtpCodec needs clarification on how mimeType is compared #2971

Closed
docfaraday opened this issue May 6, 2024 · 10 comments · Fixed by #2975
Closed

Comparison rules for RTCRtpCodec needs clarification on how mimeType is compared #2971

docfaraday opened this issue May 6, 2024 · 10 comments · Fixed by #2975

Comments

@docfaraday
Copy link
Contributor

Recently I've found multiple wpt that are doing case-sensitive comparisons on the mimeType field. Mime-types are case insensitive (see https://datatracker.ietf.org/doc/html/rfc2045#section-5.1); this should be called out in https://w3c.github.io/webrtc-pc/#dfn-codec-dictionary-match

@jan-ivar
Copy link
Member

jan-ivar commented May 7, 2024

This is a side-effect of not using interfaces to represent platform codecs #2845 (comment)

Following that decision, it seems we need to support JS constructing the inputs to setCodecPreferences out of whole cloth, and we'd need to support case-insensitive mimeType matching here as RFC 2045 says.

@jan-ivar
Copy link
Member

jan-ivar commented May 7, 2024

WPTs that conjure up inputs to test browser tolerance need to use case-insensitive match.

WPTs that solely operate on codecs emitted by the browser itself are probably fine checking that case matches IMHO.

@alvestrand
Copy link
Contributor

A WPT should never depend on anything that is not written in the specs.
The fact that all implementations today are consistent about the case of their self-generated media types doesn't mean that a WPT can depend on that being so forever.

@jan-ivar
Copy link
Member

I didn't mean WPTs relying on all browsers being consistent, only that they're self-consistent.

But sure, if @docfaraday can provide a list of the WPTs in question we can fix them to be case-insensitive.

Just seems a pity to allow some browser to return vIdEo/Vp8. I'm curious how other specs deal with this, if at all.

@alvestrand
Copy link
Contributor

The SDP specs say that if a browser gets vIdEo/Vp8, it has to behave exactly as if it got ViDeO/vP8.
It is good if we test this.

@Orphis
Copy link
Contributor

Orphis commented May 16, 2024

I did notice that and added some proper comparison in a few newer tests like in https://github.com/web-platform-tests/wpt/blob/master/webrtc-extensions/RTCRtpParameters-codec.html#L28

It would be interesting to clarify which method should be used in JS. Can the mimeType also contain UTF-8 characters?

@jan-ivar
Copy link
Member

jan-ivar commented May 22, 2024

RTCRtpCodec's mimeType must be:

  • "listed in [IANA-RTP-2]."
    • which states "Both type and subtype names are case-insensitive as defined in RFC4288."
    • it further says its "registration procedures ... can be found in [RFC4855]"
      • which says "RFC 4288 [1] defines media type specification and registration
        procedures that use the Internet Assigned Numbers Authority (IANA) as
        a central registry."
        • which is "Obsoleted by: 6838", where section 4.2 says
          • "Both top-level type and subtype names are case-insensitive."
          • and also
            Type and subtype names MUST conform to the following ABNF:
            
            type-name = restricted-name
            subtype-name = restricted-name
            
            restricted-name = restricted-name-first *126restricted-name-chars
            restricted-name-first  = ALPHA / DIGIT
            restricted-name-chars  = ALPHA / DIGIT / "!" / "#" /
                                     "$" / "&" / "-" / "^" / "_""_
            
            • where ALPHA is defined in core rules as
              ALPHA          =  %x41-5A / %x61-7A   ; A-Z / a-z
              

This seems clear enough to me: case-insensitive, and no UTF. Can we close?

@jan-ivar
Copy link
Member

Sorry my bad we need to update the codec dictionary match. I'll do a PR.

@jan-ivar
Copy link
Member

It would be interesting to clarify which method should be used in JS. Can the mimeType also contain UTF-8 characters?

This is a good question. Since we take DOMString as an input, we need to lower-case JS input in order to compare it. While we could UTF-8 encode it to bytes and use byte-lowercasing, we can probably just use ascii-lowercase.

@alvestrand
Copy link
Contributor

note - quote from 6886 is missing two lines:

 restricted-name-chars =/ "." ; Characters before first dot always
                              ; specify a facet name
 restricted-name-chars =/ "+" ; Characters after last plus always
                              ; specify a structured syntax suffix
  • and . are permitted in names, and have special meaning (facet and structured syntax respectively).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants