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

[Stephan Wenger <[e-mail]>]

HI Francesca,
Thanks very much for clearing the discuss, and we will implement as documented below.
Stephan

On 6/20/22, 03:02, "Francesca Palombini via Datatracker" <[e-mail]> wrote:

    Francesca Palombini has entered the following ballot position for
    draft-ietf-avtcore-rtp-vvc-16: 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/about/groups/iesg/statements/handling-ballot-positions/ 
    for more information about how to handle DISCUSS and COMMENT positions.


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



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

    # ART AD Review of draft-ietf-avtcore-rtp-vvc-16

    cc @fpalombini

    Thank you for the work on this document, and for addressing my previous
    DISCUSSes.

    Thanks for posting the media-types review request
    https://mailarchive.ietf.org/arch/msg/media-types/iinskT_KIviiCsmnL32ql4PuQfU/
    and thanks to Martin Dürst for his review.

    Re the IANA registration: in recent years we have preferred to use "IETF" for
    change controller, to indicate that this comes from a consensus document, as
    document in
    https://datatracker.ietf.org/doc/html/draft-leiba-ietf-iana-registrations-00.
    So in this case I would suggest using "IETF <[e-mail]>". I am ok with
    no changes for the other review comments, thanks for the replies.

    Francesca

    ## Comments

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

    EDIT: from the authors - "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. " I
    suggest adding a sentence clarifying the above to avoid confusion in the reader.

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

    ### Wrong reference

    Section 4.3:
    ```
          header.  This payload structure is specified in Section 4.4.1.
    ```
    4.4.1 should be 4.3.1.

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

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

    ## Notes

    This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
    [`ietf-comments` tool][ICT] to automatically convert this review into
    individual GitHub issues.

    [ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
    [ICT]: https://github.com/mnot/ietf-comments