Re: [AVTCORE] John Scudder's No Objection on draft-ietf-payload-rtp-jpegxs-16: (with COMMENT)

[Tim Bruylants <[e-mail]>]

> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
>
> Thanks, I found this spec very readable -- modulo the fact that I have no expertise in the subject area! Below are some questions and comments I hope may be useful.
>
> I'm concerned that since the underlying ISO21122-{1,2,3} normative references are not readily available, it's not possible to do a complete review. I take it on faith that the document has > received review within the WG by subject matter experts who are conversant with, and have access to, the relevant ISO specifications.
>
> 1. Section 4.1
>
>        In the case of an interlaced frame, the
>        JPEG XS header segment of the second field SHALL be in its own
>        packetization unit.
>
> I’m confused why the second field even needs its own header segment, considering you earlier told us (§3.4) that
>
>    Both picture segments SHALL contain identical
>    boxes (i.e. concatenation of the video support box and the colour
>    specification box is byte exact the same for both picture segments of
>    the frame).
>
> Surely this means the VS and CS boxes could have been elided from the second field? (Probably they’re left in for uniformity, but I thought it worth asking.)

This repetition is required (by choice) in order to facilitate simpler hardware implementations and make all video frames (interlace and progressive) independent. The headers are typically only around 80 bytes in size, which is negligible.

> 2. Section 4.1
>
>    Due to the constant bit-rate of JPEG XS, the codestream packetization
>    mode guarantees that a JPEG XS RTP stream will produce a constant
>    number of bytes per frame, and a constant number of RTP packets per
>    frame.  To reach the same guarantee with the slice packetization
>    mode, an additional mechanism is required.  This can involve a
>    constraint at the rate allocation stage in the JPEG XS encoder to
>    impose a constant bit-rate at the slice level, the usage of padding
>    data, or the insertion of empty RTP packets (i.e. a RTP packet whose
>    payload data is empty).
>
> The “… additional mechanism is required” text is ambiguous. Does this mean to say that an implementation MUST use an (implementation-specific!) method, that makes its output CBR?
> That’s insinuated by the use of the word “required”. Or, does it mean that if an implementation wishes to render a CBR stream instead of a VBR one, it will need to adopt one of these strategies? Assuming your intent is the latter, I think the text should be clarified, for example
>
> OLD
>    To reach the same guarantee with the slice packetization
>    mode, an additional mechanism is required.
>
> NEW
>    If an implementation wishes to provide the same guarantee
>    with the slice packetization mode, it will need to use an
>    additional mechanism.

We agree and used your suggestion.

> 3. Section 4.3
>
>       In the case that the Transmission mode
>       (T) is set to 0, the slice packetization mode SHALL be used and K
>       SHALL be set to 1.
>
> Presumably the reason for this is evident to someone conversant with JPEG XS?

Yes it is a consequence of the fact that codestream packetization is always in-order, and only slice packetization mode supports out-of-order mode. We have clarified this in the text.

> 4. Section 7.1
>
>          level:  The JPEG XS level [ISO21122-2] in use.  Any white space in
>          the level name SHALL be omitted.  Examples of valid levels
>          names are '2k-1' or '4k-2'.
>
> Nit: s/levels/level/ (alternately, delete “names”).

We fixed this in the text.

>       width:  Determines the number of pixels per line.  This is an
>          integer between 1 and 32767.
>
>       height:  Determines the number of lines per frame.  This is an
>          integer between 1 and 32767.
>
> It would be less ambiguous to say “between 1 and 32767 inclusive”.

Indeed, the "inclusive" is important. We added this.