[AVTCORE] John Scudder's No Objection on draft-ietf-avtcore-rtp-evc-06: (with COMMENT)
John Scudder via Datatracker <noreply@ietf.org> Tue, 12 December 2023 22:32 UTC
Return-Path: <noreply@ietf.org>
X-Original-To: avt@ietf.org
Delivered-To: avt@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id 2BB30C14F61A; Tue, 12 Dec 2023 14:32:44 -0800 (PST)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
From: John Scudder via Datatracker <noreply@ietf.org>
To: The IESG <iesg@ietf.org>
Cc: draft-ietf-avtcore-rtp-evc@ietf.org, avtcore-chairs@ietf.org, avt@ietf.org, bernard.aboba@gmail.com, bernard.aboba@gmail.com
X-Test-IDTracker: no
X-IETF-IDTracker: 12.0.0
Auto-Submitted: auto-generated
Precedence: bulk
Reply-To: John Scudder <jgs@juniper.net>
Message-ID: <170242036417.34935.15813429835240964704@ietfa.amsl.com>
Date: Tue, 12 Dec 2023 14:32:44 -0800
Archived-At: <https://mailarchive.ietf.org/arch/msg/avt/QkwdyOzQ_Be2tJ2tkOC4JP-tUu4>
Subject: [AVTCORE] John Scudder's No Objection on draft-ietf-avtcore-rtp-evc-06: (with COMMENT)
X-BeenThere: avt@ietf.org
X-Mailman-Version: 2.1.39
List-Id: Audio/Video Transport Core Maintenance <avt.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/avt>, <mailto:avt-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/avt/>
List-Post: <mailto:avt@ietf.org>
List-Help: <mailto:avt-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/avt>, <mailto:avt-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 12 Dec 2023 22:32:44 -0000
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. ## 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(Quinn Grammarly saw my edit, it wanted to restore, I removed. Grammarly is wrong.) - 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.
- [AVTCORE] John Scudder's No Objection on draft-ie… John Scudder via Datatracker