Re: [payload] AD Evaluation of draft-ietf-payload-rtp-vc2hq-04

[James Barrett <[e-mail]>]

On 5 Mar 2018, at 23:11, Ben Campbell <[e-mail]> wrote:
> 
> Hi,
> 
> Here is my AD Evaluation of draft-ietf-payload-rtp-vc2hq-04. It???s mostly in good shape, but I do have some comments and questions. I would like to discuss at least my substantive comments before IETF LC.
> 
> Thanks!
> 
> Ben.
> 
> *** Substantive Comments:
> 
> ??4.1: ??? A Sequence Header packet SHOULD have the same timestamp as the next picture which will follow it in the stream.  An End of Sequence packet SHOULD have the same timestamp as the previous picture which appeared in the stream.???
> 
> Why are those SHOULDs not MUSTs? Do you envision circumstances where it might be reasonable to violate them? What are the consequences if you do?

I honestly can???t think of any real consequences for violating this, but since these packets need to have some value for timestamp it seemed reasonable to make a recommendation.

> ??4.2, definitions of I and F: Same question as for ??4.1.

Yes, these should probably be MUST.

> - Slice Prefix Bytes and Slice Size Scaler: Both contain disclaimers saying ???unlikely to be too large.??? But IIUC, there are normative requirements later in the draft that prevent them from ever being to large. If my understanding is correct, it would be helpful to clarify this here. (Otherwise reviewers may see ???unlikely to be ??? as a red flag.)

Ok, I???ll clarify that.

> - This section is inconsistent about using normative keywords for field contents. Most do, but starting with ???Fragment Length??? they are stated descriptively. For matters of definition, I am okay with either approach but it would be good to be consistent.

Ok, I???ll fix them all to use normative keywords.

> ??4.4: "Every High Quality Picture Fragment MUST contain no more than 65535 slices.???
> 
> This seems non-constraining giving the following requirement that it must not contain over 65535 bytes.

Fair point. Maybe I should just list the bytes limit and then add the explanation ???and hence cannot contain over 65535 slices either???.

> ??5: ??? The circuit breakers is to be implemented and followed.???
> Was there a reason not to use a MUST here? (also it seems like there???s a missing word after ???breakers???)

Yeah, that seems wrong. 

> - ??? If used on a closed network which has been correctly provisioned for the expected data rates then profile MAY be used without congestion control, but on the open internet some sort of congestion control approach MUST be taken.???
> 
> That sort of statement is dangerous; code that is intended for use in private networks has a habit of escaping onto the internet. Is the code supposed to know?  Would it make sense to require implementations to default to using congestion control unless explicitly configured otherwise?

How about a change to the same sort of wording used in RFC 8130?

"Since UDP does not provide congestion control, applications that use
 RTP over UDP SHOULD implement their own congestion control above the
 UDP layer [RFC8085] and MAY also implement a transport circuit
 breaker [RFC8083].  Work in the RMCAT working group [RMCAT] describes
 the interactions and conceptual interfaces necessary between the
 application components that relate to congestion control, including
 the RTP layer, the higher-level media codec control layer, and the
 lower-level transport interface, as well as components dedicated to
 congestion control functions."


> *** Editorial Comments and Nits:
> 
> ??2: Please consider using the new boilerplate from RFC 8175. There are a few lowercase instances of ???should??? and ???must???. At least some of the ???should??? instances to not appear to be normative

Do you mean 8174? If so then yes, that makes sense.

> ??3: " Each High Quality Fragment data unit contains either a set of parameters for a picture or a series of coded Slices.
> 
> Does this mean ???set of parameters for (a picture or a series of coded slices), or (set of parameters for a picture) or (a series of coded slices)?
> 
> Also it would be good to clarify in ??3 that only fragments are used with this payload format.

It means the latter. Yes I will reword to be clearer.

> ??4: Caption for figure 2: The text describes figure 2 as ?????? carries the Picture Fragment containing the VC-2 Transform Parameters for a Picture. It would help to mention that (perhaps in a shortened form) in the caption.

ok.

> ??4.4: ??? If a Sequence intended for tranmission does not conform to these restrictions then it MAY be possible to simply convert it???"
> 
> This seems like a statement of fact rather than a grant of permission. As such, it should not use the 2119 keyword.

Yes, how about:

???Informative note: Some streams which do not meet these restrictions can be converted into streams which do without the need to reencode the encoded data simply by adjusting the distribution of slices into fragments, others cannot.???



Should I submit a revised draft with the above changes made?

--
James P. Weaver (n?? Barrett)
Research Engineer
BBC R&D North Lab,
Floor 5 Dock House, MediaCity,
M50 2LH
Tel: [phone]
e-mail: [e-mail]





