[MMUSIC] Eric Rescorla's Discuss on draft-ietf-mmusic-sdp-bundle-negotiation-49: (with DISCUSS and COMMENT)

[Eric Rescorla <[e-mail]>]

Eric Rescorla has entered the following ballot position for
draft-ietf-mmusic-sdp-bundle-negotiation-49: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-mmusic-sdp-bundle-negotiation/



----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

Rich version of this review at:
https://mozphab-ietf.devsvcdev.mozaws.net/D3382









DETAIL
>      When an offerer generates an offer, in which it wants to add a new
>      bundled "m=" section, the offerer MUST:
>
>      o  Assign the offerer BUNDLE address:port (previously selected
>         [Section 8.3.1] or newly suggested [Section 8.5.1]) to the added
>         "m=" section; or

IMPORTANT: This doesn't sound right. You can't use the existing
address:port, because if the peer rejects BUNDLE but accepts the m=
section then it's broken.


>      o  When the BUNDLE transport has been established, ICE connectivity
>         checks and keep-alives only need to be performed for the BUNDLE
>         transport, instead of per individual "m=" section within the
>         BUNDLE group.
>
>      o  In an offer, if the offer assigns a unique address:port to one or

IMPORTANT: This does not define how to interact with trickle ICE.


----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

>
>    Negotiating Media Multiplexing Using the Session Description Protocol
>                                    (SDP)
>               draft-ietf-mmusic-sdp-bundle-negotiation-49.txt
>
>   Abstract

This document is quite a bit improved from when I read it before, but
it's still very confusing. The primary problem here is the repeated
use of "address;port" as synechdoche for the m= section which is the
"head" of the BUNDLE group (i.e., that which is indicated by the
BUNDLE-tag). That's not what's going on and it's misplaced
concreteness. I would invent some new term ("head" is actually not
bad) and then use it throughout for this concept.


>      to negotiate which "m=" sections will become part of a BUNDLE group.
>      Within a BUNDLE group, each "m=" section will use a BUNDLE transport
>      for sending and receiving bundled media.
>
>      Within a BUNDLE group, each endpoint uses a single address:port
>      combination for sending and receiving bundled media.  The

Nit: "pair"?


>      combination for sending and receiving bundled media.  The
>      address:port combination is referred to as the BUNDLE address:port.
>      In addition to negotiating the BUNDLE group, the offerer and answerer
>      [RFC3264] use the BUNDLE extension to negotiate the BUNDLE
>      addresses:ports, one for the offerer (offerer BUNDLE address) and one
>      for the answerer (answerer BUNDLE address:port).  Once the offerer

Why is one of these called "address" and the other "address:port"?


>      can be used to request that specific media is only used if the "m="
>      section describing the media is kept within a BUNDLE group.
>
>      This specification updates RFC 3264 [RFC3264], to allow assigning a
>      zero port value to a "m=" section without meaning that the media
>      described by the "m=" section is disabled or rejected.

1. Bundle lets you have multiple SDP m= sections on a single 5-tuple
2. The underling mechanism here is that you put those m= sections in
the same BUNDLE group
3. In order to facilitate demuxing, we define a new RTP extension for
MID.
4. It is sometimes desirable to say that an m= section should only be
negotiated if it can be bundled. This is done with port=0 and a
=bundle-only.



>      can be used to request that specific media is only used if the "m="
>      section describing the media is kept within a BUNDLE group.
>
>      This specification updates RFC 3264 [RFC3264], to allow assigning a
>      zero port value to a "m=" section without meaning that the media
>      described by the "m=" section is disabled or rejected.

This section is kind of a laundry list. I think it would be helpful to
break it out conceptually. Specifically.

1.
2.
3.



>
>      "m=" section: SDP bodies contain one or more media descriptions,
>      referred to as "m=" sections.  Each "m=" section is represented by an
>      SDP "m=" line, and zero or more SDP attributes associated with the
>      "m=" line.  A local address:port combination is assigned to each "m="
>      section.

Was this intended to be a bullet?


