Re: [MMUSIC] WGLC for draft-ietf-mmusic-sdp-bundle-negotiation-19

[Adam Roach <[e-mail]>]

On 3/30/15 16:01, Flemming Andreasen wrote:
> Please review and provide any comments you may have on the document 
> changes by Monday, April 13, 2015. Comments should be sent to the 
> document authors and the MMUSIC WG list. If you review the document 
> but do not have any comments, please send a note to that effect as well.

I have reviewed the body of the document, and have a number of comments. 
These are of varying severity, ranging from editorial suggestions to 
proposed normative changes. They appear in document order.

---------------------------------------------------------------------------

In the Abstract:
   "This
    specification also updates sections 5.1, 8.1 and 8.2 of RFC 3264 to
    allow an answerer to assign a non-zero port value to an "m=" line in
    an SDP answer, even if the "m=" line in the associated SDP offer
    contained a zero port values.

This seems to be a bit of a deep dive for the abstract, and I would propose
removing it. If it does remain, it needs to include something like "under
certain circumstances." But I think removal is the better option.

The introduction should also qualify the similar statement in this 
paragraph:

    This specification also updates sections 5.1, 8.1 and 8.2 of RFC 3264
    [RFC3264].  The update allows an answerer to assign a non-zero port
    value to an "m=" line in an SDP answer, even if the "m=" line in the
    associated SDP offer contained a zero port value.

---------------------------------------------------------------------------

Especially because the term is overloaded, you should expand the meaning
of "SDES" on first use; something like:

   "This specification also defines a new Real-time Transport Protocol
    (RTP) [RFC3550] Source Description (SDES) item..."

---------------------------------------------------------------------------

Terminology: Since we are using '"m=" line' in a way that isn't (to my
knowledge) defined in another document, this section should include a
definition for that term. Yes, it's in the introduction, but that's not 
where
people look for definitions of terms.

---------------------------------------------------------------------------

Section 5 should cite RFC 4961 for the definition of "symmetric RTP."

---------------------------------------------------------------------------

Section 5 has this normative requirement:

    A given address:port
    combination MUST NOT be used for sending media associated with
    multiple BUNDLE groups.

It's not clear what this gains us. Everywhere else, we only require a unique
5-tuple per bundle. That seems like the correct restriction to impose.
(If we change this, it also has impacts on section 8.3.3)

---------------------------------------------------------------------------
Section 5:

    Each "m=" line, whose identification-tag is listed in the
    identification-tag list, is associated with a given BUNDLE group.

Repunctuate as:

    Each "m=" line whose identification-tag is listed in the
    identification-tag list is associated with a given BUNDLE group.

