[payload] Eric Rescorla's No Objection on draft-ietf-payload-rtp-vc2hq-06: (with COMMENT)

[Eric Rescorla <[e-mail]>]

Eric Rescorla has entered the following ballot position for
draft-ietf-payload-rtp-vc2hq-06: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-payload-rtp-vc2hq/



----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Rich version of this review at:
https://mozphab-ietf.devsvcdev.mozaws.net/D4183


I am not sure that this specification can be interoperably
implemented. I have noted a number of points below. I believe these
are largely minor and so have not made this a DISCUSS, but it is
important they be resolved.

This document would also benefit from significant editorial work.
Based on S  4.5.1, I take the structure to be that you start with the
VC-2 stream and then use RTP headers to contain the information in
Parse Info blocks. If that is correct, it would be much clearer if
stated upfront.



IMPORTANT
S 4.2.
>   
>      The fields of the extended headers are defined as follows:
>   
>      Extended Sequence Number: 16 bits  MUST Contain the high-order
>            16-bits of the 32-bit packet sequence number, a number which
>            increments with each packet.  This is needed since the high

Increments by one?


S 4.2.
>            data rates of VC2 Sequences mean that it is highly likely that
>            the 16-bit sequence number will roll-over too frequently to be
>            of use for stream synchronisation.
>   
>      B: 1 bit  MUST be set to 1 if the packet contains the first byte of
>            an Auxiliary Data or Padded Data Unit.

And otherwise must be 0?


S 4.2.
>   
>      B: 1 bit  MUST be set to 1 if the packet contains the first byte of
>            an Auxiliary Data or Padded Data Unit.
>   
>      E: 1 bit  MUST be set to 1 if the packet contains the final byte of
>            an Auxiliary Data or Padded Data Unit.

And otherwise must be 0?


S 4.2.
>            from a new picture until all the coded data from the current
>            picture has been sent.
>   
>            If the receiver does not receive a transform parameters packet
>            for a picture then it MAY assume that the parameters are
>            unchanged since the last picture, or MAY discard the picture.

How does this interact with packet loss?

COMMENTS
S 3.
>      the decoder.
>   
>      Each Sequence consists of a series of 13-octet Parse Info headers and
>      variable length Data Units.  The Sequence begins and ends with a
>      Parse Info header and each Data Unit is preceded by a Parse Info
>      Header.  Data Units come in a variety of types, the most important

This text isn't very clear to me. Is the following valid: PI | Data |
PI? How about PI | PI | Data | PI.  PI | PI | PI | Data | PI?




S 3.
>      should not be assumed.
>   
>      The High Quality (HQ) profile for VC-2 restricts the types of Parse
>      Info Headers which may appear in the Sequence to only:
>   
>      o  Sequence Headers,

The text above says that Sequence Headers are a type of Data Unit. So
I'm confused by this text.


S 4.
>   
>   4.  Payload format
>   
>      This specification only covers the transport of Sequence Headers,
>      High Quality Fragments, Auxiliary Data, and (optionally) End of
>      Sequence Headers and Padding Data.

So it doesn't include Parse Info?


S 4.
>         Picture (Figure 2),
>   
>      o  A Picture Fragment containing VC-2 Coded Slices (Figure 3) for a
>         picture,
>   
>      o  The end of a VC-2 Sequence (Figure 4)

It would be helpful if you cited specific sections of VC-2 for these.


S 4.
>      .                                                               .
>      +---------------------------------------------------------------+
>   
>                  Figure 6: RTP Payload Format For Padding Data
>   
>      All fields in the headers longer than a single bit are interprted as

Nit: interpreted.


S 4.2.
>   
>      Data Length: 32 bits  For an auxiliary data unit this contains the
>            number of bytes of data contained in the uncoded payload
>            section of this packet.  For a Padding Data Unit this field may
>            have any value and simply indicates the size of the recommended
>            padding.

This seems like a very large field given that RTP datagrams are almost
never this large, so I am suspecting the "uncoded payload" means pre-
compressed? Can you be clearer..