>      o  Bundled media: All media associated with a given BUNDLE group.
>
>      o  Initial offer: The first offer, within an SDP session (e.g. a SIP
>         dialog when the Session Initiation Protocol (SIP) [RFC3261] is
>         used to carry SDP), in which the offerer indicates that it wants
>         to create a given BUNDLE group.

This is a very idiosyncratic definition, because it's *not* the first
offer in the session, but the one where the BUNDLE group is
introduced. Given that 3264 has a different meaning for this same
term, and the difference is critical, I would strongly urge you to
pick a different term.


>      The BUNDLE extension is indicated using an SDP 'group' attribute with
>      a semantics value [RFC5888] of "BUNDLE".  An identification-tag is
>      assigned to each bundled "m=" section, and each identification-tag is
>      listed in the SDP 'group:BUNDLE' attribute identification-tag list.
>      Each "m=" section whose identification-tag is listed in the
>      identification-tag list is associated with a given BUNDLE group.

How about "is part of" instead of "associated with"


>      Each "m=" section whose identification-tag is listed in the
>      identification-tag list is associated with a given BUNDLE group.
>
>      SDP bodies can contain multiple BUNDLE groups.  Any given bundled
>      "m=" section MUST NOT be associated with more than one BUNDLE group
>      at any given time.

Again "part of"


>      assign a zero port value to the "m=" section.  According to [RFC3264]
>      an answerer will reject such an "m=" section.  By including an SDP
>      'bundle-only' attribute in such an "m=" section, the offerer can
>      request that the answerer accepts the "m=" section if the answerer
>      supports the BUNDLE extension, and if the answerer keeps the "m="
>      section within the associated BUNDLE group.

I would put some of this text up before the attribute definition to
explain why the attribute exists.


>
>      SDP offers and answers can contain multiple BUNDLE groups.  The
>      procedures in this section apply independently to a given BUNDLE
>      group.
>
>   8.1.  Multiplex Category Considerations

Multiplexing?


>      When a BUNDLE group is initially negotiated, and a unique
>      address:port is assigned to each bundled "m=" section (excluding any
>      bundle-only "m=" section) in the initial offer [Section 8.2], any
>      IDENTICAL and TRANSPORT mux category SDP attributes included in the
>      BUNDLE group MUST explicitly be included in each bundled
>      "m=a€œ section (excluding any bundle-only "m=" sections).

Something bad happened here with your editor.


>      BUNDLE group MUST explicitly be included in each bundled
>      "m=a€œ section (excluding any bundle-only "m=" sections).
>
>      When an offerer or answerer includes SDP attributes in bundled "m="
>      sections within a BUNDLE group for which the offerer and answerer
>      BUNDLE addresses:ports have been selected, IDENTICAL and TRANSPORT

This is pretty opaque. It's not that the address:port pair have been
selected but rather that BUNDLE has been negotiated for that m=
section (though of course the address:port being selected is a side
effect of this).




>      The semantics of some SDP attributes only apply to specific types of
>      media.  For example, the semantics of the SDP 'rtcp-mux' and SDP
>      'rtcp-mux-only' attributes only apply to "m=" sections describing
>      RTP-based media.  However, as described in Section 8.1, there are
>      cases where IDENTICAL and TRANSPORT mux category SDP attributes are
>      only included in the "m=" sections indicated by the BUNDLE-tag.  That

Huh? This is section 8.1.

This whole paragraph is a mess. I would say

"One consequence of these rules is that because SDP attributes which
are appropriate only to some other m= section may appear in the m=
section indicated by the BUNDLE-tag. For instance, the BUNDLE-tag may
indicate a data channel but contain the 'rtcp-mux' attribute because
that applies to an RTP m= section that is part of the BUNDLE group"




>      type of media.
>
>   8.2.  Generating the Initial SDP Offer
>
>      When an offerer generates an initial offer, to negotiate a BUNDLE
>      group, it MUST:

This sentence is ungrammatical. "In order to negotiate a BUNDLE group
in an initial offer, the offerer MUST"


>      within the BUNDLE group, the offerer MUST:
>
>      o  Include an SDP 'bundle-only' attribute [Section 8.2.1] in the "m="
>         section; and
>
>      o  Assign a zero port value to the "m=" section.

