Subject: [AVTCORE] AD Review of draft-ietf-payload-rtp-jpegxs-11

From: "Murray S. Kucherawy" <[email-masked]>
Date: Sun, 2 May 2021 21:16:24 -0700

Hi there, here's my AD Review of this document.

Per the shepherd writeup, I have requested a review by the SDP Directorate.

First: In the shepherd writeup, please correct the spelling of my name.
:-)  Also, while we're in there, question (12) asks what reviews were done,
but the answer simply says that the document contains a media type
registration, which doesn't really answer the question.  Were any of the
media type review resources asked for feedback on this?  Later on the
shepherd says he reviewed it, but it doesn't say what official reviews may
have been undertaken.  As you can see later on, there's a good chunk of the
registration missing.

In Section 2, please update the BCP 14 text to use the new form (see RFC
8174, which must also be added as a normative reference).

Also in Section 2, the term "SOC Marker", though defined here, is not used
below here.  Section 3.2 references the EOC Marker; should it reference
both?

In Section 4, we encounter "SSRC" for the first time.  A definition for it
would be helpful, or a reference.

In Section 4.2, "As per specified" can just be "As specified".
Alternatively, "As per RFC 3550, ..."  While I'm thinking about it, all
these double references (e.g., "RFC 3550 [RFC3550]") throughout the
document could be cleaned up.

The diagrams in Section 4.4 show (eventually) how the EOC marker fits into
this, but not the SOC marker.  Should they?

Section 4.5 uses a SHOULD.  SHOULD leaves the implementer with a choice, so
it's a good idea to include guidance about when one might legitimately do
something other than what the SHOULD says to do.  Or, if there isn't any,
maybe this ought to be a SHALL?

In Section 6.1, it's clear what a receiver would do with some of the
optional parameters when they are absent (i.e., defaults are clear), but
not in all cases.  What can a receiver safely assume about "depth",
"width", or "height", for example, when not specified?

Also on 6.1, this media type registration is missing required fields,
including "Interoperability Considerations", "Published Specification",
"Applications that use this media type", "Fragment identifier
considerations", "Additional information" (which has four sub-fields",
"Contact information", "Intended Usage", "Restrictions On Usage", "Author",
and "Change Controller".  Please see RFC 6838.  Further, it would probably
be a good idea to mention either in the registration or in the Security
Considerations of this document that the payload being registered does not
(or does) contain code that is executable.  The media type reviewers prefer
people to be explicit on this point.

-MSK

---

From: Tim Bruylants <[email-masked]>
Date: Mon, 3 May 2021 14:57:31 +0000

Dear Murray,

Thank you for the review. We have addressed your comments in draft-ietf-payload-rtp-jpegxs-12.

Here is the detailed answer to the comments:

>In Section 2, please update the BCP 14 text to use the new form (see RFC 8174, which must also be added as a normative reference).

  We have updated the text to comply with RFC 8174.

> Also in Section 2, the term "SOC Marker", though defined here, is not used below here.  Section 3.2 references the EOC Marker; should it reference both?

  1) The term "SOC marker" is used in section 2 under "JPEG XS codestream header".
  2) We clarified section 3.2, referencing "SOC marker".

> In Section 4, we encounter "SSRC" for the first time.  A definition for it would be helpful, or a reference.

  SSRC is Synchronization Source, as defined in RFC3550. We modified the text to clarify this.

> In Section 4.2, "As per specified" can just be "As specified".  Alternatively, "As per RFC 3550, ..."  While I'm thinking about it, all these double references (e.g., "RFC 3550 [RFC3550]") throughout the document could be cleaned up.

  We have cleaned up the "double references" throughout the text.

> The diagrams in Section 4.4 show (eventually) how the EOC marker fits into this, but not the SOC marker.  Should they?

  We clarified that the SOC marker is part of the JPEG XS codestream header. The latter is clearly mentioned in the diagrams. Explicitly mentioning SOC in the diagrams would clutter the layout, and would probably require to explicitly add the other markers in the codestream header as well.

> Section 4.5 uses a SHOULD.  SHOULD leaves the implementer with a choice, so it's a good idea to include guidance about when one might legitimately do something other than what the SHOULD says to do.  Or, if there isn't any, maybe this ought to be a SHALL?

  We reworded section 4.5 slightly. The message is that actual traffic shaping and timing delivery is outside the scope of this specification and does not influence the payload packetization. We also moved the section one level up (became section 5).

> In Section 6.1, it's clear what a receiver would do with some of the optional parameters when they are absent (i.e., defaults are clear), but not in all cases.  What can a receiver safely assume about "depth", "width", or "height", for example, when not specified?

  We thought this was clarified by section 6.2.1. Nevertheless, we added a short paragraph in section 6 (now section 7).

