Subject: Re: [MMUSIC] AD Evaluation of draft-ietf-mmusic-sdp-bundle-negotiation-47 - Ben's editorial comments

From: Christer Holmberg <[email-masked]>
Date: Sun, 21 Jan 2018 19:35:25 +0000

Hi,

In this reply I address the editorial comments. Please see inline.

*** Editorial Comments and nits ***

---

> - Terminology issues: I think some of the terminology here will confuse readers who are new to the concepts. 
> In particular, IIUC, “bundle address” means address + port. I think many readers will fall back to assuming that 
> address means IP address. (Especially when we talk about a “unique address” assigned to an m= section, when 
> much of the time that may just mean a unique port.)

Section 2 does define "bundle address" as an address:port combination.

But, I see your point.

Would "bundle address-port", "bundle addressport", "bundle addrprt", or something similar, be better? I don't want to say "bundle address:port combination" in every instance...

---

> I also find defining the term “Bundle Tag” to mean the first tag in the list to be confusing. It may be easier to just refer to the “first tag in the group list”.

Again, section 2 does define the meaning.

---

> — Examples: Any chance of some of the examples showing IPv6? I suspect it’s a trivial change for at least some examples, since they use domain names.

I can do that.

---

> -abstract: The abstract seems longer and more detailed than strictly necessary. The first two sentences along with a brief mention 
> of the update to 3264 and the SDES item/RTP header extension would be sufficient.

That's fine with me.

I do know that people requested certain things to be in the Abstract, but I assume they will speak up if they think something should be kept. 

---

> -Section 1,
> — first paragraph: This sort of jumps into things without much context. (e.g. that we are talking about media 
> sessions signaled using SDP offer/answer.”

I could say:

"When the SDP offer/answer mechanism [RFC3264] is used to negotiate the establishment of multimedia 
communication sessions, if separate transports (5-tuples) are negotiated for each individual media stream,
each transport consumes additional resources (especially when Interactive Connectivity Establishment
(ICE) [I-D.ietf-ice-rfc5245bis] is used).  For this reason, it is attractive to use a single transport for multiple 
media streams."

> s/consume/consumes

Will be fixed as suggested.

> — paragraphs 5-9: Most of the instances of the word “also” are unneeded.

I can remove them the unneeded ones.

> — paragraphs 7 and 9: These paragraphs both talk about updates to 3264; it would be better to keep them together.

I think they can be combined into a single paragraph.

> — 2nd to last paragraph, “The procedures in this specification apply independently to a given BUNDLE group.” : I don’t 
> understand what that sentence is trying to convey.

I says that, if your SDP contains multiple BUNDLE groups, the procedures are applied independently to each of them.

---

> - Section 2: The terminology list would be more readable as a bullet list or hanging indent list.

I will fix that.

---

> -3: There are at least one lower case “must” and a number of lower case “shoulds”. Please consider 
> using the boilerplate from RFC 8174, or verify that the lower case versions are not intended to be keywords.

I found a couple of "must" instances. Both shall be upper case.

There are 6-7 "should" instances, and it seems all of them can be modified to non-keywords.

---

> -5:
> — First paragraph: inconsistent tense. Please consider s/ “ … section will use …” / “… section uses…”

I will fix as suggested.

> — 2nd paragraph: The citation to RFC5888 looks like a reference for the “BUNDLE” semantic value, rather than
> for the grouping framework. Please consider something to the effect of the following:
> OLD:
>      The BUNDLE extension is indicated using an SDP ’group’ attribute with a "BUNDLE" semantics value [RFC5888].
> NEW:
>     The BUNDLE extension is indicated using an SDP ’group’ attribute with a semantics value [RFC5888] of "BUNDLE”.

I will fix as suggested.

---

> - Section 8.1:
> — Consider moving this section to later in the offer/answer details. I think it would be easier for a reader to 
> understand this section after they have some more general context.

The reason it is in the beginning is because it is part of the generic procedures.

Would you like to place it at the end of Section 8?

> — Title: Spell out “Multiplex” in the title.

I will fix as suggested.

> — First Paragraph: The wording could be taken to mean that all attributes with a mux category of 
> IDENTICAL or TRANSPORT must be included in the first place. I think the point is that, if they are 
> present, they must be included in each m= section. I suggest something like the following:
> OLD:
>    … IDENTICAL and TRANSPORT mux category SDP attributes MUST explicitly be included in each 
> bundled "m=“ section ...
> NEW:
>    … Any IDENTICAL and TRANSPORT mux category SDP attributes included in the BUNDLE group 
> MUST explicitly be included in each bundled "m=“ section …

I will fix as suggested.

---

> -8.2.1, first paragraph:
> — The phrase “represented by the … BUNDLE-tag” occurs repeatedly throughout the document. The use of “represented” there 
> seems awkward. I think “indicated” would be a better word choice.

This is one of the words that have been changed multiple times throughout the work of the document...

I can see whether your suggestion can be done with a search/replace, but if it requires more than that I would not like to do it.

> — should “offerer BUNDLE address” be “offerer suggested BUNDLE address”?

Yes. I will fix as suggested.

---

> -8.5.2: Should “add a bundled m= section” be “add a _new_ bundled m= section”?

Yes. I will fix as suggested.

---

> -9:
> — first paragraph: “ … different protocols on top of the transport-layer protocol …”
> Upper layer protocols?

Yes. Will fix as suggested.

> — Last paragraph: This paragraph seems mostly (or entirely) redundant to section 9.1.

The text gives an overview of what is done in this document, and what needs to be done elsewhere. I'd like to keep it.

---

-10.2:

> — " RTCP packets for which no appropriate "m=" section can be identified
>    MUST be processed as usual by the RTP layer, updating the metadata
>   associated with the corresponding RTP streams, but are not passed to
>   any "m=" section.”
>
> I don’t think things gets passed to sections of SDP. What does this really talk about?

This is another thing that it took forever to agree upon. The idea was to use different "layers" when processing received RTP/RTCP packets.

However, I do agree that the text is a little confusing.

Perhaps something like:

   "RTCP packets that cannot be associated with an appropriate "m=" section 
   MUST still be processed as usual by the RTP layer, updating the metadata
   associated with the corresponding RTP streams."

...and in the prior paragraph:

   "For each RTCP packet received (including each RTCP packet that is
   part of a compound RTCP packet), the packet is processed as usual by
   the RTP layer, then associated with the appropriate "m=" sections, and
   processed for the RTP streams represented by those "m=" sections."

