Re: [Last-Call] Artart last call review of draft-ietf-avtcore-rtp-evc-05

[Marc Blanchet <marc.blanchet@xxxxxxxxxxx>]

Le 2 oct. 2023 à 21:29, Stephan Wenger <stewe@xxxxxxxxx> a écrit :

Hi Marc, all,

We appreciate your review.  Please see inline.  Unless we hear otherwise,

My comments were essentially editing suggestions. So I leave it to the group/editors/AD to decide what to do with them. 

One additional comment below. No need to reply. 

Regards, Marc.

we will address the issues labelled as “FIX” only in our working copy.

Stephan

 

From: Marc Blanchet via Datatracker <noreply@xxxxxxxx>
Date: Saturday, September 30, 2023 at 08:39
To: art@xxxxxxxx <art@xxxxxxxx>
Cc: avt@xxxxxxxx <avt@xxxxxxxx>, draft-ietf-avtcore-rtp-evc.all@xxxxxxxx <draft-ietf-avtcore-rtp-evc.all@xxxxxxxx>, last-call@xxxxxxxx <last-call@xxxxxxxx>
Subject: Artart last call review of draft-ietf-avtcore-rtp-evc-05

Reviewer: Marc Blanchet
Review result: Ready with Nits

I've been assigned as ART reviewer for this draft. I am no expert in RTP
payload, so this review did not verify the validity of the technical solution,
if is sound or not, etc., while I did read it with the most diligence I could.

with the i18n eye, since no user/generic strings seems to be used, don't see
any i18n issues.

Comments (not really substantive to the protocol):
- section 7.3.2.1: "An implementation SHOULD be able to understand all media
type parameters (including all optional media type parameters), even if it
doesn't support the functionality related to the parameter. ". This sentence
seems weird to me. If a new media type parameter is defined and the
implementation is already used in the field, then there is no way that the
implementation will be able to understand that parameter. Maybe I’m not
understanding the context here. 

A comment like this has come up before, but we claim the sentence is clear to those implementing payload formats.  To explain, all parameters defined in the media type registration are optional.  That means, they do not need to be included in the SDP that’s being used in negotiation, and senders who don’t plan to ever send an optional parameter obviously do not have to implement the sending of it.  However, that does not mean that a receiver is at the same liberty.  A receiver really SHOULD implement receiving, and parsing all optional parameters and ideally have application logic that decides whether it can possibly accept a media stream advertised with a parameter that it’s sending part would never send.  It’s common implementation practice to take shortcuts here, which is why we provide the aforementioned guidance.  However, nothing seriously breaks if an implementer does not follow the recommendation, as there are defaults for each optional parameter.  Worst case, the media would fall back to a basic interoperability mode (like: small picture size, relatively low frame rate).  Also, there are valid reasons for not implementing the SDP part of the RFC at all, for example if the session negotiation is conducted using Jingle or something.  Therefore, SHOULD seems to be right.

- section 9: "the RTP packet is RECOMMENDED but
NOT REQUIRED based on the thoughts". Should this be rephrased using SHOULD and
MUST NOT to use RFC2119 wording? 

FIX: RECOMMENDED and REQUIRED are both RFC2119 keywords.  We should lower-case the “NOT” in “NOT REQUIRED”.  Otherwise, I think this is a style question only.

- section 9: "For example, it would be a bad
and insecure implementation practice to forward any _javascript_ a decoder
implementation detects to a web browser.". Cannot parse this sentence. Maybe it
is missing a « to » or something. Or it is me. 

FIX: I’m not an English native speaker, nor is any one of my co-authors.  However, the sentence seems fine.  That said, we could rephrase to “For example, forwarding all received _javascript_ code detected by a decoder implementation to a web-browser unchecked would be a bad and insecure implementation practice.”.  Does that work better for you? 

Yes. The problem (just understanding the sentence, not about what it intends to convey, which I think I got) I had was that part: "any _javascript_ a decoder implementation detects to”. So new sentence is easier for me to understand.

Note that there are good and valid reasons for receiving and forwarding _javascript_.  I have seen a prototype doing just that to get a display set up.  But we want to guide away from a naïve implementation that find a string in an SEI message, figures out it may be _javascript_, and forwards it without having a clear idea what’s going on and from whom that _javascript_ comes and what it is intended to do.

- section 10: Congestion
Control. is after the Security section. Typically, Security is the last section
before IANA. No big issue here. Maybe RTP docs do ordering differently. More an
observation than real issue.

Thanks.  In this, like in many other places, we follow the VVC payload that became an RFC 9328 not a year ago.  I don’t mind moving sections around, but also do not see a necessity.

Nits:
- s/MPGEG-2/MPEG-2/ (unless MPGEG is something I don't know)

FIX.  Typo, will be fixed.

- section 7.3.2.1, figure 11: the values "O" and "X" are not used in the table.
maybe remove them?

FIX: Leftover from RFC9328, VVC payload format.  Will be removed.

Orthography suggestions (also depends on UK vs US orthographies):
- s/neighbor/neighbour/
- s/signaled/signalled/
- s/signaling/signalling/
- s/behavior/behaviour/

We prefer the US spelling as that is what ISO/IEC is using—and this payload format is there to transport data according to an ISO/IEC spec.

-- 
last-call mailing list
last-call@xxxxxxxx
https://www.ietf.org/mailman/listinfo/last-call