> Also on 6.1, this media type registration is missing required fields, including "Interoperability Considerations", "Published Specification", "Applications that use this media type", "Fragment identifier considerations", "Additional information" (which has four sub-fields", "Contact information", "Intended Usage", "Restrictions On Usage", "Author", and "Change Controller".  Please see RFC 6838.  Further, it would probably be a good idea to mention either in the registration or in the Security Considerations of this document that the payload being registered does not (or does) contain code that is executable.  The media type reviewers prefer people to be explicit on this point.

  We have reworked the media type registration section to follow RFC 6838 and RFC 4855.

  We added the notion that the payload format and the JPEG XS encoding do NOT contain executable code (to the Security Considerations section).

Kind regards,
Tim Bruylants

From: avt <[email-masked]> On Behalf Of Murray S. Kucherawy
Sent: Monday 3 May 2021 06:16
To: [email-masked]
Subject: [AVTCORE] AD Review of draft-ietf-payload-rtp-jpegxs-11

First: In the shepherd writeup, please correct the spelling of my name.  :-)  Also, while we're in there, question (12) asks what reviews were done, but the answer simply says that the document contains a media type registration, which doesn't really answer the question.  Were any of the media type review resources asked for feedback on this?  Later on the shepherd says he reviewed it, but it doesn't say what official reviews may have been undertaken.  As you can see later on, there's a good chunk of the registration missing.

In Section 2, please update the BCP 14 text to use the new form (see RFC 8174, which must also be added as a normative reference).

Also in Section 2, the term "SOC Marker", though defined here, is not used below here.  Section 3.2 references the EOC Marker; should it reference both?

In Section 4, we encounter "SSRC" for the first time.  A definition for it would be helpful, or a reference.

In Section 4.2, "As per specified" can just be "As specified".  Alternatively, "As per RFC 3550, ..."  While I'm thinking about it, all these double references (e.g., "RFC 3550 [RFC3550]") throughout the document could be cleaned up.

The diagrams in Section 4.4 show (eventually) how the EOC marker fits into this, but not the SOC marker.  Should they?

Section 4.5 uses a SHOULD.  SHOULD leaves the implementer with a choice, so it's a good idea to include guidance about when one might legitimately do something other than what the SHOULD says to do.  Or, if there isn't any, maybe this ought to be a SHALL?

In Section 6.1, it's clear what a receiver would do with some of the optional parameters when they are absent (i.e., defaults are clear), but not in all cases.  What can a receiver safely assume about "depth", "width", or "height", for example, when not specified?

Also on 6.1, this media type registration is missing required fields, including "Interoperability Considerations", "Published Specification", "Applications that use this media type", "Fragment identifier considerations", "Additional information" (which has four sub-fields", "Contact information", "Intended Usage", "Restrictions On Usage", "Author", and "Change Controller".  Please see RFC 6838.  Further, it would probably be a good idea to mention either in the registration or in the Security Considerations of this document that the payload being registered does not (or does) contain code that is executable.  The media type reviewers prefer people to be explicit on this point.

-MSK

---

From: "Murray S. Kucherawy" <[email-masked]>
Date: Mon, 3 May 2021 09:57:58 -0700

On Mon, May 3, 2021 at 7:57 AM Tim Bruylants <[email-masked]> wrote:

> > Also in Section 2, the term "SOC Marker", though defined here, is not
> used below here.  Section 3.2 references the EOC Marker; should it
> reference both?
>
>
>

Right, it was referenced in the Glossary that defines it, but not
elsewhere.  That seemed strange.

>   We have cleaned up the "double references" throughout the text.

Thanks.  The RFC Editor would probably have done the same, but now your
AUTH48 might come a little sooner.

> The diagrams in Section 4.4 show (eventually) how the EOC marker fits
> into this, but not the SOC marker.  Should they?
>
>
>
>   We clarified that the SOC marker is part of the JPEG XS codestream
> header. The latter is clearly mentioned in the diagrams. Explicitly
> mentioning SOC in the diagrams would clutter the layout, and would probably
> require to explicitly add the other markers in the codestream header as
> well.
>

Fair enough.  In my layperson's reading of the work, it was odd that the
"end" marker was somehow worth mentioning explicitly but the "start" marker
was being taken for granted.

>   We thought this was clarified by section 6.2.1. Nevertheless, we added a
> short paragraph in section 6 (now section 7).
>
>
This could also be because I'm unfamiliar with the base work.  It might be
useful to mention in the registration that optional parameters are in fact
overrides to metadata within the payload, since I think that's what you're
telling me here.  But that's not a blocker for starting Last Call.

>   We added the notion that the payload format and the JPEG XS encoding do
> NOT contain executable code (to the Security Considerations section).
>

Great, thanks!  I'll prepare the Last Call request now.

-MSK