Re: [payload] Review of draft-ietf-payload-rtp-aptx-02

"John Lindsay" <Lindsay@worldcastsystems.com> Mon, 21 October 2013 17:25 UTC

Content-class: urn:content-classes:message
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Date: Mon, 21 Oct 2013 18:25:32 +0100
Message-ID: <8C4E0C2409735E4FBC22D754A238F94D02FDEAA9@APTSBS.apt.local>
In-Reply-To: <C15918F2FCDA0243A7C919DA7C4BE9940E71F203@xmb-aln-x01.cisco.com>
Thread-Topic: Review of draft-ietf-payload-rtp-aptx-02
Thread-Index: AQHOzdbw2vjQftbAz0q6DC+dHfSdj5n/aArg
References: <C15918F2FCDA0243A7C919DA7C4BE9940E71F203@xmb-aln-x01.cisco.com>
From: John Lindsay <Lindsay@worldcastsystems.com>
To: "Ali C. Begen (abegen)" <abegen@cisco.com>, payload@ietf.org
Cc: draft-ietf-payload-rtp-aptx@tools.ietf.org
Subject: Re: [payload] Review of draft-ietf-payload-rtp-aptx-02
Precedence: list

Hi

A new doc has been submitted in response to comments.

See notes below.

John


-----Original Message-----
From: Ali C. Begen (abegen) [mailto:abegen@cisco.com] 
Sent: 20 October 2013 21:57
To: payload@ietf.org
Cc: draft-ietf-payload-rtp-aptx@tools.ietf.org
Subject: Review of draft-ietf-payload-rtp-aptx-02

The WGLC has ended a few weeks ago and I just completed my review.
Everything looks good except a few typos. In general, I would avoid
using "ms" until it is defined "milliseconds (ms)".
> Changed ms to milliseconds throughout the doc.
> Ran doc through spell checker but didn't change very much if you have
> specifics let me know

Authors, if you can submit a revision by tomorrow (21st of Oct.), we can
ship this right away.

Thanks,
-acbegen

Other comments regarding media subtype registration (Thanks to Bjoern
Hoehrmann):

1) The registration in Section 6.1 uses the old template. 
The current template is here:
http://tools.ietf.org/html/rfc6838#section-5.6
> Updated please ensure you agree

2) Regarding ptime: It might be better to say "Defaults to 4
(milliseconds)", otherwise some might be confused whether the `ms` is
part of the value.
> Done

3) Regarding stereo channels: If the value includes characters like `{`
then in protocols like HTTP the value would have to be enclosed in
double quotes since `{` is not allowed as a bare token in that protocol.
The situation may be similar in RTP, and if so, that's not sufficiently
clear from the text above. The same goes for some other parameters and
other characters like `,`.
> I did not change this as these parameters are passed by SDP and are
allowed. In addition they are used by everyone who has implemented so
far without issue??

Is there some other way that we can use to avoid this potential problem?

4) Regarding embedded-autosync-channels: This should be rephrased to
avoid combining "MUST" and "only"; it is not clear whether specifying
more than the first channel is optional or forbidden. Same for the
`embedded-aux-channels` parameter.
>>> Removed the "only" as it still has the same meaning as they are
pairs hence it can only refer to Channel 1 or 2

5) Regarding the encoding considerations: This should say "framed". 
>>> Don't understand issue? Can you give further explanation?

6) "Restrictions on usage" field can say "RTP applications only".
> Not sure what is meant by this?
>Changed Intended usage: RTP applications only at the bottom of page 14?
Can you ensure you agree?

7) Person & email address to contact for further information: This
should be "John Lindsay email:lindsay@worldcastsystems.com"
> Again cannot see the issue? Is this supposed to be in quotes?
> Changed to quotes but not 100% sure this was the issue?
> Person & email address to contact for further information: "John  
> Lindsay email:lindsay@worldcastsystems.com"

8) Author/Change controller: This should say "IETF Payload Working Group
delegated from the IESG"
> Done

[payload] Review of draft-ietf-payload-rtp-aptx-02 Ali C. Begen (abegen)
Re: [payload] Review of draft-ietf-payload-rtp-ap… John Lindsay
Re: [payload] Review of draft-ietf-payload-rtp-ap… Ali C. Begen (abegen)