> — “ Rules for additional processing of the various types of RTCP packets
>   are explained below.”
>
> Should that say “Additional rules for processing…”?

Yes. I will fix as suggested.

---

> -11, 2nd bullet: " previously selected [Section 8.3.1 ] or new suggested [ Section 8.5.1]”
> s/new/newly

Will fix as suggested.

> — last paragraph: It may be worth clarifying that this doesn't change any requirement specified by the 
> application or signaling protocol. For example, WebRTC still requires ICE even though bundle is used.

Question for clarification: Do you want to say that some applications might mandate ICE even if BUNDLE is not used, or that some applications might mandate support of ICE together with BUNDLE?

---

> 11.1:
> — 2nd paragraph: “ previously selected or new suggested “ s/new/newly

Will fix as suggested.

> — 3rd paragraph: Construct of the form of “MUST only” can be ambiguous. Please consider restating 
> in the form of a MUST NOT construction. (For example, “MUST only do foo when….” can be interpreted 
> as “MUST NOT do foo unless…” or “MUST NOT do anything but foo when…”   [Note: This construct repeats 
> several times in the next few sections. ]

What about?
 
   "When an answerer assigns a BUNDLE address to an "m=" section within a
   BUNDLE group (the "m=" section represented by the answerer BUNDLE-
   tag), the answerer onlys include SDP 'candidate' attributes (and
   other applicable ICE-related media-level SDP attributes) in that "m="
   section, following the procedures in Section 8.1. The answerer MUST NOT
   include ICE-related media-level SDP attributes in any other "m=" sections."

---

> -19, 3rd paragraph from end: It’s seems a bit odd to thank a co-author (Cullen) for text contributions :-)

Ok, I will remove him from section 19 :)

Regards,

Christer

---

From: Ben Campbell <[email-masked]>
Date: Thu, 25 Jan 2018 17:15:10 -0600

> On Jan 21, 2018, at 1:35 PM, Christer Holmberg <[email-masked]> wrote:
> 
> Hi,
> 

Yes, it does—but the redefinition of a well-known term often fails to stick in people’s heads.

> 
> But, I see your point.
> 
> Would "bundle address-port", "bundle addressport", "bundle addrprt", or something similar, be better? I don't want to say "bundle address:port combination" in every instance…

I struggled with a suggestion when I originally wrote the comment, and didn’t come up with something I liked. “Transport” seems descriptive, but there’s similarly well-known usage of that that doesn’t quite match. Maybe “bundle address:port”?

> Again, section 2 does define the meaning.

Again, memory of definitions may tend to slip by the time one finishes a 67 page document. But I’m okay leaving this one as is, since it doesn’t collide with people’s preconceived definitions of something.

> I can do that.

I think that would be worthwhile. The lack of IPv6 examples in drafts has come up in IESG review recently.
> media streams.”
That helps, thanks.

