Re: Genart last call review of draft-ietf-mmusic-rid-14

[Adam Roach <[e-mail]>]

Thanks for your review!

On 2/27/18 2:54 PM, Robert Sparks wrote:
> Nits:
>
> I could see implementor disagreement around what's allowed when modifying a
> session driven by the statement in the introduction that "They do not relax any
> existing descriptions." (where "They" here are the restrictions communicated
> with this attribute). Please look for a way to make it clear that an updating
> offer-answer round _can_ result in fewer restrictions than the original round
> did, just not fewer restrictions than the description places on the media if
> the "rid" extension is not present.

Good point. I've rephrased as "They do not relax any restrictions 
imposed by other mechanisms."

> (Micronit): I don't think the "To be clear" in the first paragraph on page 5
> helps.

Removed.

> I also worry that "it" may not have a clear meaning in "Such
> implementations must send it" later in that paragraph.

I've replaced "it" with "that SDES item".

> At the description of max-bpp on page 7, the last sentence is awkward. Do you
> perhaps mean "These values MUST NOT be encoded with more than four digits to
> the right of the decimal point."?

Yep. I'll change the phrasing to what you suggest.

> In the second paragraph of section 6, "its own unique 5-tuple" is arcane if the
> reader hasn't read the rest of the rtcweb work. Could you provide a description
> or a pointer to a description here?

I've added an inline description.

> At section 6.3, where you say 'For each "a=rid" line:', should you say 'For
> each "a=rid" line that has not been discarded by the requirements in section
> 6.2:'?

I've changed it to:

   For each "a=rid" line that has not been discarded by previous processing:

Which, I think, accomplishes the same thing while being more watertight.

> I found "a non-empty union" to be a confusing description of the condition you
> are trying to convey in section 8. (A union of two sets, at least one of which
> is not empty is going to be non-empty). I'm not sure intersection is the right
> word here either. Perhaps you could find a different way to characterize the
> condition?

Thanks for catching this. Intersection is what was meant here. I believe 
the example helps illustrate what is meant (in fact, I suspect the 
example is compelling enough that no one caught the 
"union"/"intersection" confusion until late review).

> The BNF for rid-id allows rid-ids like "This-is_my-favorite" or
> "Gm-Cqzkj4VHVD". But all the examples use single digits for rid-ids. I see the
> statement that "the actual identifiers used for RIDs are expected to be
> opaque". I strongly encourage putting some opaque ones in the examples.

Note that the rid-ids in this document eventually end up in RtpStreamID 
SDES items. As such, the ids used in examples in this document follow 
the recommendations in 
<https://tools.ietf.org/html/draft-ietf-avtext-rid-09#section-3.3>.

> Consider reminding people that ABNF quoted strings are case-insensitive, and
> that the grammar as written will allow things like "MaX-WiDtH" and "rECv". If
> that's not what you want, consider bringing in RFC7405.

Good catch. I believe the intention here is to match general SDP 
behavior as much as possible. RFC 4566 is frustratingly silent on case 
sensitivity of SDP attribute names, although the places where alphabetic 
characters are used in its ABNF, they are generally made case-sensitive. 
To make this as unsurprising as possible, I believe the best path 
forward is also making these values case-sensitive, for which I will use 
RFC 7405 syntax.


You can view the diffs between -14 and -15 at 
<https://www.ietf.org/rfcdiff?url1=draft-ietf-mmusic-rid-14&url2=draft-ietf-mmusic-rid-15>

/a