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

From: Eric Rescorla <[email-masked]>
Date: Tue, 19 Jun 2018 06:54:36 -0700

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..

---

From: "Roni Even (A)" <[email-masked]>
Date: Wed, 27 Jun 2018 06:48:58 +0000

Hi,

S 4.2.

Increments by one?

RE: I assume that you meant that the sequence number is incremented in each packet and not the extended sequence number. 
BTW: if you meant the sequence number you do not need to say " a number which increments with each packet." Since this is the definition in RFC3550

   sequence number: 16 bits
      The sequence number increments by one for each RTP data packet
      sent, and may be used by the receiver to detect packet loss and to
      restore packet sequence.  The initial value of the sequence number
      SHOULD be random (unpredictable) to make known-plaintext attacks
      on encryption more difficult, even if the source itself does not
      encrypt according to the method in Section 9.1, because the
      packets may flow through a translator that does.  Techniques for
      choosing unpredictable numbers are discussed in [17].

Roni

-----Original Message-----
From: Eric Rescorla [mailto:[email-masked]] 
Sent: Tuesday, June 19, 2018 4:55 PM
To: The IESG
Cc: [email-masked]; [email-masked]; [email-masked]; [email-masked]
Subject: Eric Rescorla's No Objection on draft-ietf-payload-rtp-vc2hq-06: (with COMMENT)

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.)

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.

Increments by one?

S 4.2.

And otherwise must be 0?

S 4.2.

And otherwise must be 0?

S 4.2.

COMMENTS
S 3.
>      Header.  Data Units come in a variety of types, the most 

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.
>      Info Headers which may appear in the Sequence to only:

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

S 4.

S 4.

S 4.

Nit: interpreted.

S 4.2.
>            have any value and simply indicates the size of the recommended

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..

---

From: Eric Rescorla <[email-masked]>
Date: Wed, 27 Jun 2018 05:03:57 -0700

On Tue, Jun 26, 2018 at 11:48 PM, Roni Even (A) <[email-masked]>
wrote:

> RE: I assume that you meant that the sequence number is incremented in
> each packet and not the extended sequence number.
>

Yes.

BTW: if you meant the sequence number you do not need to say " a number
> which increments with each packet." Since this is the definition in RFC3550
>
>    sequence number: 16 bits

Well, it seems like if you are going to bother to say it, you should be
precise, but this is a comment, so do as you think best.

> 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.
>
> 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.
> >
> 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.
> >