Re: [MMUSIC] Comments on draft-alvestrand-mmusic-msid-01

[Harald Alvestrand <[e-mail]>]

Thanks for the review!


On 10/15/2012 11:05 PM, Paul Kyzivat wrote:
> Harald,
>
> I looked this over, and have a few observations:
>
> Section 2, 1st paragraph
>
> OLD:
>
>    This document extends the Source-Specific Media Attributes framework
>    [RFC5576] by adding a new "msid" attribute that can be used with the
>    "a=ssrc" SDP attribute.  This new attribute allows endpoints to
>    associate RTP media streams that are carried in different RTP
>    sessions, as well as allowing application-specific information to the
>    association.
>
> The above suggests the mechanism is only for associating media streams 
> in *different* sessions. But they can also be in the *same* session. 
> So I would suggest:
>
> NEW:
>
>    This document extends the Source-Specific Media Attributes framework
>    [RFC5576] by adding a new "msid" attribute that can be used with the
>    "a=ssrc" SDP attribute.  This new attribute allows endpoints to
>    associate RTP media streams that are carried in the same or
>    different RTP sessions, as well as allowing application-specific
>    information to the association.

Thanks, I'll change that. (Needed in the abstract too).

>
> Syntax:
>
>      ; "attribute" is defined in RFC 4566.
>      ; This attribute should be used with the ssrc-attr from RFC 5576.
>      attribute =/ msid-attr
>      msid-attr = "msid:" identifier [ " " appdata ]
>      identifier = 1*64 ("0".."9" / "a".."z" / "-")
>      appdata = 1*64 ("0".."9" / "a".."z" / "-")
>
> Do you *really* want to allow identifiers like "-----", "---abc--" and 
> "0---0--0-0"??? If these seem silly and error prone then the syntax 
> could be made more restrictive.
>
> If you *do* want to be this flexible, why not go further, and use 
> <token> as defined in 4566 for both <identifier> and <appdata>?

The list of "token" characters is

    token-char =          %x21 / %x23-27 / %x2A-2B / %x2D-2E / %x30-39
                          / %x41-5A / %x5E-7E

or !#$%&'*+-./^_`{|}~ in addition to A-Za-z0-9.

Some of these are metacharacters in Javascript.
However, metacharacter exploits was the only reason I could see for 
restricting the syntax - "allows stuff that looks silly" doesn't seem to 
merit extra constraints.
Your mileage may vary; to some degree, it's a matter of taste.

>
> Since it is recommended to generate the identifier via a random number 
> generator, then why not define a representation that is a common 
> encoding of an integer, such as hex or base64?
The reason for this particular pick was that I couldn't see a reason to 
disallow either hex or UUIDs.
There's a few characters missing for Base64 (+ and /), so I obviously 
wasn't thinking of Base64 whenI wrote this text - if people want base64, 
we can add them.
> The examples are obviously not generated this way. So maybe 
> RECOMMENDED is too strong for use of random numbers. (It was offered 
> as a possibility, but not recommended, in the prior version. Why the 
> change?)
Because the more I thought about it, the more it seemed that collisions 
were a bad idea, and collisions are more likely for non-random IDs than 
for randomly generated IDs (for reasonable quality random number 
generators).

>
> Section 3 - Msid-Semantic:
>
>    The ABNF of msid-semantic is:
>
>      attribute =/ msid-semantic-attr
>      msid-semantic-attr = "msid-semantic:" " " identifier token
>      token = <as defined in RFC 4566>
>
> Above appears broken - there is no separator between identifier and 
> token. Presumably you meant:
>
>      attribute =/ msid-semantic-attr
>      msid-semantic-attr = "msid-semantic:" " " identifier " " token
>      token = <as defined in RFC 4566>
Thanks - I missed that.
>
> Also, the text isn't clear on the relationship between
> a=msid-semantic:
> a=ssrc-group:
> a=group:
>
> Are these all just *analogous* but independent mechanisms for 
> grouping? Or is it intended that these can be used together to group 
> things of all these types together?
That's because I'm not clear on what it should be.

There's a good reason to say that one can generate an SDP offer with 
both a=msid-semantic and  a=ssrc-group for the same semantic in the same 
offer, because this offers one-round-trip negotiation with both 
endpoints that understand a=msid-semantic and endpoints that only 
understand a=group, but since msid-semantic is able to express the 
semantics of a=ssrc-group, I don't see why a response should reply with 
both ssrc-group and msid-semantic for the same semantic.
>
> RFC 5576 makes it clear that ssrc-group and group are analogous but 
> independent with independent registries.
>
> I find in the iana considerations section that msid-semantic is 
> reusing the "Semantics for the "ssrc-group" SDP Attribute" registry, 
> and registers "WMS" in that registry. So apparently one can use WMS 
> with a=ssrc-group. That suggests it is intended that can be used 
> together.
>
> BUT, ssrc-group is a media level attribute. That implies that the 
> namespace for ssrc-group tokens is independent for each media section 
> in the SDP. But msid-semantic is session-wide. Using it would couple 
> the namespaces of different media sections.
I'm not clear what you want to say here. What namespaces would be 
coupled by msid-semantic?
Since ssrc-group doesn't have tokens (it's just ssrcs), it doesn't seem 
to define a namespace.
>
> I don't know the answer here, but it at least needs clarification and 
> may require some changes.

At least some clarification.
>
> Section 4:
>
> This section feels like it belongs in a separate draft. If it were 
> non-normative then it might be ok as an example. But it is normative. 
> The following clearly is:
It was the original purpose of the draft - the split into a general 
mechanism and a specific usage was done at the behest of MMUSIC. So yes, 
it's definitely intended to be normative.
>
>    When an SDP description is updated, a specific msid continues to
>    refer to the same media stream; an msid value MUST NOT be reused for
>    another media stream within a PeerConnection's lifetime.
>
> It isn't clear if that is intended only for WebRTC media streams. 
> Hopefully so, since it is in a section scoped that way, and might not 
> be desirable for all uses.
The limitation is defined in terms of WebRTC MediaStreams, not RTP media 
streams, so yes. I'll make sure to make the casing right, and add the 
qualifier.
>
> Even for this use that limitation is problematic. Anything that 
> requires the generation of new SDP in a session to be aware of all the 
> historical SDP in that session is problematic. It is easy to get 
> situations (transfer, federation) where contributors to the SDP come 
> and go without knowledge of history prior to their joining.

I don't think that it requires one to be aware of history in practice. 
It just requires that you either record history, or that the identifier 
contains enough randomness, and that the random number generator is run 
once for every new MediaStream.

>
> (This is already a problem in SDP with the requirement that a payload 
> number can never be reused for a different codec within an RTP 
> session. It is my impression that this is often violated, usually 
> without problem.)

I don't know the rules for that - can one renegotiate the codec 
parameters for a payload type, or are those too supposed to be locked down?

Of course this is much more problematic in multiparty sessions than in 
offer/answer negotiated sessions....
>
>     Thanks,
>     Paul
> _______________________________________________
> mmusic mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/mmusic