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

From: John Scudder via Datatracker <[email-masked]>
Date: Wed, 16 Jun 2021 14:13:59 -0700

John Scudder has entered the following ballot position for
draft-ietf-payload-rtp-jpegxs-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/iesg/statement/discuss-criteria.html
for more information about DISCUSS and COMMENT positions.

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

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

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.

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?

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

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

---

From: Tim Bruylants <[email-masked]>
Date: Thu, 17 Jun 2021 19:02:21 +0000

> 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
>
> I’m confused why the second field even needs its own header segment, considering you earlier told us (§3.4) that
>
> 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.

> 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

We agree and used your suggestion.

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

> Nit: s/levels/level/ (alternately, delete “names”).

We fixed this in the text.

> It would be less ambiguous to say “between 1 and 32767 inclusive”.

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