I would reorder these, because the zero port is the main semantic.


>
>      NOTE: If the offerer assigns unique addresses:ports to multiple
>      bundled "m=" sections, the offerer needs to be prepared to receive
>      bundled media on each unique address:port, until it receives the
>      associated answer and finds out which address:port combination has
>      been selected as the offerer BUNDLE-address.

Why is this an "If"? There are only two choices: "unique" and "zero".
Just put this graf up before the discussion of bundle-only and you can
remove the conditional


>
>      It is RECOMMENDED that the offerer assigns the suggested offerer
>      BUNDLE address:port to a bundled "m=" section that the offerer
>      assumes it is unlikely that the answerer will reject, or move out of
>      the BUNDLE group.  How such assumption is made is outside the scope
>      of this document.

I would say "believes" rather than "assumes"


>        b=AS:1000
>        a=mid:bar
>        a=rtcp-mux
>        a=rtpmap:31 H261/90000
>        a=rtpmap:32 MPV/90000
>        a=extmap 1 urn:ietf:params:rtp-hdrext:sdes:mid

Line breaks between the m= sections would help here.


>
>      When an answerer generates an answer that contains a BUNDLE group,
>      the following general SDP grouping framework restrictions, defined in
>      [RFC5888], also apply to the BUNDLE group:
>
>      o  The answerer MUST NOT include a BUNDLE group in the answer, unless

Comma not needed here. Also, this negative phrasing is hard to follow.
I would say "MUST only ... if" here and below


>      o  The answerer MUST NOT include an "m=" section within a BUNDLE
>         group, unless the offerer requested the "m=" section to be within
>         that BUNDLE group in the corresponding offer.
>
>      o  If the answer contains multiple BUNDLE groups, the answerer MUST
>         NOT move an "m=" section from one BUNDLE group to another.

This bullet is unnecessary as it immediately follows from the
preoceeding bukllet.


>
>      If the answer contains a BUNDLE group, the answerer MUST:
>
>      o  Select an offerer BUNDLE address:port [Section 8.3.1]; and
>
>      o  Select an answerer BUNDLE address:port [Section 8.3.2].

This isn't right, because each BUNDLE group has its own address:port,
so instead "For each BUNDLE group" contained in the offer.

More importantly, it's not the address:port that the answerer chooses,
but rather the m= section. I.e.,

o Determine the first m= section in the BUNDLE group which will be
BUNDLEd in the answer
o Select an answerer BUNDLE address:port


>      o  The answerer will not reject the "m=" section [Section 8.3.4]; and
>
>      o  The "m=" section does not contain a zero port value.
>
>      If all of the criteria above are fulfilled, the answerer MUST select
>      the suggested offerer BUNDLE address:port.

Again, this isn't selecting the address:port but rather the m=section


>      bundled "m=" section the answerer MUST assign a zero port value and
>      include an SDP 'bundle-only' attribute.
>
>      The answerer MUST NOT assign an answerer BUNDLE address:port to an
>      "m=" section that is not within the BUNDLE group, or to an "m="
>      section that is within another BUNDLE group.

This repeats text in S 8.3.


>      group in an answer, it MUST first check the following criteria:
>
>      o  In the corresponding offer, an offerer BUNDLE address:port
>         (previously selected [Section 8.3.1] or newly suggested
>         [Section 8.5.1]) has been assigned to the "m=" section by the
>         offerer; or

This text is baffling. As far as I can see, the first condition is
"don't pick it as the head m= section if you intend to move it out".
The second is some entirely different case. Can you rewrite it so it
makes sense.


>      either reject the whole offer, reject each bundled "m=" section
>      within the BUNDLE group [Section 8.3.4], or keep the "m=" section
>      within the BUNDLE group in the answer and later create an offer where
>      the "m=" section is moved out of the BUNDLE group [Section 8.5.3].
>
>      When the answerer generates an answer, in which it moves a bundled

No comma


