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

[Francesca Palombini <[e-mail]>]

Hi Stephan,

Thanks for the quick reply – replies inline prefaced with FP

Thanks,
Francesca

From: Stephan Wenger <[e-mail]>
Date: Wednesday, 15 June 2022 at 21:08
To: Francesca Palombini <[e-mail]>, The IESG <[e-mail]>
Cc: [e-mail] <[e-mail]>, [e-mail] <[e-mail]>, [e-mail] <[e-mail]>, [e-mail] <[e-mail]>
Subject: Re: Francesca Palombini's Discuss on draft-ietf-avtcore-rtp-vvc-16: (with DISCUSS and COMMENT)

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.



FP: Ah, I see. I was wondering what that “:” signified.. Then I suggest to add a sentence saying that, rather than modify the figure, just to clarify to the reader. Anyway – this is not worth blocking the document over, and I will update my DISCUSS accordingly.



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



FP: If he did, apologies I have missed it – do let me know and I will also remove this DISCUSS point, if not, this should not be a problem but it will delay the document a few days.



    ----------------------------------------------------------------------

    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.



FP: I understand, and I got that they are supposed to be understood from the VVC spec. My comment was for readability purposes – but I am ok with no changes.



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



FP: sounds good.



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



FP: Thanks.



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



FP: I’d suggest fixing it with a new revision before AUTH48, especially if you have more non-blocking points from other AD reviews, but as long as it is clarified, I am happy :)