Re: [AVTCORE] John Scudder's No Objection on draft-ietf-avtcore-rtp-evc-06: (with COMMENT)

[John Scudder <[e-mail]>]

Hi Stephan,

Thanks for your detailed reply. Two followups below. I have edited out the rest for brevity, anything I have not commented on, is agreed.

On Dec 13, 2023, at 12:15 PM, Stephan Wenger <[e-mail]<mailto:[e-mail]>> wrote:



Hi John,

Excellent review.  Thanks.  Please see my responses inline.  I apologize for the wordcount, but I think that such a thoughtful review requires quite a bit of context and explanation.

Would you please sign off on the resolution of the MUST RFC 2119 codeword in section 6?  While you have not raised it to DISCUSS level, I have mentally done so for myself :-)


Done.


Thanks,
Stephan

From: John Scudder via Datatracker <[e-mail]<mailto:[e-mail]>>
Date: Wednesday, December 13, 2023 at 04:32
To: The IESG <[e-mail]<mailto:[e-mail]>>
Cc: [e-mail]<mailto:[e-mail]> <[e-mail]<mailto:[e-mail]>>, [e-mail]<mailto:[e-mail]> <[e-mail]<mailto:[e-mail]>>, [e-mail]<mailto:[e-mail]> <[e-mail]<mailto:[e-mail]>>, [e-mail]<mailto:[e-mail]> <[e-mail]<mailto:[e-mail]>>, [e-mail]<mailto:[e-mail]> <[e-mail]<mailto:[e-mail]>>
Subject: John Scudder's No Objection on draft-ietf-avtcore-rtp-evc-06: (with COMMENT)
[…]

## COMMENTs

### Section 1.1.4, Type field description appears to be wrong

As best I can tell from my skim of [EVC], the type field, although six bits,
can only represent values from 0 to 62, not the more obvious 0 to 63, and uses
an oddball encoding, viz

   NalUnitType = nal_unit_type_plus1 − 1

[…]
Thanks for this!  We will investigate further and if there were a bug, we will correct it.  That will require some thinking.  If there were indeed a bug, the same bug is likely present in a number of RFCs specifying payload formats of the H.26x series, so this is a comparatively big deal for us!
As you were curious for the reasoning of the numbering range: the syntax element nal_unit_type_plus1 (lower case, with underscores) represents bits on the wire, and has a value range of 1..63.  The value 0 is reserved to prevent start code emulation.  Historically (pre 2003), video codec bitstreams used 15 or 23 or more consecutive zero bits followed by a 1 bit to indicate the start of a coded picture.  In H.264 (ca. 2003), the startcode was placed to the start of each NAL unit.  The NAL unit header is one of the few syntax elements that are outside of the start code emulation prevention mechanism of section 7.4.2.1., for reasons I could explain, but that would take a lot of letters.  We had to make sure that in the 16 bit NAL unit header there are at least one nonzero bit, and that was solved by disallowing 0 for nal_unit_type_plus1.  Once that value is parsed, the logic of the standard is such that the value of nal_unit_type_plus1 minus 1 is assigned to a variable NalUnitType (identified by capitalization and no underscores), and only that variable is used throughout the rest of the EVC spec.  The value rage of that variable is 0..62 inclusive.

Thanks for the detailed explanation! I rely on you to make any changes you deem necessary, but the essence of what I identified as a possible problem is that the draft as it stands says "This field specifies the NAL unit type”, but as you explain above, the nal_unit_type_plus1 bits on the wire are different from the NalUnitType, and therefore also different from the “NAL unit type”; the mapping from “this field” to “the NAL unit type” is not the identity function as the text implies. Probably the fix is something like,

OLD:
      nal_unit_type_plus1.  This field specifies the NAL unit type as in
      Table 4 of [EVC].  If the value of this field is less than and
      equal to 23, the NAL unit is a VCL NAL unit.  Otherwise, the NAL
      unit is a non-VCL NAL unit.  For a reference of all currently
      defined NAL unit types and their semantics, please refer to
      Section 7.4.2.2 in [EVC].

NEW:
      nal_unit_type_plus1.  This field allows the NAL Unit Type to be
      computed. The NAL Unit Type is equal to the value found in this
      field, minus 1, in other words,

         NalUnitType = nal_unit_type_plus1 − 1

      The NAL unit type is detailed in
      Table 4 of [EVC].  If the value of NalUnitType is less than or
      equal to 23, the NAL unit is a VCL NAL unit.  Otherwise, the NAL
      unit is a non-VCL NAL unit.  For a reference of all currently
      defined NAL unit types and their semantics, please refer to
      Section 7.4.2.2 in [EVC]. Note that nal_unit_type_plus1
      MUST NOT be zero.

Optionally, you could add something like "also note that as a consequence of this encoding, the possible values of NAL unit type are in the range of 0 to 62." But perhaps given the other verbiage, this is unnecessary.

This is just an illustrative example, I don't expect you to adopt my wording, although if you want to you're certainly welcome.

[…]
### Section 6

“All normal RTP mechanisms related to buffer management apply. In particular,
duplicated or outdated RTP packets (as indicated by the RTP sequence number and
the RTP timestamp) are removed. To determine the exact time for decoding,
factors such as a possible intentional delay to allow for proper inter-stream
synchronization, MUST be factored in.”

Normally I expect that a MUST will be more specifically prescriptive without
requiring the implementor to exercise imagination or creativity. The one here
seems to be of the form “implementations MUST do the right thing” which seems
not that helpful to the implementor. It’s not wrong as far as it goes, but the
RFC 2119 keyword seems out of place.

Another tricky one here.  Thanks for your diligence!
The language you are reciting above has been around verbatim (plus/minus capitalization of the word “MUST”; see below) since RFC 3984, ca. 2005.  In RFC 3984, the depacketization section was labelled as informative and hence sloppiness in drafting was to be expected 😊  RFC 7798 (HEVC payload, 2016) inherited the language.  The section there was normative, but “must” was not capitalized while the “convention” section 2 said that only capitalized MUSTs indicate a normative behavior.  Insofar, in RCC 7798, that lower-case “must” was to be interpreted as plain language.  I vaguely recall discussion in the working group at the time along the lines of your argument, and the outcome speaks for itself.  It was not a drafting mistake.  Fast forward to 2021 and the VVC payload, RFC 9328.  Here, we capitalized the MUST, and no comments were received from the WG or during IETF last call, nor from the IESG.  I don’t recall a conscious decision in this direction, and, after your comment, do consider it a drafting mistake.
Based on your comment, I think RFC3984 and RFC7798 were doing the right thing in not stating an in practice not actionable “MUST” requirement.  The minimum change would be to change the “MUST” to a “must”, arriving at plain language.  Would that address your comment?  If so, I will also submit an erratum against RFC 9328 requesting the same change.

That would be just fine. Thanks for the careful analysis.

Regards,

—John