Subject: [MMUSIC] Initial comments on draft-ietf-mmusic-rid-01

From: Christer Holmberg <[email-masked]>
Date: Tue, 2 Feb 2016 12:15:53 +0000

Hi,

I was asked to take a look at the RID draft.

I think some editorial work is going to be needed. I realize it may not be the most important focus at this point, but if the draft is anyway updated for technical reasons the editorials can be fixed at the same time in order to make the draft easier to read. After all, there is a long list of authors, so... :)

I will not comment on all of the editorial issues ones I found (hopefully my general comments will get rid of most of them), but in this e-mail focus mostly on sections 1 and 6 (with subsections).

GENERAL:
------------

Q_G_1:

Please pay attention to usage of consistent terminology.

For example, I see "a= lines", "a= parameters", "a= attributes" type of terminology...

For example, I see "format of the RTP payload", "RTP payload format".

Q_G_2:

The draft talks a lot about "payload", "codec", "format" etc. I think the draft should contain a definition of those. If they are defined elsewhere, such text can say e.g.:

   "As defined in [RFCXXXX], payload refers to blah blah blah..."

Q_G_3:

Is there a risk of conflicts between information provided in a=rid and a=fmtp/rtpmap? If so, how shall it be treated?

SECTION 1:
-------------

Q_1_1:

The text says:

   "Payload Type (PT) in RTP provides a mapping between the format of the
   RTP payload and the media format description specified in the
   signaling.  For applications that use SDP for signaling, the
   constructs rtpmap and/or fmtp describe the characteristics of the
   media that is carried in the RTP payload, mapped to a given PT."

Assuming the scope of the draft is SDP, I think the paragraph can be shorter, e.g.:

   "Payload Type (PT) in RTP provides a mapping between the RTP payload format
   and the associated SDP media description. The SDP rtpmap and/or fmtp attributes
   are used, for a given PT, to the describe the characteristics of the media that is carried
   in the RTP payload."

Q_1_2:

The text says:

   "However, the assignment of static mapping of payload
   codes to payload formats and multiplexing of RTP with other protocols
   (such as RTCP) could result in limited number of payload type numbers
   available for the application usage."

What is a "payload code"? Shall it be "codec"?

Q_1_3:

The text says:

   "and to effectively map a RID RTP Stream to its corresponding constraints."

I know there is a reference for "RID RTP Stream" in the Terminology section, would I think it would be good to have the reference also in the Introduction.

Q_1_4:

The text says:

   "As described in Section 6.2.1, this mechanism achieves backwards
   compatibility via the normal SDP processing rules, which require
   unknown a= parameters to be ignored.  This means that implementations
   need to be prepared to handle successful offers and answers from
   other implementations that neither indicate nor honor the constraints
   requested by this mechanism."

SECTION 6:
-------------

Q_6_1:

The text says:

    "For each media description in the offer,..."

It should say for each media description that describes RTP based media, or something like that...

Q_6_2:

The text says:

    "It MUST generate a "rid-identifier" that is unique within a media description"

Later, in section 6.5, the draft says that a new offer with "proposed changes" should use a new ID value. What about a subsequent RTP session? Is it ok to re-use the value(s) from the previous session?

Q_6_3:

The text says:

   "The Offerer then chooses zero or more "a=rid" constraints
   (Section 5) to be applied to the rid, and adds them to the
    "a=rid" line."

What is meant by "the rid"?

Q_6_4:

In section 6.2.1, the text says:

   "As a result, ignoring the "a=rid" line is always guaranteed to result in a valid session description."

While this is true, a few words about the consequences of the fact that RID won't be used is needed.

For example, if the offerer has indicated some constraints using a=rid, and the answerer does not support a=rid, does the offerer have to be prepared to receive media "outside" of those constraints, etc?

Just saying that unsupported attributes are ignored, but it doesn't matter because the SDP will still be valid, is not enough when it comes to describing the backward compatibility :)

Q_6_5:

