Subject: Re: [AVTCORE] AD review of draft-ietf-avtcore-cc-feedback-message-07

From: Colin Perkins <[email-masked]>
Date: Wed, 2 Sep 2020 23:10:31 +0100

> On 23 Jul 2020, at 22:38, Barry Leiba <[email-masked]> wrote:
> 
> Thanks for the work on this clear, easy-to-read document, and I'm
> sorry it took me a while to get through the workload to review it.
> Here's my AD review, and I'll set the state to "revised I-D needed"
> and start last call after -08 is posted.
> 
> Everything here is minor; please let me know if you disagree with
> anything below.
> 
> — Section 1 —
> Please include reference citations for RTP and RTCP.

Done.

>   As a direct result,
>   the different congestion control (i.e., rate adaptation) designs are
>   not interoperable.
> 
> Is “i.e.,” the right term here, or do you mean “such as”?  I would
> also move the parenthetical one word later, rather than having it
> split the phrase “congestion control designs”.

I meant “i.e.”, but it’s probably clearer to just remove the parenthetical.

> — Section 2 —
> Please use the current BCP 10 boilerplate, found in RFC 8174, and add
> a normative reference to that RFC.

Done.

> — Section 3 —
> 
>   o  RTP sequence number: The receiver of an RTP flow needs to feedback
>      the sequence numbers of the received RTP packets to the sender
> 
> Nit: “feed back”, as a verb, should be two words.  Or perhaps “needs
> to feed the sequence numbers … back to the sender”.  (Check other uses
> of “feedback” as a verb also.)

Fixed.

> — Section 3.1 —
> 
>   The contents of each 16-bit packet metric block comprises the L, ECN,
>   and ATO fields are as follows:
> 
> Nit: It looks like “are” is an extra word here, yes?

Fixed.

>      1 represent the
>      packet was received and the subsequent bits in the block need to
>      be parsed.
> 
> Nit: “1 represents that the”

Fixed.

> — Section 5 —
> 
>   However, if
>   multiple consecutive congestion control feedback packets are lost,
>   the sender SHOULD rapidly reduce its sending rate towards zero, as
>   this likely indicates a path failure.
> 
> Doesn’t “reduce” already mean “towards zero”?  Is something further
> meant by saying “towards zero” that should be made clearer (I suspect
> so)?

Nothing further meant. Fixed.

> — Section 6 —
> 
>   When used with "ccfb" feedback, the wildcard payload type ("*") MUST
>   be used.
> 
> This doesn’t read well as written, and sorta doesn’t make much sense.
> I think you just want to say this:
> 
> NEW
>   The payload type used with “ccfb” feedback MUST be the wildcard type (“*”).
> END

Agree, fixed.

> — Section 8 —
> 
>   An efficient congestion control algorithm requires more fine grained
>   information on per packet reception quality
> 
> Nit: hyphenate “fine-grained” and “per-packet”.  Also hyphenate the
> former in the following sentence.

Fixed.

>      This is the opposite of the sender based congestion
>      control approach suggested in this memo, so TMMBR cannot be used
>      to convey the information needed for a sender based congestion
>      control.
> 
> Nit: hyphenate “sender-based” in both places, and also at the end of
> the section.

Fixed.

> — Section 11 —
> 
>   at a greater rate than the path can support, thereby congesting the
>   path.
> 
> Is “congesting” really used as a verb generally?  Better as “thereby
> causing congestion on the path.”

I use “congesting” in that way, but perhaps English as used in Scotland is different to more international uses :) Your suggestion is also fine.

I’ll submit -08 with these fixes.

Cheers,
Colin

---

From: Barry Leiba <[email-masked]>
Date: Wed, 2 Sep 2020 19:15:56 -0400

Thanks, Colin, for addressing my pickiness.  😁

Barry

On Wed, Sep 2, 2020 at 6:10 PM Colin Perkins <[email-masked]> wrote:

> I use “congesting” in that way, but perhaps English as used in Scotland is
> different to more international uses :) Your suggestion is also fine.
>
>
>