Re: [MMUSIC] Initial comments on draft-ietf-mmusic-rid-01

Adam Roach <adam@nostrum.com> Wed, 03 February 2016 15:31 UTC

Return-Path: <adam@nostrum.com>
X-Original-To: mmusic@ietfa.amsl.com
Delivered-To: mmusic@ietfa.amsl.com
Received: from localhost (ietfa.amsl.com [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id B06451ACE3D; Wed, 3 Feb 2016 07:31:43 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.9
X-Spam-Level:
X-Spam-Status: No, score=-1.9 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, HTML_MESSAGE=0.001, RP_MATCHES_RCVD=-0.001] autolearn=ham
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 5Jn4vdKYAzwL; Wed, 3 Feb 2016 07:31:37 -0800 (PST)
Received: from nostrum.com (raven-v6.nostrum.com [IPv6:2001:470:d:1130::1]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id F0B4E1ACDFF; Wed, 3 Feb 2016 07:31:36 -0800 (PST)
Received: from Orochi.local (99-152-145-110.lightspeed.dllstx.sbcglobal.net [99.152.145.110]) (authenticated bits=0) by nostrum.com (8.15.2/8.14.9) with ESMTPSA id u13FVUbK014804 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Wed, 3 Feb 2016 09:31:31 -0600 (CST) (envelope-from adam@nostrum.com)
X-Authentication-Warning: raven.nostrum.com: Host 99-152-145-110.lightspeed.dllstx.sbcglobal.net [99.152.145.110] claimed to be Orochi.local
To: Christer Holmberg <christer.holmberg@ericsson.com>, "mmusic@ietf.org" <mmusic@ietf.org>
References: <7594FB04B1934943A5C02806D1A2204B37D669A1@ESESSMB209.ericsson.se>
From: Adam Roach <adam@nostrum.com>
Message-ID: <56B21D52.6060002@nostrum.com>
Date: Wed, 03 Feb 2016 09:31:30 -0600
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:38.0) Gecko/20100101 Thunderbird/38.5.1
MIME-Version: 1.0
In-Reply-To: <7594FB04B1934943A5C02806D1A2204B37D669A1@ESESSMB209.ericsson.se>
Content-Type: multipart/alternative; boundary="------------030206090103020300090306"
Archived-At: <http://mailarchive.ietf.org/arch/msg/mmusic/9OLqkmLFPOr53m582w6ANs5XDYY>
Cc: "mmusic-chairs@ietf.org" <mmusic-chairs@ietf.org>
Subject: Re: [MMUSIC] Initial comments on draft-ietf-mmusic-rid-01
X-BeenThere: mmusic@ietf.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: Multiparty Multimedia Session Control Working Group <mmusic.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/mmusic>, <mailto:mmusic-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/mmusic/>
List-Post: <mailto:mmusic@ietf.org>
List-Help: <mailto:mmusic-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/mmusic>, <mailto:mmusic-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 03 Feb 2016 15:31:43 -0000

On 2/2/16 06:15, Christer Holmberg wrote:
> I was asked to take a look at the RID draft.

Thanks for a really quick turnaround on reviewing it!

What would be really helpful is input on the explicitly-called-out open 
issues.

> GENERAL:
>
> ------------
>
> Q_G_1:
>
> Please pay attention to usage of consistent terminology.
>
> For example, I see “a= lines”, “a= parameters”, “a= attributes” type 
> of terminology…
>

I think this is consistent already, with one exception (which will be 
fixed in -02). If you can point out particular instances that are 
causing issues, I'd appreciate it.

To be clear, the phrase '"a=rid" line' refers to an entire line; e.g.:

a=rid:1 send max-width=1280;max-height=720;max-fps=30

Any reference to a "parameter" refers to a parameter on such a line. For 
example, "send" in the preceding line. Or "max-fps=30".

The only way I think this could be made more clear would be with the 
addition of a rather superfluous paragraph in the terminology section, 
along the lines of:

    "The term 'attribute' is used in this document as having the same
    meaning as that word in Section 5.13 of [RFC4566]. The term 'line'
    is used in this document as having the same meaning as that word as
    used in section 5, and *almost* all of its subsections, in
    [RFC4566]. The term 'parameter' is used in this document to have its
    common English meaning, with deep parallels to its use in the
    'a-fmtp:' description in section 6 of [RFC4566]."

The point, in case it's not clear, is that these terms in the document 
are being used as they've been used to describe SDP components for decades.

> Q_G_2:
>
> The draft talks a lot about “payload”, “codec”, “format” etc. I think 
> the draft should contain a definition of those. If they are defined 
> elsewhere, such text can say e.g.:
>
>    “As defined in [RFCXXXX], payload refers to blah blah blah…”
>

Again referring to RFC4566, these terms are used freely with no 
definition or external citation. Are you expecting people to implement 
this SDP extension without having first understood base SDP?

> Q_G_3:
>
> Is there a risk of conflicts between information provided in a=rid and 
> a=fmtp/rtpmap? If so, how shall it be treated?
>

Section 8 exists solely to address this question. What do you believe is 
missing?

> SECTION 1:
>
> -------------
>
> Q_1_1:
>
> The text says:
>
>    “Payload Type (PT) in RTP provides a mapping between the format of the
>
>    RTP payload and the media format description specified in the
>
>    signaling.  For applications that use SDP for signaling, the
>
>    constructs rtpmap and/or fmtp describe the characteristics of the
>
>    media that is carried in the RTP payload, mapped to a given PT.”
>
> Assuming the scope of the draft is SDP, I think the paragraph can be 
> shorter, e.g.:
>
>    “Payload Type (PT) in RTP provides a mapping between the RTP 
> payload format
>
>    and the associated SDP media description. The SDP rtpmap and/or 
> fmtp attributes
>
>    are used, for a given PT, to the describe the characteristics of 
> the media that is carried
>
>    in the RTP payload.”
>

Sure, why not. I've updated accordingly.

> Q_1_2:
>
> The text says:
>
>    “However, the assignment of static mapping of payload
>
>    codes to payload formats and multiplexing of RTP with other protocols
>
>    (such as RTCP) could result in limited number of payload type numbers
>
>    available for the application usage.”
>
> What is a “payload code”? Shall it be “codec”?
>

In the language used by RFC 4566, this would be "RTP payload type 
number". I'll update accordingly.

> Q_1_3:
>
> The text says:
>
>    “and to effectively map a RID RTP Stream to its corresponding 
> constraints.”
>
> I know there is a reference for “RID RTP Stream” in the Terminology 
> section, would I think it would be good to have the reference also in 
> the Introduction.
>

Based on this and other previous comments, I'm biting the bullet and 
moving the terminology section to the front of the document, before the 
introduction.

> Q_1_4:
>
> The text says:
>
>    “As described in Section 6.2.1, this mechanism achieves backwards
>
>    compatibility via the normal SDP processing rules, which require
>
>    unknown a= parameters to be ignored.  This means that implementations
>
>    need to be prepared to handle successful offers and answers from
>
>    other implementations that neither indicate nor honor the constraints
>
>    requested by this mechanism.”
>


It does. Did you want to comment on it?


> SECTION 6:
>
> -------------
>
> Q_6_1:
>
> The text says:
>
>     “For each media description in the offer,…”
>
> It should say for each media description that describes RTP based 
> media, or something like that…
>

Updated: "For each RTP media description..."

> Q_6_2:
>
> The text says:
>
>     “It MUST generate a "rid-identifier" that is unique within a media 
> description”
>
> Later, in section 6.5, the draft says that a new offer with “proposed 
> changes” should use a new ID value. What about a subsequent RTP 
> session? Is it ok to re-use the value(s) from the previous session?
>

Sure, but I think that's pretty self-evident. I can't think of any 
construct that calls out constraints between non-concurrent RTP sessions.

> Q_6_3:
>
> The text says:
>
>    “The Offerer then chooses zero or more "a=rid" constraints
>
>    (Section 5) to be applied to the rid, and adds them to the
>
>     "a=rid" line.”
>
> What is meant by “the rid”?
>

Updated: "...to the the RID RTP Stream..."

> Q_6_4:
>
> In section 6.2.1, the text says:
>
>    “As a result, ignoring the "a=rid" line is always guaranteed to 
> result in a valid session description.”
>
> While this is true, a few words about the consequences of the fact 
> that RID won’t be used is needed.
>
> For example, if the offerer has indicated some constraints using 
> a=rid, and the answerer does not support a=rid, does the offerer have 
> to be prepared to receive media “outside” of those constraints, etc?
>
> Just saying that unsupported attributes are ignored, but it doesn’t 
> matter because the SDP will still be valid, is not enough when it 
> comes to describing the backward compatibility :)
>