>
>      o  MUST NOT assign an SDP 'bundle-only' attribute to the "m="
>         section.
>
>      An answerer MUST NOT move an "m=" section from one BUNDLE group to
>      another within an answer.  If the answerer wants to move an "m="

This is the third time you have said this. If you want to say it here,
say "Because ..., if"


>      it MUST first check the following criteria:
>
>      o  In the corresponding offer, an offerer BUNDLE address:port
>         (previously selected [Section 8.3.1] or newly suggested
>         [Section 8.5.1]) has been assigned to the "m=" section by the
>         offerer.

Same comment here as above. These are entirely different conditions.


>      o  In the corresponding offer, an offerer BUNDLE address:port
>         (previously selected [Section 8.3.1] or newly suggested
>         [Section 8.5.1]) has been assigned to the "m=" section by the
>         offerer.
>
>      If the criteria above is fulfilled, the answerer can not reject the

"criteria... are"


>      reject the whole offer, reject each bundled "m=" section within the
>      BUNDLE group, or keep the "m=" section within the BUNDLE group in the
>      answer and later create an offer where the "m=" section is disabled
>      within the BUNDLE group [Section 8.5.4].
>
>      When an answerer generates an answer, in which it rejects a bundled

No comma


>        m=video 0 RTP/AVP 32
>        b=AS:1000
>        a=mid:bar
>        a=bundle-only
>        a=rtpmap:32 MPV/90000
>        a=extmap 1 urn:ietf:params:rtp-hdrext:sdes:mid

Please put line breaks in between m= sections here too


>   8.4.  Offerer Processing of the SDP Answer
>
>      When an offerer receives an answer, if the answer contains a BUNDLE
>      group, the offerer MUST check that any bundled "m=" section in the
>      answer was indicated as bundled in the corresponding offer.  If there
>      is no mismatch, the offerer MUST use the offerer BUNDLE address:port,

address:port, again. It's the m=section that's relevant.


>      bundled "m=" section.
>
>      NOTE: As the answerer might reject one or more bundled "m=" sections,
>      or move a bundled "m=" section out of a BUNDLE group, each bundled
>      "m=" section in the offer might not be indicated as bundled in the
>      answer.

"each" -> "an given"


>      When the offerer generates a subsequent offer, the offerer BUNDLE-tag
>      MUST represent the bundled "m=" section to which the offerer BUNDLE
>      address:port (previously negotiated or newly suggested) has been
>      assigned.
>
>   8.5.1.  Suggesting a New Offerer BUNDLE Address:Port

Again it would make more sense to describe this as suggesting a new m=
section.


>      section out of a BUNDLE group.
>
>   8.5.4.  Disabling a Media Description in a BUNDLE Group
>
>      When an offerer generates an offer, in which it wants to disable a
>      bundled "m=" section, the offerer:

Are you sure disable -> port 0, as opposed to "inactive"


>      configuration and packetization.
>      [I-D.ietf-mmusic-sdp-mux-attributes] lists SDP attributes, whose
>      attribute values are required to be identical for all codecs that use
>      the same payload type value.
>
>   10.2.  Associating RTP/RTCP Streams with Correct SDP Media Description

Nit "the correct"


>      "m=" sections), following the procedures for IDENTICAL mux category
>      attributes in Section 8.1.  In addition, the offerer MAY include an
>      SDP 'rtcp-mux-only' attribute [I-D.ietf-mmusic-mux-exclusive] in a
>      RTP-based bundled "m=" section.
>
>      NOTE: Whether the offerer associates the SDP 'rtcp-mux-only'

associates -> includes


>         an offerer BUNDLE address:port (previously selected
>         [Section 8.3.1] or newly suggested [Section 8.5.1]) to a bundled
>         "m=" section (the "m=" section indicated by the offerer BUNDLE-
>         tag), the offerer only includes ICE-related media-level SDP
>         attributes in that "m=" section, following the procedures in
>         Section 8.1.

You should rewrite this graf and the one below to be about BUNDLED-
ness not port assignment. I.e., if you are definitely bundled, willing
to bundle, or not bundled.


>      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.
>
>   11.1.  SDP Offer/Answer Procedures

Same point here as above. Talking about address:port isn't helpful.