Subject: Re: [AVTCORE] AD Review of draft-ietf-payload-vp9-12

From: Jonathan Lennox <[email-masked]>
Date: Fri, 7 May 2021 14:31:32 -0400

Hi - sorry for not responding sooner.  Responses inline.

Do you want me to publish a version -13 with the changes mentioned here?  In the meantime my changes are in the GitHub repo at https://github.com/juberti/draughts/tree/master/vp9 <https://github.com/juberti/draughts/tree/master/vp9>.

> On Apr 19, 2021, at 6:32 PM, Murray S. Kucherawy <[email-masked]> wrote:
> 
> This is my review of draft-ietf-payload-vp9-12.
> 
> Video codecs are not exactly my strong suit, so finding technical errors wasn't an expected part of my mission here.  I'm going on faith that this is all technically correct, and just looking for editorial or procedural things.
> 
> -MSK
> 
> --
> 
> Shepherd writeup:
> 
> * We should probably explain why a document coming out of AVTCORE bears a name that doesn't start with "draft-ietf-avtcore-...".  The shepherd writeup should mention this.  Or if you don't want to do that, please let me know what the history is there so I can include it in a note to the IESG.

Either in the writeup or in the note is fine, but the history is that the Payload Working Group was merged back into the AVTCore group in September 2019, whereas this draft was first adopted as a WG item of the Payload group in 2015.  (Yes, we’ve been very slow about finishing this draft.)

> * The writeup identifies draft-ietf-avtext-lrr as a downward reference, but that document is in the queue already for Proposed Standard status, and this one is going for the same, so I don't believe this is a downward reference (once both are published).
>  <>
> General nits:
> 
> * Referring to an I-D as a "memo" is a little outdated.  Suggest "document" or "specification" instead.

Ok.

> 
> Section 1:
> 
> * Should this document obsolete VP8 (RFC 6386)?

No, VP8 and VP9 are two different codecs, and they’re both expected to remain in active use.

> Section 3:
> 
> * The Editor's Note seems like it should've been resolved by now.  If no suggestion for improved terminology arrives by the end of IESG Evaluation, should that paragraph be removed?

I’ve kept the clarification (that “Picture Group” and “Group of Pictures” are different), but removed the request for better terminology.

> Section 4.1:
> 
> * For "Timestamp", did you mean to say "alternately" (i.e., every other one; alternating) or "alternatively" (i.e., presenting a different choice)?

“Alternatively”; changed.

> Section 4.2:
> 
> * First sentence, "The" shouldn't be capitalized.

Ok.

> 
> * The use of CONDITIONALLY RECOMMENDED or CONDITIONALLY REQUIRED is puzzling because one of them is a BCP 14 word and one isn't, but both look like they're supposed to be.  This might be better described in the prose for each of these fields, and just use the BCP 14 words here (or omit them entirely).

I’ve made “Conditionally” only initially capitalized, and clarified the description of the fields to have capitalized BCP 14 words.

> * The description of PID refers to a "maximum ID", but that's not specified anywhere.  Is it simply the largest PID that will fit in the available bits, or is there some other maximum to be observed?

The maximum that will fit in the available bits; I’ve clarified.

> 
> * For the "D:" bullet, I think "MUST be set to one if current spatial layer SID frame depends on spatial layer SID-1 frame of the same picture. MUST only be set to zero if current spatial layer SID frame does not depend on spatial layer SID-1 frame of the same picture." can be reduced to "MUST be set to one if and only if the current spatial layer SID frame depends on spatial layer SID-1 frame of the same picture."

Good improvement.

> 
> Section 6.1:
> 
> * Does the media type's subtype name have to be "VP9", in specifically that case?

No, media subtype names aren’t case-sensitive.  Do you think it would be clearer to write it in lower-case here?

> * Per RFC 6838, please change Required Parameters to "N/A" instead of "None".

Ok.

> * I think it's unusual for the BCP 14 text to appear in a media type registration (specifically, the Optional Parameters).  You might consider moving that discussion to some other prose section, and in the media type registration just list the supported parameters and their syntaxes.

I’ve reorganized this section; I think it generally flows better now.

Thank you for your comments!

— 
Jonathan Lennox
[email-masked]

---

From: "Murray S. Kucherawy" <[email-masked]>
Date: Fri, 7 May 2021 11:48:30 -0700

Hi Jonathan,

On Fri, May 7, 2021 at 11:31 AM Jonathan Lennox <[email-masked]>
wrote:

> Do you want me to publish a version -13 with the changes mentioned here?
> In the meantime my changes are in the GitHub repo at
> https://github.com/juberti/draughts/tree/master/vp9.
>

Yes, please.

>
> Shepherd writeup:
>
> * We should probably explain why a document coming out of AVTCORE bears a
> name that doesn't start with "draft-ietf-avtcore-...".  The shepherd
> writeup should mention this.  Or if you don't want to do that, please let
> me know what the history is there so I can include it in a note to the IESG.
>
>
> Either in the writeup or in the note is fine, but the history is that the
> Payload Working Group was merged back into the AVTCore group in September
> 2019, whereas this draft was first adopted as a WG item of the Payload
> group in 2015.  (Yes, we’ve been very slow about finishing this draft.)
>

Ah, thanks for that.  I'll do it in the note.  The merger predates the time
of most people on the current IESG, so I'm just hoping to head off any
questions during IESG Evaluation.

No, VP8 and VP9 are two different codecs, and they’re both expected to
> remain in active use.

OK, just checking.

>
> * The use of CONDITIONALLY RECOMMENDED or CONDITIONALLY REQUIRED is
> puzzling because one of them is a BCP 14 word and one isn't, but both look
> like they're supposed to be.  This might be better described in the prose
> for each of these fields, and just use the BCP 14 words here (or omit them
> entirely).
>
>
> I’ve made “Conditionally” only initially capitalized, and clarified the
> description of the fields to have capitalized BCP 14 words.
>

OK, that at least looks less strange.  Fair compromise.

> clearer to write it in lower-case here?

Not necessarily.  Again, just checking; there have been media type
applications I've processed in the past that came in in all-caps because
the applicant didn't know they're case-insensitive.

When the new version appears, I'll send it for Last Call.

-MSK