In section 6.3, the text says:

    "Having performed verification of the SDP offer as described"

I assume something is missing behind "described"?

I guess the answer to the question above may clarify it, but what is meant by "verification"?

Q_6_6:

In section 6.4, I see a mix of "MUSTs" and "SHALLs", but I can't really see the difference.

Q_6_7:

In section 6.4, the text talks about cases where "the offerer shall consider the a= line as rejected".

It is unclear whether it is meant to mean that the answerer rejected the a= line (which I agree would be a little strange), or if the offerer shall reject the a= line.

Assuming it's the offerer, I think it would be more clear the say "the offerer shall reject the a= line".

I also think it would be better to say "discard" instead of "reject", because "reject" often means that some additional action is taken, which I assume is not the case here.

Regards,

Christer

---

From: Adam Roach <[email-masked]>
Date: Wed, 3 Feb 2016 09:31:30 -0600

> I was asked to take a look at the RID draft.

Thanks for a really quick turnaround on reviewing it!

What would be really helpful is input on the explicitly-called-out open 
issues.

> For example, I see “a= lines”, “a= parameters”, “a= attributes” type 
> of terminology…
>

I think this is consistent already, with one exception (which will be 
fixed in -02). If you can point out particular instances that are 
causing issues, I'd appreciate it.

To be clear, the phrase '"a=rid" line' refers to an entire line; e.g.:

a=rid:1 send max-width=1280;max-height=720;max-fps=30

Any reference to a "parameter" refers to a parameter on such a line. For 
example, "send" in the preceding line. Or "max-fps=30".

The only way I think this could be made more clear would be with the 
addition of a rather superfluous paragraph in the terminology section, 
along the lines of:

    "The term 'attribute' is used in this document as having the same
    meaning as that word in Section 5.13 of [RFC4566]. The term 'line'
    is used in this document as having the same meaning as that word as
    used in section 5, and *almost* all of its subsections, in
    [RFC4566]. The term 'parameter' is used in this document to have its
    common English meaning, with deep parallels to its use in the
    'a-fmtp:' description in section 6 of [RFC4566]."

The point, in case it's not clear, is that these terms in the document 
are being used as they've been used to describe SDP components for decades.

> Q_G_2:
>
> The draft talks a lot about “payload”, “codec”, “format” etc. I think 
> the draft should contain a definition of those. If they are defined 
> elsewhere, such text can say e.g.:
>
>    “As defined in [RFCXXXX], payload refers to blah blah blah…”
>

Again referring to RFC4566, these terms are used freely with no 
definition or external citation. Are you expecting people to implement 
this SDP extension without having first understood base SDP?

> Q_G_3:
>
> Is there a risk of conflicts between information provided in a=rid and 
> a=fmtp/rtpmap? If so, how shall it be treated?
>

Section 8 exists solely to address this question. What do you believe is 
missing?

> SECTION 1:
>
> -------------
>
> Q_1_1:
>
> The text says:
>
>    “Payload Type (PT) in RTP provides a mapping between the format of the
>
>    media that is carried in the RTP payload, mapped to a given PT.”
>
>    “Payload Type (PT) in RTP provides a mapping between the RTP 
> payload format
>
>    in the RTP payload.”

Sure, why not. I've updated accordingly.

> Q_1_2:
>
> The text says:
>
>    “However, the assignment of static mapping of payload
>
>    available for the application usage.”
>
> What is a “payload code”? Shall it be “codec”?
>

In the language used by RFC 4566, this would be "RTP payload type 
number". I'll update accordingly.

> Q_1_3:
>
> The text says:
>
>    “and to effectively map a RID RTP Stream to its corresponding 
> constraints.”
>
> I know there is a reference for “RID RTP Stream” in the Terminology 
> section, would I think it would be good to have the reference also in 
> the Introduction.
>

Based on this and other previous comments, I'm biting the bullet and 
moving the terminology section to the front of the document, before the 
introduction.

