Re: [AVTCORE] Francesca Palombini's Discuss on draft-ietf-avtcore-rtp-vvc-16: (with DISCUSS and COMMENT)

[Bernard Aboba <[e-mail]>]

I asked for an SDP review but not a media-types review.

Stephan - can you initiate and CC me and Jonathan?

On Wed, Jun 15, 2022 at 12:07 PM Stephan Wenger <[e-mail]> wrote:

> Hi Francesca,
>
> Thanks for your review.  Some comments inline in blue.
>
> To summarize: small editorial changes to respond to one discuss and
> several of the comments.  No action on others.  Media type review unclear,
> Bernard or we can initiate.
>
> Stephan
>
>
>
>
>
> On 6/15/22, 09:53, "Francesca Palombini via Datatracker" <
> [e-mail]> wrote:
>
>
>
> […]
>
>
>
>
>
>     The document, along with other ballot positions, can be found here:
>
>     https://datatracker.ietf.org/doc/draft-ietf-avtcore-rtp-vvc/
>
>
>
>
>
>
>
>     ----------------------------------------------------------------------
>
>     DISCUSS:
>
>     ----------------------------------------------------------------------
>
>
>
>     # ART AD Review of draft-ietf-avtcore-rtp-vvc-16
>
>
>
>     cc @fpalombini
>
>
>
>     Thank you for the work on this document.
>
>
>
>     I have two DISCUSS points - hopefully easy to resolve - and a few non
> blocking
>
>     comments, but answers will be appreciated.
>
>
>
>     Francesca
>
>
>
>     ## Discuss
>
>
>
>     ### DONL and NALU size in figures 5 and 6
>
>
>
>     Section 4.3.2:
>
>     ```
>
>        The first aggregation unit in an AP consists of a conditional 16-bit
>
>        DONL field (in network byte order) followed by a 16-bit unsigned
> size
>
>        information (in network byte order) that indicates the size of the
>
>     ```
>
>     Which indicates DONL to be a 16-bit field, but in the figure 5 DONL
> appears to
>
>     be 24 bits.
>
>
>
>     ```
>
>        An aggregation unit that is not the first aggregation unit in an AP
>
>        will be followed immediately by a 16-bit unsigned size information
>
>        (in network byte order) that indicates the size of the NAL unit in
>
>     ```
>
>
>
>     Same for the NALU size: 16 bits in the paragraph above, but 24 bits in
> figure 6.
>
>
>
> Aggregation units can start and end at octet boundaries.  We tried to
> emphasize that by having the first octet in the 32-bit dword belonging to
> something else.  That’s why there’s the colon between bit 7 and bit 8.  The
> colon signifies the start and end of the aggregation unit.  We think that’s
> inline with the common “syntax” of ASCII drawings in RFCs.
>
> If you want, we can put letters (“unrelated”) into bits 0..7 in both
> figs.  Or, add a sentence saying that in the figs the aggregation unit is
> delimited by the colon.  Or do nothing.  Please let us know.
>
>
>
>     ### IANA Media type review request missing
>
>
>
>     As specified by RFC6838, it is strongly encouraged to post the media
> type
>
>     registration to the media-types mailing list for review (see
>
>
> https://mailarchive.ietf.org/arch/msg/media-types/3_DukpPWrpkTXO-zynjJlShtC1w/
>
>     for an example of a  registration review). Is there any reason this
> was not
>
>     done here? If not, please post to the media-types mailing list, and I
> will
>
>     remove the discuss with no objections raised in a week or so.
>
>
>
> Didn’t Bernard ask for a media type review a while back?  If not,
> Bernard, would you do so, or should I?
>
>
>
>     ----------------------------------------------------------------------
>
>     COMMENT:
>
>     ----------------------------------------------------------------------
>
>
>
>     ## Comments
>
>
>
>     ### Values from \[VVC\] undefined
>
>
>
>     In section 3.1.1, there are a number of values that are not defined:
> GDR_NUT,
>
>     CRA_NUT, IDR_W_RADL, IDR_N_LP. I understand these come from \[VVC\]
> and are
>
>     reported as is, however they make the text harder to parse since to
> reference
>
>     to these values is given.
>
>
>
> It’s just numbers, and those numbers are meaningless without an
> understanding of the VVC spec.  Generally, the RFC-to-be is unimplementable
> without a good understanding of the high level syntax of the VVC spec—which
> is quite common for RTP payload format.  Suggest no change here.
>
>
>
>     ### Wrong reference
>
>
>
>     Section 4.3:
>
>     ```
>
>           header.  This payload structure is specified in Section 4.4.1.
>
>     ```
>
>     4.4.1 should be 4.3.1.
>
>
>
> Correct, and this needs to be updated and the hyperlink added.  Suggest
> handling it during AUTH48, as it is a clerical error.
>
>
>
>     ### sprop-max-don-diff
>
>
>
>     sprop-max-don-diff appears first in section 4.3.1 - it would be good
> to add a
>
>     reference to 7.2, where its meaning is defined.
>
>
>
> Yes, and you are the second AD to spot this.  Suggest having a forward
> reference at the first occurrence, can do so during AUTH48.
>
>
>
>     ### Base 64
>
>
>
>     In Section 7.2, Base64 is used - please specify if the encoding
> follows "Base
>
>     64 Encoding" (Section 4) or "Base 64 Encoding with URL and Filename
> Safe
>
>     Alphabet" (Section 5) of RFC 4648. (This can easily be done in one
> sentence,
>
>     rather than repeated everytime base64 is mentioned).
>
>
>
> Will implement as suggested.  AUTH48?  There are a number of occurances…
>
>
>
>
>