|
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 |
