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

[Suhas Nandakumar <[e-mail]>]

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]>
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 fe30efd02423cb054e50efd0248742ac7a52c8f91bc2
>         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]
> https://www.ietf.org/mailman/listinfo/mmusic
>