(cf. <https://www.google.com/search?q=restrictive+clause>)

---------------------------------------------------------------------------

Section 6:
   This whole section feels really out of place here; it's assuming that the
   reader has knowledge about BUNDLE offer/answer handling that he won't 
learn
   until reading section 8. I would propose that you leave the attribute
   definition in section 14.3, and move the semantics into section 8.

---------------------------------------------------------------------------

Section 6:
   "In order to ensure that an answerer that does not supports the BUNDLE"

s/supports/support/

---------------------------------------------------------------------------

Section 6:

    NOTE: Once an offerer BUNDLE address has been selected, the offerer
    can ensure that an bundled "m=" line is accepted by the answerer only
    if the answerer keeps the "m=" line within the associated BUNDLE
    group by assigning the offerer BUNDLE address to the "m=" line. If
    the answerer does not keep that "m=" line within the BUNDLE group,
    the answerer will reject it.  Therefore, the SDP 'bundle-only'
    attribute is not needed in such cases

I can't make any sense of that paragraph, and it's only partially 
because the
passive "has been selected" doesn't indicate who is performing the 
selection.
In particular, it doesn't sound like the offerer is *ensuring* anything. 
Maybe
we're trying to say he can detect something?  But even with that
interpretation, it's not clear what circumstances it's trying to say don't
require the use of 'bundle-only.' Or is this trying to say that subsequent
offers (as defined in section 2) don't need to use 'bundle-only' because
you know whether the peer supports bundle? That makes some sense, but it's
really hard to get that meaning out of the text.

Let me take a stab at something.

    NOTE: Subsequent offers do not need to include the 'bundle-only' 
attribute,
    since the offerer will already know whether its peer supports the BUNDLE
    mechanism. With that knowledge, the offerer can assign a new 'm=' 
line to
    an existing BUNDLE address. By doing so, they ensure that the answerer
    either accepts the line into the BUNDLE, or completely rejects it.

If that's what we mean, I propose replacing the paragraph with it. If it's
not, then the paragraph needs to be rewritten. Also, if that's what we mean,
then it needs corresponding text in section 8.3.1 (although if you take the
suggestion to move the semantics from section 6 into section 8, it might
make more sense to just put it there only rather than describe it in two
places).

---------------------------------------------------------------------------

Section 8.1:

    The offerer and answerer MUST follow the rules and restrictions
    defined in Section 7 when creating offers and answers.

This adds nothing. I would propose removing it (or, if you don't want to
remove it, I would propose that consistency calls for having every section
normatively require that implementors follow the normative statements of 
every
other section in the document).

---------------------------------------------------------------------------

Section 8.2.1:

    o  Add an SDP 'group:BUNDLE' attribute to the offer;

Replace with:

    o  Add an SDP 'group:BUNDLE' attribute to the offer at the session 
level;

---------------------------------------------------------------------------

Section 8.3.1:

    When an answerer generates an answer, which contains a BUNDLE group,

Replace with:

    When an answerer generates an answer that contains a BUNDLE group,

(cf. <https://www.google.com/search?q=which+that>)

---------------------------------------------------------------------------

The procedure described in Section 8.3.2 has a rather unfortunate property
that I'm not sure we want. If I were to offer something like:

a=group:BUNDLE foo bar
m=video 20002 RTP/AVP 97
a=rtpmap:97 H261/90000
a=mid:foo
m=video 0 RTP/AVP 97 98
a=rtpmap:97 H261/90000
a=rtpmap:98 VP8/90000
a=bundle-only
a=mid:bar

And the remote endpoint wanted to reject the first stream, it would be 
forced
to also reject the second. The historical intention of using 
"bundle-only" was
as a mechanism for UDP port preservation on the offerer's side. This 
procedure
unintentionally turns it into a semantic binding among streams in bundle.

I think we could restore the original purpose of the bundle-only handling
if we used the port from the rejected 'm=' line containing the BUNDLE-tag
for any 'bundle-only' lines.

--------------------------------------------------------------------------

Section 8.3.4 is structured very awkwardly. It calls out
two conditions that preclude moving an m= line out of a bundle (shared
address, bundle-only), and one that allows it. It would be much clearer
if it were stated in those terms rather than describing the rejection of
a media stream as if it were the same thing as accepting a media stream
with different addressing information.

--------------------------------------------------------------------------

Section 8.4.1:

    When an offerer receives an answer, if the answer contains a BUNDLE
    group, the offerer MUST check that any bundled "m=" line in the
    answer was indicated as bundled in the associated offer.

This is backwards as compared to the logic described in the section.
The remainder of the text talks about checking whether that bundled
"m=" lines in the *offer* appear in the *answer* (and, if they don't,
moving them out of the bundle).

If the answer has a bundle that contains an m-line that wasn't bundled
in the offer, it's an error.

Also, 8.4.1 is the only subsection in 8.4, which is otherwise empty.
I suggest removing 8.4.1, and putting its contents directly in 8.4.

--------------------------------------------------------------------------

The optionality in section 8.5.2 seems unnecessarily complex, and it's
difficult to see what it gains us. Why don't we simply specify that each
pre-existing m= line MUST have a single shared address?

To be clear: if we keep this behavior, then I think we want the answer 
to that
question in the document.

--------------------------------------------------------------------------

Section 8.5.4 should explicitly mention the case in which the line being 
moved
out is the one identified by the BUNDLE-tag. I think all we need to say is
that the BUNDLE-tag for a bundle changes if the corresponding m= line is
removed from the bundle.

--------------------------------------------------------------------------

Section 9.1: "publically" is more conventionally spelled "publicly"

--------------------------------------------------------------------------

Section 10.2:

    An offerer and answerer can in an offer and answer inform each other
    which SSRC values they will use inside sent RTP/RTCP packets, by
    associating one or more SDP 'ssrc' attributes [RFC5576] with each
    bundled "m=" line which contains a payload type value that is also
    used inside another bundled "m=" line.

That's really hard to follow. I propose reformulating along the lines of:

    Peers to a session can inform each other which SSRC values they will use
    for RTP and RTCP through the use of the SDP 'ssrc' attribute [RFC5576].
    To allow for proper association with this mechanism, the 'ssrc' 
attribute
    needs to appear in each "m=" line that shares a payload type with any
    other "m=" line in the same bundle.

Also, doesn't the final paragraph of this section kind of make the rest
of the section a waste of space?

--------------------------------------------------------------------------

Section 10.3.1:

    When a BUNDLE group, which contains RTP based media, is created,...

Change to:

    When a BUNDLE group that contains RTP based media is created,...

--------------------------------------------------------------------------

Section 10.3 (and its subsections) doesn't clearly explain where RTCP 
will be
received in the various described scenarios. In particular, I believe we 
need
to clarify that, while users might indicate a bunch of different rtcp ports
(using the 'rtcp' attribute) in bundled "m=" lines, if the answer accepts an
"m=" line into a bundle, then the rtcp port indicated in the section 
associated
with the selected BUNDLE address will be used.

--------------------------------------------------------------------------

11.2.3:

    When an answerer generates an answer, which contains a BUNDLE group,

Replace with:

    When an answerer generates an answer that contains a BUNDLE group,

--------------------------------------------------------------------------

Section 12, general:

Figuring out what changed in these rather large swaths of text is difficult.
It would be very helpful if these were broken down like:

12.2 Changes to section 5.1 (2nd paragraph) of RFC 3264

   The final sentence of the paragraph is modified to change the meaning
   of port zero.

12.2.1 Original Text
   ...
12.2.2 New Text
   ...
12.3 Changes to section 8.2 (2nd paragraph) of RFC 3264
   ...
12.3.1 Original Text
   ...
12.3.2 New Text
   ...

etc.

--------------------------------------------------------------------------

Section 13.1:

    The
    endpoint informs the remote endpoint about the identification-tag
    using the procedures in [RFC5888], and the remote endpoint then
    inserts the identification-tag in RTCP- and RTP packets sent towards
    the other endpoint.

It's really hard to follow which endpoint is doing what. Perhaps something
like this would be clearer:

    A media recipient informs the media sender about the identification-tag
    associated with an "m=" line through the use of an 'mid' attribute
    [RFC5888]. The media sender then inserts the identification-tag in RTCP
    and RTP packets sent to the media recipient.

--------------------------------------------------------------------------

Section 14:

There is no IANA section that registers the "BUNDLE" token for this 
registry:
http://www.iana.org/assignments/sdp-parameters/sdp-parameters.xhtml#sdp-parameters-13

For sections 14.2 and 14.3, I believe it is traditional for WG items to list
the WG as contact for IANA registrations rather than individual document
authors.

--------------------------------------------------------------------------

I have not reviewed the examples or the appendix.

/a