[…]

> I says that, if your SDP contains multiple BUNDLE groups, the procedures are applied independently to each of them.

How about “If an SDP offer or answer contains multiple BUNDLE groups, the procedures in this specification apply to each group independently.”?

[…]

> Would you like to place it at the end of Section 8?

Hrm. I guess that’s not great either, and I understand your point. I withdraw the comment.

[…]

> I can see whether your suggestion can be done with a search/replace, but if it requires more than that I would not like to do it.

Okay

[…]

> The text gives an overview of what is done in this document, and what needs to be done elsewhere. I'd like to keep it.

Okay.

>   processed for the RTP streams represented by those "m=" sections.”

I was going to suggest changing “passed to” to “associated with” in the first paragraph, but I like your placement of that better.

[…]

> Question for clarification: Do you want to say that some applications might mandate ICE even if BUNDLE is not used, or that some applications might mandate support of ICE together with BUNDLE?

The latter..

[…]

>   include ICE-related media-level SDP attributes in any other "m=" sections.”

WFM.

[…]

Thanks!

Ben.

---

From: Christer Holmberg <[email-masked]>
Date: Fri, 26 Jan 2018 12:10:21 +0000

Hi,

Please see inline.

>>> In particular, IIUC, ³bundle address² means address + port. I think
>>>many readers will fall back to assuming that
>>> address means IP address. (Especially when we talk about a ³unique
>>>address² assigned to an m= section, when
>Yes, it does‹but the redefinition of a well-known term often fails to
>stick in people¹s heads.
>
>> 
>> But, I see your point.
>> 
>> Would "bundle address-port", "bundle addressport", "bundle addrprt", or
>>something similar, be better? I don't want to say "bundle address:port
>>combination" in every instanceŠ
>
>I struggled with a suggestion when I originally wrote the comment, and
>didn¹t come up with something I liked. ³Transport² seems descriptive, but
>there¹s similarly well-known usage of that that doesn¹t quite match.
>Maybe ³bundle address:port²?

It has previously been suggested to use ³BUNDLE transport², but that could
mean that other things have to be changed too. ³BUNDLE address:port² would
be a smaller and "safer" change, so if you are ok with that...

---

>>> ‹ 2nd to last paragraph, ³The procedures in this specification apply
>>>independently to a given BUNDLE group.² : I don¹t
>> I says that, if your SDP contains multiple BUNDLE groups, the
>>procedures are applied independently to each of them.
>
>How about ³If an SDP offer or answer contains multiple BUNDLE groups, the
>procedures in this specification apply to each group independently.²?

Ok. I will fix as suggested.

---

>> 
>>> -8.2.1, first paragraph:
>>> ‹ The phrase ³represented by the Š BUNDLE-tag² occurs repeatedly
>>>throughout the document. The use of ³represented² there
>>> seems awkward. I think ³indicated² would be a better word choice.
>> 
>> I can see whether your suggestion can be done with a search/replace,
>>but if it requires more than that I would not like to do it.
>> 
>
>Okay

I had a quick look, and I think I could replace with "indicated".

---

>>> ‹ last paragraph: It may be worth clarifying that this doesn't change
>>>any requirement specified by the
>> Question for clarification: Do you want to say that some applications
>>might mandate ICE even if BUNDLE is not used, or that some applications
>>might mandate support of ICE together with BUNDLE?
>
>The latter..

What about:

"Support and usage of ICE mechanism together with the BUNDLE extension
   is OPTIONAL, and the procedures in this section only apply when the
   ICE mechanism is used. Note that applications might mandate usage
of the ICE mechanism even if the BUNDLE extension is not used."

Regards,

Christer

---

From: Ben Campbell <[email-masked]>
Date: Mon, 29 Jan 2018 15:21:37 -0600

> On Jan 26, 2018, at 6:10 AM, Christer Holmberg <[email-masked]> wrote:
> 
> Hi,
> 
> Please see inline.
> 
>>>> In particular, IIUC, łbundle address˛ means address + port. I think
>>>> address means IP address. (Especially when we talk about a łunique
>>>> address˛ assigned to an m= section, when
>> stick in peopleąs heads.
>> didnąt come up with something I liked. łTransport˛ seems descriptive, but
>> thereąs similarly well-known usage of that that doesnąt quite match.
>> Maybe łbundle address:port˛?
> 
> It has previously been suggested to use łBUNDLE transport˛, but that could
> mean that other things have to be changed too. łBUNDLE address:port˛ would
> be a smaller and "safer" change, so if you are ok with that…
> 

Okay

[…]

WFM.

Thanks!

Ben.