Re: [AVTCORE] Francesca Palombini's No Objection on draft-ietf-payload-vp9-13: (with COMMENT)

[Jonathan Lennox <[e-mail]>]

> On Jun 1, 2021, at 5:34 PM, Francesca Palombini via Datatracker <[e-mail]> wrote:
> 
> Francesca Palombini has entered the following ballot position for
> draft-ietf-payload-vp9-13: No Objection
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> Thank you for the work on this document.
> 
> I have some non blocking questions and comments, which I hope will help improve
> the document.
> 
> Francesca
> 
> 1. -----
> 
>   Timestamp:  The RTP timestamp indicates the time when the input frame
>      was sampled, at a clock rate of 90 kHz.  If the input picture is
>      encoded with multiple layer frames, all of the frames of the
>      picture MUST have the same timestamp.
> 
> FP: I think it would be useful to add a reference to RFC 3550, regarding "RTP
> timestamp". Also, I find it curious that RFC 3550 is not mentioned up to the
> end of section 4.1 (I would think a reference to it would be present in the
> introduction)

Added to both places, thanks.

> 2. -----
> 
>      Otherwise, PID MUST NOT be present.  If the SS field was present
>      in the stream's most recent start of a keyframe (i.e., non-
>      flexible scalability mode is in use), then the PID MUST also be
>      present in every packet.
> 
> FP: Is there any reason why this is not formulated in terms of V bit being set?
> (I believe the rest of the text is consistently talking about bit being set)

No, fixed.

> 
> 3. -----
> 
>      described by "Reference indices" below.  This MUST only be set to
>      1 if the I bit is also set to one; if the I bit is set to zero,
>      then this MUST also be set to zero and ignored by receivers.  The
> 
> FP: Why is that the it MUST only be set to 1 if I is also set to 1? I was
> looking for the motivation, but could not find it. Some more text would have
> been helpful to me.

The Reference indices are defined as offsets from the Picture ID, so without I, there’s nothing for them to reference.  I’ve added a parenthetical to clarify this.

> 4. -----
> 
>   Z:  Not a reference frame for upper spatial layers.  If set to 1,
>      indicates that frames with higher spatial layers SID+1 of the
>      current and following pictures do not depend on the current
> 
> FP: I am not sure if the text it meant to say higher spatial layers than SID+1
> (inclusive?)

Yes, 

> 5. -----
> 
>     The field MUST be present if the I bit is equal to one.  If set,
>      the PID field MUST contain 15 bits; otherwise, it MUST contain 7
> 
> FP: "If set" - I understand by the context this should be "If M is set" (how
> it's written now it could be interpreted by "if the PID field is set", which
> does not make sense, but better be clear)

Ok.

> 
> 6. -----
> 
>      or 15-bit index.  The PID SHOULD start on a random number, and
>      MUST wrap after reaching the maximum ID (0x7f or 0x7fff depending
>      on the index size chosen).  The receiver MUST NOT assume that the
> 
> FP: So is the intention that the PID is increased by one for each picture? Does
> the order matter? The way the text is written "reaching the maximum ID" would
> suggest so, but I could not find any text about that, if I have missed it
> please let me know.

Yes, I’ve clarified this.

> 
> 7. -----
> 
>       SID-1 frame of the same picture, otherwise MUST set to zero.
> 
> FP: s/MUST set/MUST be set

Ok.

> 
> 8. -----
> 
>         depends on.  TL0PICIDX MUST be incremented when TID is equal to
>         0.  The index SHOULD start on a random number, and MUST restart
> 
> FP: Does it matter by how much? If so, it should be stated.

By 1, thanks, clarified.

> 
> 9. -----
> 
>      temporal layer ID (TID), switch up point (U), and the R reference
>      indices (P_DIFFs) are specified.
> 
> FP: I couldn't find the R bit defined anywhere. I assume its meaning is "if
> set, P_DIFF is present" but this should be clearly stated in the text.

I think that was just a typo for “Reference indices”, fixed.

> 
> 10. -----
> 
> FP: Please expand MCU, LRR on first use

Expanded LRR; changed MCU to selective forwarding middlebox with a reference to RFC 7667.

> 11. -----
> 
> Section 7. IANA
> 
> FP: I checked the mailarchive for the subtype registration and could not find
> it. I leave it to Murray to let me know if we are more lenient about subtype
> requests, but I would have appreciated the registration being posted to the
> media-types mailing list.

Sorry about that; with my AVTCore chair hat on we’ve probably been sloppy about sending RTP-only subtypes to the media-types mailing list.