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

[Christer Holmberg <[e-mail]>]

Hi Adam,

My reply to your editorial comments.

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

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.

I suggest to remove it.


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.

In the Introduction, I suggest to modify the text to:

    "This specification also updates sections 5.1, 8.1 and 8.2 of RFC 3264
    [RFC3264].  The update allows an answerer, under certain
    circumstances described in the specification, 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..."

I'll fix as suggested.

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

> 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 reviewed the BFCFbis draft last week, and that uses "m line" terminology, without any definition, and I believe it exists also in other drafts.

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

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

I'll fix as suggested.

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

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)

Sections 5 and 8.3.3 talk about different things:

Section 5 talks about the address:port used for sending media, while section 8.3.3 talks about the address:port used for receiving media (i.e. the address:port included in SDP).

We could perhaps remove the restriction saying that a single address:port can only be used for sending media associated with a single BUNDLE group (section 5). 

But, I don't think we should allow usage of a single address:port for receiving media associated with multiple BUNDLE groups (and possible non-bundle "m=" lines), because even without BUNDLE each "m=" line must have a unique address:port for receiving media.

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

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

I'll fix as suggested.

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

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.

People have previously said that we should have a dedicated section where the attribute is described.

And, as there has been a number of restructure changes throughout the process of the draft, so I'd like to keep it as it is.

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

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

I'll fix as suggested.

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

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.

That is what we mean.

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

Are you sure you mean 8.3.1? That is about generating the answer.

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

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

I can fix as suggested, if it makes things more clear. However, the SDP group attribute is only defined as a session-level attribute to begin with.

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

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

I'll fix as suggested.

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

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

Perhaps something like:

"8.3.4.  Moving A Media Description Out Of A BUNDLE Group

  When an answerer wants to move a "m=" line out of a BUNDLE group, it
  MUST check whether, in the associated offer, the "m=" line
  contains a unique address. If so, when the answerer moves the
   "m=" line out of the BUNDLE group, it MUST assign a
  unique address to the corresponding "m=" line in the answer.

  In the following cases, the "m=" line cannot simply be moved out of
  a BUNDLE group, but MUST also reject the "m=" line [Section 8.3.5]:

   o  In the associated offer, if the "m=" line contains a shared
      address (e.g. a previously selected offerer BUNDLE address), the
      answerer MUST reject the moved "m=" line; 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.

   In addition, in either case above, the answerer MUST NOT place the
   identification-tag, associated with the moved/rejected "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.
>
> 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 as suggested.

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

> 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 could add the following note:

	"NOTE: If the offerer BUNDLE-tag represents the moved "m=" line, the offerer needs 
	to select a new offerer BUNDLE-tag, which represents a "m=" line that is kept within
	the BUNDLE group."

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

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

I'll fix as suggested.

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

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?

I have been trying hard to use consistent terminology throughout the document, so I would like to try to stick to that.

So, perhaps something like:

	"An offerer and answerer can inform each other which SSRC values they will use inside 
	RTP/RTCP packets sent to each other. This is done by associating one or more SDP 'ssrc' 
	attributes [RFC5576] with each bundled "m=" line that contains a payload type value that 
	it shares with any other "m=" line within the same BUNDLE group."

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

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

I'll fix as suggested.

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

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

For the answerer, section 10.3.2.3 says:

	"If the answerer does not accept the usage of RTP/RTCP multiplexing
	within the BUNDLE group, it MUST NOT associate an SDP 'rtcp-mux'
	attribute with any bundled "m=" line in the answer.  The answerer
	will use the RTP and RTCP port values associated with the selected
	offerer BUNDLE address for sending RTP and RTCP packets associated
	with each RTP-based bundled "m=" line towards the offerer."

For the offerer, section 10.3.2.4 says:

	"If the answerer associated an SDP 'rtcp' attribute with the "m=" line
   	representing the answerer BUNDLE address, the offerer will use the
   	attribute port value for sending RTCP packets associated with each
   	bundled RTP-based "m=" line towards the answerer.  Otherwise the
   	offerer will use the next higher port value associated with the
   	answerer BUNDLE address for sending RTCP packets towards the
   	answerer."

So, I think it is indicated that the RTCP port is derived from the selected BUNDLE address (offerer or answerer).

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

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

I'll fix as suggested.

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

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

People have had different opinions - and IESG will have their opinion.

So, my suggestion would be to NOT change anything at this point.

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

> 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 am using "endpoint" terminology throughout the document. 

But, perhaps something like:

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

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

> 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 took some random attributes from the IANA SDP attribute registry list, and in the associated RFCs they all had individuals listed as contacts.

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

> I have not reviewed the examples or the appendix.

Depending on what changes we do, e.g. related to suggesting a new offerer BUNDLE address, I'll check that the examples are still aligned.

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

Regards,

Christer