> Q_1_4:
>
> The text says:
>
>    “As described in Section 6.2.1, this mechanism achieves backwards
>
>    requested by this mechanism.”
>

It does. Did you want to comment on it?

> SECTION 6:
>
> -------------
>
> Q_6_1:
>
> The text says:
>
>     “For each media description in the offer,…”
>
> It should say for each media description that describes RTP based 
> media, or something like that…
>

Updated: "For each RTP media description..."

> Q_6_2:
>
> The text says:
>
>     “It MUST generate a "rid-identifier" that is unique within a media 
> description”
>
> Later, in section 6.5, the draft says that a new offer with “proposed 
> changes” should use a new ID value. What about a subsequent RTP 
> session? Is it ok to re-use the value(s) from the previous session?
>

Sure, but I think that's pretty self-evident. I can't think of any 
construct that calls out constraints between non-concurrent RTP sessions.

> Q_6_3:
>
> The text says:
>
>    “The Offerer then chooses zero or more "a=rid" constraints
>
>     "a=rid" line.”
> What is meant by “the rid”?
>

Updated: "...to the the RID RTP Stream..."

>    “As a result, ignoring the "a=rid" line is always guaranteed to 
> result in a valid session description.”
>
> While this is true, a few words about the consequences of the fact 
> that RID won’t be used is needed.
>
> For example, if the offerer has indicated some constraints using 
> a=rid, and the answerer does not support a=rid, does the offerer have 
> to be prepared to receive media “outside” of those constraints, etc?
>
> Just saying that unsupported attributes are ignored, but it doesn’t 
> matter because the SDP will still be valid, is not enough when it 
> comes to describing the backward compatibility :)
>

Sure, but this is true regardless of whether the remote endpoint 
supports "a=rid". It is quite possible that the result of applying the 
rules in the "answer" section results in any given m-section containing 
zero "a=rid" lines. I've added text to the "Offerer Processing of the 
SDP Answer" section:

    It is important to note that there are several ways in which an
    offer can
    contain a media section with "a=rid" lines, but the corresponding media
    section in the response does not. This includes situations in which the
    answerer does not support "a=rid" at all, or does not support the
    indicated
    constraints. Under such circumstances, the offerer MUST be prepared to
    receive a media stream to which no constraints have been applied.

>     “Having performed verification of the SDP offer as described”
>
> I assume something is missing behind “described”?
>
> I guess the answer to the question above may clarify it, but what is 
> meant by “verification”?
>

It's referring to the immediately preceding section, which uses the word 
"verify" several times. For absolute clarity, I've updated its initial 
paragraph to include the word "verification":

    If the answerer supports the "a=rid" attribute, the following
    verification
    steps are executed, in order, for each "a=rid" line in a given media
    description:

And I've explicitly cited it in the sentence you point out:

    Having performed verification of the SDP offer as described in
    Section 6.2.2, the answerer shall perform the following steps to
    generate the SDP answer.

> Q_6_6:
>
> In section 6.4, I see a mix of “MUSTs” and “SHALLs”, but I can’t 
> really see the difference.
>

I suppose that's good, since RFC2119 defines them to have no difference.

MUST, SHALL, and REQUIRED all indicate the same normative level. This is 
kind of nice, since it allows authors to choose among several terms 
according to which one flows better in the sentence.

> Q_6_7:
>
> In section 6.4, the text talks about cases where “the offerer shall 
> consider the a= line as rejected”.
>
> It is unclear whether it is meant to mean that the answerer rejected 
> the a= line (which I agree would be a little strange), or if the 
> offerer shall reject the a= line.
>
> Assuming it’s the offerer, I think it would be more clear the say “the 
> offerer shall reject the a= line”.
>
> I also think it would be better to say “discard” instead of “reject”, 
> because “reject” often means that some additional action is taken, 
> which I assume is not the case here.
>

Agreed. I've changed these to use language along the lines of "...shall 
discard the 'a=rid' line."

I'll have an -02 version out shortly with these changes incorporated.

/a