Re: [MMUSIC] draft-ietf-mmusic-sdp-mux-attributes-05 review

[Ari Keränen <[e-mail]>]

Hi Suhas,

The updates you propose below look good to me and thanks for the 
clarifications too.


Cheers,
Ari

On 11/12/14 09:47, Suhas Nandakumar wrote:
> Hello Ari
>
>    Many thanks for the review. Please see inline
>
> Thanks
> Suhas
>
> On Wed, Dec 10, 2014 at 8:27 PM, Ari Keränen <[e-mail]
> <mailto:[e-mail]>> wrote:
>
>     Hi Suhas,
>
>     Here are my review comments on the SDP mux draft. Technically I
>     think the doc is in pretty good shape, so these are mostly editorial
>     issues.
>
>
>     Cheers,
>     Ari
>
>
>     Abstract
>
>         In the RTCWEB WG, there is a need to use a
>         single 5-tuple for sending and receiving media associated with
>         multiple media descriptions ("m=" lines).
>
>     Bundling is not RTCWEB-only feature, so I would rather drop the
>     RTCWEB reference at least from the abstract, and write it in more
>     general way (e.g., "in order to save resources and decrease
>     connection setup times, these is a need..." like in section 3)
>
>
>         Such a requirement has
>         raised concerns over the semantic implications of the SDP attributes
>         associated with the RTP Media Streams multiplexed over a single
>         underlying transport layer flow.
>
>     I think we are beyond "raised concerns" and have actually found out
>     it was a real issue, so I would update the wording here.
>
>
> [Suhas] - I will try to make it more general in the next version.
>
>
>         The scope of this specification is to provide a framework for
>         analyzing the multiplexing characteristics of SDP attributes.
>
>     s/scope/purpose/
>
> [Suhas] - Accepted
>
>     1.  Introduction
>
>         Real-Time Communication in WEB-browsers (Rtcweb) framework requires
>         Real-time Transport Protocol (RTP) as the media transport protocol
>         and Session Description Protocol (SDP) [RFC4566] for describing and
>         negotiating multi-media communication sessions.
>
>     This (and the following paragraph) should also be more general. Here
>     perhaps you can use RTCWEB as an example of where such feature is
>     required, but adding an RTCWEB reference (e.g. the use case doc)
>     would be good here for context.
>
>     I think it would make sense also to reference the bundle draft
>     already here and writing a sentence or two about the relationship
>     with this draft.
>
>     also:
>       s/requires/uses/
>       s/multi-media/multimedia/
>
> [Suhas] - Accpted
>
>
>     3.  Motivation
>
>         This imposes further restrictions on applicability of
>         these SDP attributes as they are defined today.
>
>     Not sure where "these" here refers to. I guess that word can either
>     be dropped, or clarify which attributes you mean.
>
>
> [Suhas] - removed the word "these" for clarity.
>
>
>     4.  SDP Attribute Analysis Framework
>
>     Would it make sense to have "before" and "after" in all the SDP
>     examples (now there's only the "before" part)?
>
>
> [Suhas] - I am not sure giving before and after examples be less
> confusing. Since we are not defining new way of doing things to contrast
> the approaches.
>
>
>     4.2.  Category: NOT RECOMMENDED
>
>     Can/should we recommend here what to do in this case? Not multiplex?
>     Not add such attributes? Either way?
>
>
> [Suhas] - the definition for the category already encompasses this as
> highlighted below.
> "Attributes that are *recommended against multiplexing* since their
> usage under multiplexing might lead to incorrect behavior."
>
>
>     4.3.  Category: IDENTICAL
>
>         Attributes that MUST be identical across all the media descriptions
>         being multiplexed.
>
>     Maybe clarify that attributes and their *values* (if exist) must be
>     identical.
>
>
>              <allOneLine>
>              a=zrtp-hash:1.10 fe30efd02423cb054e50efd0248742__ac7a52c8f91bc2
>              df881ae642c371ba46df
>              </allOneLine>
>
>     The draft uses now 3 different ways of presenting too long SDP lines
>     (see e.g., 4.5, 4.7 and 14.3.1.1). I guess they are from the
>     defining RFCs, but I would rather see uniform style across this
>     document. Personally I prefer the style of section 4.5 (2 char indent).
>
> [Suhas] - Will move all the examples to reflect the format in Section 4.5
>
>     4.4.  Category: SUM
>
>     Should clarify where do you put the SUM'ed value.
>
>
> [Suhas] .. Since the calculated SUM value is expected to be consumed in
> the software processing SDP, the definition of this category already
> captures this fact.
>
>
>
>     4.5.  Category: TRANSPORT
>
>         Attributes that can be set normally for multiple items in a
>         multiplexed group but the software MUST pick just one of the
>         attribute of the given type for use.  The one chosen is the
>     attribute
>         associated with the "m=" line that represents the information being
>         used for the transport of the RTP.
>
>     How do you "pick" the one; by repeating the same value for each
>     bundled "m="-line?
>
>     Also, do we need to consider non-RTP cases too?
>
> [Suhas] - The expectation is to repeat as per the BUNDLE spec. Hence we
> have just added the way such a value is chosen but not the way the chose
> value is represented in the SDP as it is defined in the BUNDLE spec.
>
>
>     4.8.  Category: SPECIAL
>
>         Attributes where the text in the source draft must be consulted for
>         further handling when multiplexed.
>
>     s/source draft/specification defining the attribute/
>
>
> [Suhas] - Accepted
>
>
>
>     5.9.  RFC6773 - DCCP-UDP Encapsulation
>
>         Since RFC6773 is about tunneling DCCP in UDP, with the UDP port
>     being
>         the port of the DCCP en-/de-capsulation service.
>
>     Can't parse the sentence. Something missing here? Or extra "since"?
>
> [Suhas] -Removed since since it clarifies the text better.
>
>
>         This MAY or MAY NOT work depending on the
>         nature of DCCP encapsulations and ports choses thus rendering it to
>         be very application dependent.
>
>     Lower-case (non-RFC2119) "may" and "may not" would be appropriate
>     here. Also s/ports choses/port choices/?
>
>
> [Suhas] - Done
>
>
>
>     5.12.  RFC5245 - Interactive Connectivity Establishment (ICE)
>
>     Since the ICE candidate behavior is already specified in the bundle
>     draft at least reference there would be good? Also please make sure
>     that these stay in sync with what bundle has until it's an RFC.
>
> [Suhas] - BUNDLE spec provides O/A procedures for ICE but does not
> provide the category assignments for the ICE attributes.
>
>
>     5.21.  RFC6230 - Media Control Channel Framework
>
>                 | cfw-id  | Not Applicable  | M     | NORMAL       |
>
>     Should that be "Not Impacted" as in other NORMAL cases?
>
>
> [Suhas] - Accepted
>
>
>
>     5.39.  RFC6189 - ZRTP
>
>         | zrtp-hash  | Complicates if all the        | M     | NOT
>          |
>         |            | m=lines are not authenticated |       |
>     RECOMMENDED  |
>         |            | as given in the example below |       |
>          |
>
>     Where's the example? You mean the one in section 4.2?
>
> [Suhas] - The example SDP was removed from this section to not duplicate
> but I had forgotten the delete the reference to the example in here.
> Hence, will remove the example refernce
>
>
>     5.46.  RFC2326 - Real Time Streaming Protocol
>
>     Could mention here briefly why RTSP is not supported (similar to the
>     next section).
>
> [Suhas] - Done
>
>
>     5.50.  3GPP TS 183.063
>
>         | PSCid               | Not Impacted | Not-Applicable | NORMAL
>          |
>
>     The "Not-Applicable" level is not defined. Could add that to section
>     5 or explain here and in 5.58.
>
> [Suhas] - Will add appropriate SDP attribute levels to these sections
>
>
>     7.1.  RFC4585 - RTP/AVPF
>
>     Are the "Attr Name" here really attribute names or should one use
>     some other name in this case?
>
> [Suhas] - Will clean up the column name for consistency
>
>
>     7.3.  RFC6285 - Unicast-Based RAMS
>
>     This one was missing the "intro description". Could add something
>     brief for consistency with rest of the doc. Same for 7.6, 9.1, 9.2,
>     10.x (I'll leave it up to you where you consider it worthwhile adding).
>
> [Suhas] - Agreed, I will add the intro text for all these sections. It
> helps readability as well.
>
>     And is the "Attr Name" field here just "Name" intentionally? Same in
>     following two sections.
>
>
>     13.1.  RFC5109 - RTP Payload Format for Generic FEC
>
>         Draft draft-lennox-payload-ulp-ssrc-__mux proposes a simple fix
>     to make
>
>     This is a bit tricky. This should be a proper reference, but
>     referencing expired drafts is not a good practice either. I would
>     lean towards taking this sentence away since (if?) the fix is not
>     going forward at the IETF.
>
> [Suhas] - Removed this reference.
>
>
>     14.2.1.  Recommendation - Procedures for Potential Configuration Pairing
>
>         that are consistent across all the bundled media descriptions.
>
>     Could add "bundled media" to the terminology section.
>
> [Suhas] removed bundled with multiplexed to keep it uniform
>
>
>     15.  IANA Considerations
>
>     It would be great if someone double-checked that the "Mux Category"
>     entries here match to the text. Probably a simple script could do
>     that too.
>
> [Suhas]  - I can work on this to be sure
>
>     _________________________________________________
>     mmusic mailing list
>     [e-mail] <mailto:[e-mail]>
>     https://www.ietf.org/mailman/__listinfo/mmusic
>     <https://www.ietf.org/mailman/listinfo/mmusic>
>
>