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

[James Barrett <[e-mail]>]

I have submitted a new version of the draft:

https://tools.ietf.org/html/draft-ietf-payload-rtp-vc2hq-07

Most of the changes are either exactly as already discussed, or minor changes to text to make sections make sense with the changes we already discussed.

I have, however, made one additional change which we didn’t already discuss in order to resolve the Auxiliary Data splitting option. Since the B and E flags in the auxiliary data packet only make sense if the packet can contain a portion of an auxiliary data unit I have adjusted the text elsewhere to match this expectation, making Auxiliary data the only exception to the “do not split a data unit across multiple packets rule”. This removes the need for use of IP fragmentation even in the extremely unlikely case of a future encoder producing Auxiliary Data Units too large to be carried within the network MTU.

I have kept the recommendation that the stream contain no oversized Auxiliary Data Units, but have noted that violating this recommendation does not make the stream impossible to transport (unlike violating the requirements listed in the rest of section 4.4).


I hope this is acceptable.

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



> On 6 Jul 2018, at 12:03, James Barrett <[e-mail]> wrote:
> 
>> 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.
> 
> Would it be clearer if I changed the second paragraph of Section 3 to read:
> 
>   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, and the type of a Data Unit is signalled in the
>   preceding Parse Info Header. The most important types are the
>   Sequence Header, which contains configuration data needed by
>   the decoder, and several types of Coded Picture, which contain
>   the coded data for the pictures themselves. Each picture
>   represents a frame in a progressively scanned video Sequence
>   or a field in an interlaced video Sequence.
> 
> Then at the start of Section 4 add:
> 
>   In this specification each RTP packet is used to carry data
>   corresponding to a single Parse Info Header and its following data
>   unit (if it has one).  A single packet MAY NOT contain data from more
>   than one Parse Info header or data unit and a single Parse Info
>   Header and Data Unit pair MUST NOT be split accross more than one
>   packet.
> 
> And change the following paragraph to read:
> 
>   This specification only covers the transport of Sequence Headers
>   (together with their accompanying data unit), High Quality Fragments
>   (together with their accompanying data unit), Auxiliary Data
>   (together with their accompanying data unit), and (optionally) End
>   Sequence Headers and Padding Data (optionally along with a data
>   unit).
> 
> 
> I think that should make the situation clearer.
> 
>> 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?
> 
> 
> Yes, as others have commented this is probably an unnecessary statement because that is the definition of the Sequence Number.
> 
> 
>> 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?
> 
> Yes.
> 
>> 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?
> 
> Yes.
> 
> 
>> 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?
> 
> 
> Would this be better?
> 
>         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.
>         Choosing between these two options is left up to the
>         implementation as it will be dependent on intended use, the
>         former may result in malformed pictures, the latter will result
>         in dropped frames.  Such an occurance is an indication either
>         of packet loss, joining a stream mid-picture, or of a non-
>         compliant transmitter.
> 
> 
>> 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?
> 
> 
> Hopefully the above revision would make this clearer. For clarification some types of parse info header are always followed by a data unit and some never are. Since the Sequence Header PI Header must always be followed by a data unit and must be the first in any sequence your second two examples are not allowed. This could be made clearer by changing paragraph 4 of Section 3 to read:
> 
>   The High Quality (HQ) profile for VC-2 restricts the types of Parse
>   Info Headers which may appear in the Sequence (and hence also the
>   types of Data Unit) to only:
> 
>   o  Sequence Headers (which are always followed by a Data Unit),
> 
>   o  High Quality Pictures (which are always followed by a Data Unit),
> 
>   o  High Quality Fragments (which are always followed by a Data Unit),
> 
>   o  Auxiliary Data (which are always followed by a Data Unit),
> 
>   o  Padding Data (which are always followed by a Data Unit), and
> 
>   o  End of Sequence (which are never followed by a Data Unit).
> 
> 
> Then combining that with the other restrictions the current definition of VC-2 allows the following structure in a stream that complies with the HQ profile:
> 
>  PI DU [ PI DU ]* PI
> 
> (Where * is “0 or more” as in regular expressions).
> 
>> 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.
> 
> Hopefully the changes above clarify this. A Data Unit type is identified by the Parse Info header that precedes it. A Sequence Header Data Unit is always preceded by a Sequence Header Parse Info Header.
> 
> 
>> 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?
> 
> 
> The parse info headers serve to structure the stream and identify the following data unit, the PC field in the packet headers essentially carries all the relevant information from the PI Header.
> 
> 
>> 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)
> 
> How about:
> 
> 
>   o  A VC-2 Sequence Header (Figure 1) (see Section 11 of the VC-2
>      specification [VC2]),
> 
>   o  A Picture Fragment containing the VC-2 Transform Parameters for a
>      Picture (Figure 2) (see Section 14 of the VC-2 specification
>      [VC2]),
> 
>   o  A Picture Fragment containing VC-2 Coded Slices (Figure 3) for a
>      picture (see Section 14 of the VC-2 specification [VC2]),
> 
>   o  The end of a VC-2 Sequence (Figure 4)(see Section 10.5.2 of the
>      VC-2 specification [VC2]),
> 
>   o  The contents of an auxiliary data unit (Figure 5)(see
>      Section 10.4.4 of the VC-2 specification [VC2]), and
> 
>   o  An indication of the presence of a padding data unit (Figure 6)
>      (see Section 10.4.5 of the VC-2 specification [VC2]).
> 
>> 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.
> 
> Yep, that needs to be corrected.
> 
>> 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..
> 
> 
> I think this might be best handled by removing the word “uncoded” here and adding the following to the first bullet point list in Section 4.4:
> 
>   o  Every Auxiliary Data Unit SHOULD be small enough that the RTP
>      packet carrying it will fit within the network MTU size.  Since
>      there is currently no specification for the format of Auxiliary
>      Data in VC-2 the mechanism for ensuring this with an encoder
>      implementation that includes Auxiliary Data Units will be
>      dependent upon the implementation's use for them.
> 
> And this to the second list:
> 
>   o  Every Auxiliary Data Unit is small enough that the RTP packet
>      carrying it will fit within the network MTU size.
> 
> In principle a VC-2 stream could have Auxiliary Data units too large to fit into the network MTU, but in practice this seems unlikely. The current VC-2 specification includes no restrictions on the contents of or size of Auxiliary Data Units beyond those imposed by the size of the fields in the PI header, but as far as I am aware the only encoders/decoders that have ever used them have used very small units. I think it is reasonable for a transmitter to reject incoming streams that violate this rule. In extremis IP fragmentation could be used to carry on oversized Auxiliary Data Unit.
> 
> --
> 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]