-----------------------------
http://www.bbc.co.uk
This e-mail (and any attachments) is confidential and 
may contain personal views which are not the views of the BBC unless specifically stated.
If you have received it in 
error, please delete it from your system.
Do not use, copy or disclose the 
information in any way nor act in reliance on it and notify the sender 
immediately.
Please note that the BBC monitors e-mails 
sent or received.
Further communication will signify your consent to 
this.
-----------------------------

--- Begin Message ---

On 5 Mar 2018, at 23:11, Ben Campbell <[e-mail]> wrote:
> 
> Hi,
> 
> Here is my AD Evaluation of draft-ietf-payload-rtp-vc2hq-04. It’s mostly in good shape, but I do have some comments and questions. I would like to discuss at least my substantive comments before IETF LC.
> 
> Thanks!
> 
> Ben.
> 
> *** Substantive Comments:
> 
> §4.1: “ A Sequence Header packet SHOULD have the same timestamp as the next picture which will follow it in the stream.  An End of Sequence packet SHOULD have the same timestamp as the previous picture which appeared in the stream.”
> 
> Why are those SHOULDs not MUSTs? Do you envision circumstances where it might be reasonable to violate them? What are the consequences if you do?

I honestly can’t think of any real consequences for violating this, but since these packets need to have some value for timestamp it seemed reasonable to make a recommendation.

> §4.2, definitions of I and F: Same question as for §4.1.

Yes, these should probably be MUST.

> - Slice Prefix Bytes and Slice Size Scaler: Both contain disclaimers saying “unlikely to be too large.” But IIUC, there are normative requirements later in the draft that prevent them from ever being to large. If my understanding is correct, it would be helpful to clarify this here. (Otherwise reviewers may see “unlikely to be “ as a red flag.)

Ok, I’ll clarify that.

> - This section is inconsistent about using normative keywords for field contents. Most do, but starting with “Fragment Length” they are stated descriptively. For matters of definition, I am okay with either approach but it would be good to be consistent.

Ok, I’ll fix them all to use normative keywords.

> §4.4: "Every High Quality Picture Fragment MUST contain no more than 65535 slices.”
> 
> This seems non-constraining giving the following requirement that it must not contain over 65535 bytes.

Fair point. Maybe I should just list the bytes limit and then add the explanation “and hence cannot contain over 65535 slices either”.

> §5: “ The circuit breakers is to be implemented and followed.”
> Was there a reason not to use a MUST here? (also it seems like there’s a missing word after “breakers”)

Yeah, that seems wrong. 

> - “ If used on a closed network which has been correctly provisioned for the expected data rates then profile MAY be used without congestion control, but on the open internet some sort of congestion control approach MUST be taken.”
> 
> That sort of statement is dangerous; code that is intended for use in private networks has a habit of escaping onto the internet. Is the code supposed to know?  Would it make sense to require implementations to default to using congestion control unless explicitly configured otherwise?

How about a change to the same sort of wording used in RFC 8130?

"Since UDP does not provide congestion control, applications that use
 RTP over UDP SHOULD implement their own congestion control above the
 UDP layer [RFC8085] and MAY also implement a transport circuit
 breaker [RFC8083].  Work in the RMCAT working group [RMCAT] describes
 the interactions and conceptual interfaces necessary between the
 application components that relate to congestion control, including
 the RTP layer, the higher-level media codec control layer, and the
 lower-level transport interface, as well as components dedicated to
 congestion control functions."


> *** Editorial Comments and Nits:
> 
> §2: Please consider using the new boilerplate from RFC 8175. There are a few lowercase instances of “should” and “must”. At least some of the “should” instances to not appear to be normative

Do you mean 8174? If so then yes, that makes sense.

> §3: " Each High Quality Fragment data unit contains either a set of parameters for a picture or a series of coded Slices.
> 
> Does this mean “set of parameters for (a picture or a series of coded slices), or (set of parameters for a picture) or (a series of coded slices)?
> 
> Also it would be good to clarify in §3 that only fragments are used with this payload format.

It means the latter. Yes I will reword to be clearer.

> §4: Caption for figure 2: The text describes figure 2 as “… carries the Picture Fragment containing the VC-2 Transform Parameters for a Picture. It would help to mention that (perhaps in a shortened form) in the caption.

ok.

> §4.4: “ If a Sequence intended for tranmission does not conform to these restrictions then it MAY be possible to simply convert it…"
> 
> This seems like a statement of fact rather than a grant of permission. As such, it should not use the 2119 keyword.

Yes, how about:

“Informative note: Some streams which do not meet these restrictions can be converted into streams which do without the need to reencode the encoded data simply by adjusting the distribution of slices into fragments, others cannot.”



Should I submit a revised draft with the above changes made?

--
James P. Weaver (né Barrett)
Research Engineer
BBC R&D North Lab,
Floor 5 Dock House, MediaCity,
M50 2LH
Tel: [phone]
e-mail: [e-mail]

--- End Message ---