Subject: Re: [MMUSIC] WGLC for draft-ietf-mmusic-sdp-bundle-negotiation-19 - Adam's editorial comments

From: Christer Holmberg <[email-masked]>
Date: Thu, 11 Jun 2015 19:14:56 +0000

Hi,

Below I am addressing Adam's editorial comments on BUNDLE-19.

(The technical comments were addressed in a separate e-mail)

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

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

I'll remove the text from the Abstract.

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

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

Will be fixed.

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

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

I suggest to add the following text to the Terminology section:

	"m=" line: SDP bodies contain one or more media descriptions. Each media description is identified by an SDP "m=" line.

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

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

I'll add a reference to RFC 4961.

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

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

I believe this has been discussed in the past. I am happy to remove the requirement from section 5. If someone disagree, please speak up NOW.

However, I don't think it has impacts on 8.3.3, because that section only says that a given BUNDLE address (which indicates where media is RECEIVED) must not be assigned to "m=" lines that are not part of the given BUNDLE group, and your suggested change does not impact that. 

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

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

Will be fixed.

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

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

I believe Flemming(?) wanted to have it in section 6, in order to avoid forward referencing when we describe the usage of the attribute in section 8.

So, while I do hear what you are saying, I really don't want to do the change. I have re-structured the draft a number of times, based on previous comments, so at this point I think we should keep things as they are. 

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

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

Will be fixed.

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

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

It is not about knowing that the answerer supports BUNDLE - it's about ensuring that a new "m=" line will either be accepted into the "m=" line, or completely rejected", by the answerer - without using the 'bundle-only' attribute.

But, I do agree that the current note text is difficult to understand, so what about:

       NOTE: Once the offerer BUNDLE address has been selected, the offerer does
       not need to include the 'bundle-only' attribute in subsequent offers. By 
       assigning the offerer BUNDLE address to an "m=" line of a subsequent offer, 
       the offerer will ensure that the answerer will either keep the "m=" line within 
       the BUNDLE group, or the answerer will have to reject the "m=" line.

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

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

I'll remove it.

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

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

Do we really need to say that? The SDP group attribute is only defined for the session level, as far as I know.

I suggest to leave the text as it is.

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

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

Will be fixed.

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

<Technical comment - addressed in separate e-mail>

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

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

What about the following:

8.3.4.  Moving A Media Description Out Of A BUNDLE Group

  When an answerer wants to move an "m=" line out of a BUNDLE group,
  it MUST first check the following criteria: 

   o  In the associated offer, the "m=" line contains a shared
      address (e.g. a previously selected offerer BUNDLE address); or

   o  In the associated offer, if an SDP 'bundle-only' attribute is
      associated with the "m=" line, and if the "m=" line contains a
      zero port value.

  If either criteria above is fulfilled, the answerer MUST reject the "m=" line
  [Section 8.3.5].

  Otherwise, if in the associated offer the "m=" line contains
  a unique address, the answerer MUST assign a unique address to the
  "m=" line in the answer (the answerer does not reject the "m=" line).

   In addition, in either case above, the answerer MUST NOT place the
   identification-tag, associated with the moved "m=" line, in the SDP
   'group' attribute identification-tag list associated with the BUNDLE
   group.

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

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

Correct, and that is what the text is trying to say.

Are you suggestion that we add text explicitly stating the error case?

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

I'll fix that. There have probably been more 8.4.x sections in the past.

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

<Technical comment - addressed in separate e-mail>

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

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

I suggest the following:

  NOTE: If the removed "m=" line is associated with the previously selected BUNDLE-tag, a
  new BUNDLE-tag needs to be selected [8.2.2].

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

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

Will be fixed.

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

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

Looks good, but I suggest to say offerer and answerer, as that is used mostly elsewhere.

      An offerer and answerer can inform each other which SSRC values they will use
      or RTP and RTCP by using the SDP 'ssrc' attribute [RFC5576].
      To allow for proper association with this mechanism, the 'ssrc' 
      attribute needs to be associated with 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?

You are basically saying that we don't need the SSRC values to begin with, as we will transport the identification-tag in the RTP/RTCP packets?

If so, that is an interesting question :) I'd like to hear what the RTP folks (Magnus, Colin, Jonathan, Roni etc) think about that.

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

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

Will be fixed.

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

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

What about the following:

10.3.1.  General

   When a BUNDLE group, which contains RTP based media, is created, the
   offerer and answerer MUST negotiate whether to enable RTP/RTCP
   multiplexing for the RTP based media associated with the BUNDLE group
   [RFC5761].

   If RTP/RTCP multiplexing is enabled, the same address:port combination
   will be used for receiving (and sending) all RTP packets and the RTCP
   packets associated with the BUNDLE group. Each endpoint will send 
   the packets towards the BUNDLE address of the other endpoint.

   If RTP/RTCP multiplexing is not enabled, separate address:port
   combinations will be used for receiving (and sending) the RTP packets
   and the RTCP packets. If the remote endpoint has associated an SDP
   'rtcp' attribute with the "m=" line associated with the BUNDLE-tag,
   the attribute value will be used for sending all RTCP packets associated 
   with the BUNDLE group towards that endpoint.  

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

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

Will be fixed.

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

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

My suggestion would be to keep the text as it is, because depending on which people perform the IESG review I may have to change it anyway.

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

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

I'll fix that.

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

> 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

Will be added.

(Nils Ohlmeier had the same comment.)

> 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 had a look in some RFCs defining SDP attributes. In some cases there was no contact, and in some cases there were document authors. None of the documents I looked at had a WG as contact, but they may exist (I only checked a few RFC). 

So, I think it is ok to list authors, and I suggest to keep the text as it is.

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

Thanks!

Regards,

Christer