Subject: Re: [payload] Gen-art 2nd LC review of draft-ietf-payload-vp8-16 (with summary)

From: Colin Perkins <[email-masked]>
Date: Tue, 14 Jul 2015 10:47:41 +0100

[cc to payload wg, rather than ietf-discuss]

Some comments inline. 
Colin

On 13 Jul 2015, at 13:01, Elwyn Davies <[email-masked]> wrote:
> I am the assigned Gen-ART reviewer for this draft. For background on Gen-ART, please see the FAQ at
> 
> <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.
> 
> Please resolve these comments along with any other Last Call comments
> you may receive.
> 
> Document: draft-ietf-payload-vp8-16.txt
> Reviewer: Elwyn Davies
> Review Date: 2015/07/10
> IETF LC End Date: 2015/07/13
> IESG Telechat date: (if known) -
> 
> Summary: Gosh, it has been a long time since the last review! Almost ready.  Still needs various minor nits fixed and some clarifications.
...
> Nits/editorial comments:
...
> s4.1, last two paras:
>>    Sequence number:  The sequence numbers are monotonically increasing
>>       and set as packets are sent.
>> 
>>       The remaining RTP header fields are used as specified in
>>       [RFC3550].
> Redefining the Sequence Number field seems gratuitous and potentially dangerous,
> given that it is *very carefully* defined in RFC 3550 and the overall intent does
> not seem any different from that in RFC 3550.  OTOH a note about the (non?-)use of the X bit
> and the Payload Type field (PT) would be useful.  I suggest replacing the paras above with:
> NEW:
>   X bit: MUST be 0.  The VP8 RTP profile does not use the RTP Header Extension capability.

This is not correct. The draft is an RTP Payload Format, not an RTP Profile. RTP Payload Formats cannot prohibit the use of RTP header extensions. 

>   PT (Payload Type):  In line with the policy in Section 3 of [RFC3551], applications
>      using the VP8 RTP payload profile MUST assign a dynamic payload type number to

This cannot be a MUST strength requirement, since the RTP/AVP profile [RFC3551] cannot be mandated (other, incompatible, RTP Profile could exist).

>      be used in each RTP session and provide a mechanism to indicate the mapping.
>      See Section 6.2 for the mechanism to be used with the Session Description Protocol (SDP)
>      [RFC4566].
> 
>      The remaining RTP Fixed Header Fields (V, P, CC, sequence number, SSRC and CSRC
>      identfiers) are used as specified in Section 5.1 of [RFC5330].
> END
> Note that this may be considered to make the reference to RFC 3551 normative.
> 
> s4.2, X bit:  There is potential for confusion between the X bit in the fixed header
> and the X bit in the VP8 Payload Descriptor.  Perhaps changing this to C for control bits
> would avoid the problem.
> 
> s4.2, M bit: Similarly, it might be better to use B (for BIG) in place of M to avoid confusion.

Agree with these two.

...
> s7: s/Cryptographic system may also allow/The cryptographic system may also allow/
> 
> s7:
>> the usage of SRTP [RFC3711] is recommended.
> RECOMMENDED?

No - see RFC 7202. Section A.13 of draft-ietf-payload-rtp-howto-14 has suggested text for this.

> s10.2: I suspect that RFC 3551 is normative.

It shouldn't need to be.

-- 
Colin Perkins
https://csperkins.org/