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

[John Scudder via Datatracker <[e-mail]>]

John Scudder has entered the following ballot position for
draft-ietf-avtcore-rtp-evc-06: 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/about/groups/iesg/statements/handling-ballot-positions/ 
for more information about how to handle DISCUSS and COMMENT positions.


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



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

First, I think it's worth explaining in your description of the field that the
encodable range is 0 to 62, not the more obvious 0 to 63 one would normally
expect from a 6-bit field. I bumped into this as a consequence of being puzzled
by paragraph 4 of Section 6, wondering why value 63 was missing.

Second and more important, I think your paragraph has an off-by-one error. You
have, "If the value of this field is less than and equal to 23, the NAL unit is
a VCL NAL unit." But the field is nal_unit_type_plus1, not NalUnitType. [EVC]
says NalUnitType <= 23 is a VCL NAL unit... which means nal_unit_type_plus1 <=
24 is a VCL NAL unit.

Finally, I think it's inaccurate to write "nal_unit_type_plus1. This field
specifies the NAL unit type", assuming the field does contain
nal_unit_type_plus1 as the name implies. Per [EVC], NalUnitType is a derived
quantity, not the raw field value. Anyway, I've probably belabored the point
more than is necessary. Suffice it to say, I think you should carefully review
this definition and the other parts of the document that reference NAL unit
type.

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

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

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

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

## NITS

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

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

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