Sure, but this is true regardless of whether the remote endpoint 
supports "a=rid". It is quite possible that the result of applying the 
rules in the "answer" section results in any given m-section containing 
zero "a=rid" lines. I've added text to the "Offerer Processing of the 
SDP Answer" section:

    It is important to note that there are several ways in which an
    offer can
    contain a media section with "a=rid" lines, but the corresponding media
    section in the response does not. This includes situations in which the
    answerer does not support "a=rid" at all, or does not support the
    indicated
    constraints. Under such circumstances, the offerer MUST be prepared to
    receive a media stream to which no constraints have been applied.



> Q_6_5:
>
> In section 6.3, the text says:
>
>     “Having performed verification of the SDP offer as described”
>
> I assume something is missing behind “described”?
>
> I guess the answer to the question above may clarify it, but what is 
> meant by “verification”?
>

It's referring to the immediately preceding section, which uses the word 
"verify" several times. For absolute clarity, I've updated its initial 
paragraph to include the word "verification":

    If the answerer supports the "a=rid" attribute, the following
    verification
    steps are executed, in order, for each "a=rid" line in a given media
    description:

And I've explicitly cited it in the sentence you point out:

    Having performed verification of the SDP offer as described in
    Section 6.2.2, the answerer shall perform the following steps to
    generate the SDP answer.


> Q_6_6:
>
> In section 6.4, I see a mix of “MUSTs” and “SHALLs”, but I can’t 
> really see the difference.
>

I suppose that's good, since RFC2119 defines them to have no difference.

MUST, SHALL, and REQUIRED all indicate the same normative level. This is 
kind of nice, since it allows authors to choose among several terms 
according to which one flows better in the sentence.

> Q_6_7:
>
> In section 6.4, the text talks about cases where “the offerer shall 
> consider the a= line as rejected”.
>
> It is unclear whether it is meant to mean that the answerer rejected 
> the a= line (which I agree would be a little strange), or if the 
> offerer shall reject the a= line.
>
> Assuming it’s the offerer, I think it would be more clear the say “the 
> offerer shall reject the a= line”.
>
> I also think it would be better to say “discard” instead of “reject”, 
> because “reject” often means that some additional action is taken, 
> which I assume is not the case here.
>

Agreed. I've changed these to use language along the lines of "...shall 
discard the 'a=rid' line."

I'll have an -02 version out shortly with these changes incorporated.

/a