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

Valid RID values mismatch between specs #2013

Closed
Orphis opened this issue Oct 23, 2018 · 10 comments
Closed

Valid RID values mismatch between specs #2013

Orphis opened this issue Oct 23, 2018 · 10 comments
Assignees
Labels
PR exists Simulcast Issue relating to Simulcast

Comments

@Orphis
Copy link
Contributor

Orphis commented Oct 23, 2018

According to WebRTC's addTransceiver(), the RID format is:

Verify that each rid value in sendEncodings is composed only of alphanumeric characters (a-z, A-Z, 0-9) up to a maximum of 16 characters.

But according to https://tools.ietf.org/html/draft-ietf-mmusic-rid-15 , we have:

rid-id = 1*(alpha-numeric / "-" / "_")

Which is correct?

@aboba aboba added the Simulcast Issue relating to Simulcast label Oct 24, 2018
@aboba aboba self-assigned this Oct 25, 2018
@aboba
Copy link
Contributor

aboba commented Nov 20, 2018

@Orphis @HTA I'd assume that the IETF specification takes precedence. The ABNF has been stable for quite a while so I'm not sure how the specifications got out of sync.

@amithilbuch
Copy link
Contributor

amithilbuch commented Nov 21, 2018

I would assume that IETF takes precedence, but i also found inconsistency in ietf.
According to: https://tools.ietf.org/html/draft-ietf-avtext-rid-09#section-3
As with all SDES items, RtpStreamId and RepairedRtpStreamId are
limited to a total of 255 octets in length. RtpStreamId and
RepairedStreamId are constrained to contain only alphanumeric
characters. For avoidance of doubt, the only allowed byte values for
these IDs are decimal 48 through 57, 65 through 90, and 97 through
122.

That draft has expired, but i haven't found a newer version.
RtpStreamId == rid-id

@amithilbuch
Copy link
Contributor

amithilbuch commented Jan 12, 2019

also found some firefox examples in related simulcast issues that show rids 'hi', 'mid', 'low' so we should make sure not to restrict to anything less than 3 characters.

@jan-ivar
Copy link
Member

jan-ivar commented Mar 5, 2019

What's the effective rid length limit here now? We're trying to fix web-platform-tests/wptwebrtc/RTCPeerConnection-addTransceiver.https.html which still says 16 bytes.

@amithilbuch
Copy link
Contributor

i think it needs to fit in a header extension, so i believe 16 is the right number.
Implementations and applications should prefer smaller values (like a single byte) to reduce packet size.

@jan-ivar
Copy link
Member

jan-ivar commented Mar 5, 2019

@amithilbuch Thanks, can you help me locate the spec support for that? Just making sure I'm not missing it.

@amithilbuch
Copy link
Contributor

we've actually decided not to formally specify it and refer to the specification in ietf.

Verify that each rid value in sendEncodings conforms to the grammar specified in Section 10 of
[MMUSIC-RID]. If one of the RIDs does not meet these requirements, throw a TypeError.
so it's referring to this:
https://tools.ietf.org/html/draft-ietf-mmusic-rid-15#section-10

it's out of date and might need a revision because it currently indicates a single character.

@jan-ivar
Copy link
Member

jan-ivar commented Mar 6, 2019

That's a normative reference, so that seems problematic if we want to allow 3 or even 16 characters. Do we need to open an issue?

@amithilbuch
Copy link
Contributor

@jan-ivar can you ask Mr. Roach what the original intent was? and if an upcoming draft is due that addresses this? if not, then i guess we should file an issue on the ietf spec.

@jan-ivar
Copy link
Member

jan-ivar commented Mar 6, 2019

@adamroach thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR exists Simulcast Issue relating to Simulcast
Projects
None yet
Development

No branches or pull requests

4 participants