Re: [MMUSIC] AD evaluation: draft-ietf-mmusic-rtsp-nat-20

[Alissa Cooper <[e-mail]>]

Hi Magnus,

Thanks. Comments inline.

On 5/27/14, 2:59 AM, "Magnus Westerlund" <[e-mail]>
wrote:

>Hi,
>
>Thanks for the review. Please see below for answers and comments on your
>questions and comments.
>
>On 2014-05-22 21:42, Alissa Cooper wrote:
>> I have reviewed draft-ietf-mmusic-rtsp-nat-20 in preparation for IETF
>>LC.
>> The document is in good shape. I have a few questions and comments below
>> that need to be resolved before issuing the last call. I’ve also
>>included
>> a list of nits that should be resolved in the next rev.
>> 
>> Comments and questions:
>> 
>> Sec 4.2:
>> "The connection address SHOULD use the same format
>>       (explicit IP or FQDN) as in the dest_addr parameter used to
>>       express fallbacks.”
>> 
>> Per 4.1, I thought the dest_addr MUST NOT be included, so this
>>requirement
>> seems contradictory.
>
>So, this is a reference to another transport specification used as
>fallback if ICE isn't supported. The format of the D-ICE transport
>spec's connection-address SHOULD be the same format (explicit IP or
>FQDN) as used in that fallback transport specification's dest_addr
>parameter.
>
>Will see what we can do to clarify that it is referencing another
>transport spec.

Thanks.

>
>> 
>> "<tcp-type-ext>:  conveys the candidates connection type (active,
>>       passive, or S-O) for TCP based candidates.  This MUST be included
>>       for candidates that have <transport> set to TCP and MUST NOT be
>>       included for other transport types, including UDP, unless
>>       explicitly specified for that transport protocol."
>> 
>> I’m assuming the last phrase means “unless <something> gets specified in
>> the future for that transport protocol.” But I’m not clear on what the
>> <something> is.
>
>So, if anyone defines a new transport protocol and you like to add this
>here. Then that protocol may chose to re-use this header. That is when
>that unless occurs. As one can define a new parameter as easily, I guess
>this "unless ..." is quite unnecessary and can be removed.

Sounds good.

>
>> 
>> Sec 4.4:
>> "The RTSP client SHOULD send the feature tag "setup.ice-d-m" in the
>>    "Supported" header in all SETUP requests that contain the "D-ICE"
>>    lower layer transport.”
>> 
>> What are the situations where the SETUP request contains “D-ICE” but
>>where
>> the feature tag should not be included? Is it for SETUP requests issued
>> after media is already playing (as described in Sec 6.13)?
>
>I would classify this SHOULD as: we have no good reason why you would
>not include the feature tag. But if you have one, we don't see a reason
>for making the implementation violate a MUST.

Would help to explain that rationale in the document.

>
>> 
>> Sec 6.5:
>> "The server determines if the SETUP request is successful from the
>>    other perspectives”
>> 
>> It’s not clear what is meant by “from the other perspectives.”
>
>As a SETUP for a media resource also the preparation and availability of
>the media resource, not only the transport needs to be fulfilled. It
>might be clear to simply use "is successful"

Agreed.

>
>> 
>> Sec 6.10:
>> "If the 480 response is received or sent in response to a PLAY
>>    request, the server MUST NOT free its gathered candidates."
>> 
>> The “received or sent” part doesn’t quite make sense, since we’re
>>talking
>> about the server, right? Shouldn’t this just say “sent”?
>
>Correct, as currently specified the 480 is only sent in Server -> Client
>direction, thus the "received or" is to be removed.

Ok.

>
>> 
>> Sec 6.13:
>> See comments on Sec 4.4 above — why is the Supported header not included
>> in the SETUP requests in this example?
>
>I don't think there exist a reason, just a missed inclusion. will address.

Ok.

>
>
>> 
>> Sec 7.1 and 7.2:
>> Again with reference to Sec 4.4 — why is the "setup.ice-d-m” feature tag
>> mandatory for media proxies but recommended for clients and signaling
>> proxies?
>
>Because a client really needs to know if a proxy supports the ICE-D
>transport. If it doesn't you end up in all the issues discussed in
>Section 7.3. If there is one change I think should be done here is to
>make 7.2 a SHALL indicate its capability. I also clarified that this
>relates to SETUP request and responses.

Good, this makes sense to me.

>
>If a client skips including a Supported header but anyway tries using
>ICE-D as transport, it will work if the nodes support it. Including the
>Supported header will only make it clear on the feature negotiation
>level that the feature is supported and also prompt the proxies to
>include their Proxy-Supported header.
>
>> 
>> Sec 7.3:
>> "Unfortunately, the usage of the "setup.ice-d-m" feature tag in the
>>    Proxy-Require will have contradicting results.”
>> 
>> This sentence and the paragraph that follows are confusing because there
>> is no prior mention of putting the feature tag in the Proxy-Require
>>header
>> (although I guess it was implicitly assumed). I think what you meant is
>> this:
>> 
>> "The usage of the “setup.ice-d-m” feature tag in the Proxy-Require
>>header
>> is NOT RECOMMENDED because it can have contradictory results.”
>> 
>
>Yes, the sentence you propose is better, will include. The text is
>written with the assumption that one knows RTSP 2.0 and its feature
>negotiation mechanisms. Thus, clearly one can include the feature tag in
>any headers that uses feature tags when relevant.

Ok.

>
>> Sec 8:
>> "This multiplexing SHALL be combined with ICE”
>> 
>> I think s/ICE/ICE-RTSP/ makes sense here.
>
>I actually removed the SHALL, it was duplicate with the SHALL in the
>last sentence. So I change this to "This multiplexing is very benefical
>when be combined with ICE for RTSP as it makes RTP and RTCP need only a
>single component ..."

Ok. 

>
>
>
>> 
>> "Due to the above mentioned benefits, RTSP servers and clients that
>>    support "D-ICE" lower layer transport in combination with RTP SHALL
>>    also implement RTP and RTCP multiplexing as specified in this section
>>    and [RFC5761].”
>> 
>> Multiplexing is not specified in this section. Did you mean as specified
>> in Appendix C of draft-ietf-mmusic-rfc2326bis and RFC5761?
>> 
>
>Yes, will address.

Ok.

>
>
>We will take care of the nits.

Great, thanks.
Alissa

>
>Cheers
>
>Magnus Westerlund
>
>----------------------------------------------------------------------
>Services, Media and Network features, Ericsson Research EAB/TXM
>----------------------------------------------------------------------
>Ericsson AB                 | Phone  [phone]
>Färögatan 6                 | Mobile [phone]
>SE-164 80 Stockholm, Sweden | mailto: [e-mail]
>----------------------------------------------------------------------
>