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

[Stephan Wenger <[e-mail]>]

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

Thanks,
Stephan

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

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Thanks for this document, which I found to be well-constructed and well-written
if one allows for my complete lack of expertise in the subject area. I have a
few comments below that I hope might be helpful.

(Ballot revised to correct a cut and paste error, no substantive changes.)

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


### Section 4.3.3

“If an FU is lost, the receiver SHOULD discard all following fragmentation
units in transmission order corresponding to the same fragmented NAL unit
unless the decoder in the receiver is known to gracefully handle incomplete NAL
units.”

I assume you don't want to get into the weeds of telling implementations how to
determine whether an FU was lost, or if it was merely out-of-order, delayed,
etc. I wonder if this should be reworded in terms of saying that a fragment
shouldn't be presented to the decoder unless all prior fragments have already
been presented.

Also, this paragraph appears to conflict with paragraph six of Section 6, which
as I read it, tells me that the decoder should only be presented with a
complete reassembled NAL unit. The text above seems to say that it's OK to
truncate, the only thing not allowed is presenting a NAL with a hole in the
middle.

This issue was also spotted by Eric and is subject of his discuss.  We will fix it.


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

### Section 6, paragraph 5, not-completely-minor nit

I haven't noted most editorial/idiom things I noticed, on the assumption the
RFC editor will do a better job with them than I would anyway. But in Section 6
there is "make a difference from”, I assume what you mean here is "distinguish
it from". If not, please say more.
Will fix.


### Section 6, paragraph 9, one last not-utterly-trivial nit

“After initial buffering, decoding, and playback are started, and the
buffering-while-playing mode is used.”

I think this should be, “After initial buffering, decoding and playback are
started, and the buffering-while-playing mode is used.” My edit deletes one
comma. Sorry for the picky edit, it could make a difference for
clarity/ambiguity so I'm calling it out. (When Grammarly saw my edit, it wanted
to restore the comma I removed. Grammarly is wrong in this instance.)

Will fix.

## NITS

- It’s regrettable that a lot of abbreviations are used in Section 1, but
they’re only defined in Section 3.

It is, and we had that discussion time and time again over the generations of video payloads, both in the WG and in IESG reviews.  The justification for the forward reference is that to the likely reader (outside of review processes), the vast majority of those acronyms are known, and that spelling the out would not be helpful as they are, also in their vast majority, terms of art to those in the know, and just gibberish to the rest.  If we were putting pages of acronym definitions before any useful text, the document not only looks awkward but was previously found close to unreadable.  I prefer to not act on this nit.

- It seems a little odd that only some Section 1.1 subsections are labeled
“informative”. Isn’t the entire description here non-normative, since the
actual payload spec is [EVC]? (And is "non-normative" what you meant by
“informative”?)
Subsection 1.1.4 is NOT labelled as informative, because the NAL unit header is being referred to later in a normative context.  This was subject to WG discussion (I think in the context of a shepherd’s review, but may be wrong).
“Informative” is ISO language.  Unless there’s a clear preference in the IETF of using “non-normative” over “informative”, I prefer not to change that word, because the key audience (implementers) are in our experience more familiar with ISO specs than with IETF RFCs.  (To integrate an EVC video codec in a system using this RTP payload format, you need to understand a good part of the 340+ pages EVC spec.  You need to grok the payload spec.  You need to understand typically only a small part of RTP (RFC 3550)—for example all the multicast support language is a no-op in practice for almost all systems.  And, you don’t generally care about the rest of the IETF’s body of work.)


- Although the specification appears to be clear and well written from the
point of view of consumption by someone who is already an expert in the field
(or so I'm guessing!) it's a somewhat dense read for a non-expert. Some of this
is unavoidable, but there is also some avoidable density due to the use of
abbreviations with no obvious mnemonic.
See above.  The intended audience are indeed people who are familiar with dense specs.  You did look at the EVC spec—that’s dense.  Compared to the EVC spec, we are positively verbose and over-explanatory here.
One example is DONL. Another example of
a symbol that would've helped my understanding to have glossed much earlier is
“sprop”. Eventually, I found Section 7 that enabled me to infer it's short for
"stream properties". Too late for me, but maybe not for future readers.

The concise—perhaps over-concise—ISO -style language surrounding interleaving and DONL has been identified as a pain point during reviews before.  However, it has never been a pain point to the intended readers and implementers.  I haven’t been involved in interop events for more than a decade, but in that distant past when I was involved, the DON/DONL mechanism of RFC3984 has not been an issue—those people who wanted to use interleaving, got it right.  I also don’t recall a single email to myself or to the AVT mailing list asking for clarification, outside of review processes like this one.
The situation with sprop is a bit more complicated, mostly because (independent of the terminology) the concept of stream properties in SDP was new when we introduced it in RFC 3984 IIRC.  We had real, new, technical complexity here.  So we had questions, and (I guess) people probably got a thing or two wrong.  However, since then, those who care about SDP (today mostly the webrtc crowd) either got it, or they simply don’t use those optional codepoints.  Be it as it may, I have not heard about confusion surrounding sprop in the last several years either, and that despite webrtc becoming more mainstream.  Perhaps that is because all the sprop stuff is optional and mostly useful for optimizing resource management in the decoder—something that was critical in 2005 but is rapidly becoming a non-issue with systems becoming more powerful every year.
I don’t believe we should act on this nit.