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

From: "Ali C. Begen (abegen)" <[email-masked]>
Date: Sun, 20 Oct 2013 20:57:08 +0000

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

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

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.

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

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.

5) Regarding the encoding considerations: This should say "framed". 

6) "Restrictions on usage" field can say "RTP applications only".

7) Person & email address to contact for further information: This should be "John Lindsay email:[email-masked]"

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

---

From: "John Lindsay" <[email-masked]>
Date: Mon, 21 Oct 2013 18:25:32 +0100

Hi

A new doc has been submitted in response to comments.

See notes below.

John

-----Original Message-----
From: Ali C. Begen (abegen) [mailto:[email-masked]] 
Sent: 20 October 2013 21:57
To: [email-masked]
Cc: [email-masked]
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??

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

>>> Don't understand issue? Can you give further explanation?

> 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:[email-masked]"
> 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:[email-masked]"

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

---

From: "Ali C. Begen (abegen)" <[email-masked]>
Date: Tue, 22 Oct 2013 17:30:57 +0000

John,

A few errors still exist (I dont think these are showstoppers but IESG tends to be picky about these when the document is shipped w/o fixing them).

>> Updated please ensure you agree

You forgot the following item (insert it based on the template):
Fragment identifier considerations: None

>>>> Don't understand issue? Can you give further explanation?

Encoding considerations: This media type is framed in RTP and contains binary data; see Section 4.8 of [RFC6838].

> Can you ensure you agree?

Intended usage: COMMON
Restrictions on usage: This media type depends on RTP framing, and hence is only defined for transfer via RTP [RFC3550].

>> Lindsay email:[email-masked]"

I am sorry. Please remove the quotes. I thought the email was missing.

The submission is now closed but will open in two weeks. Please update the draft at that time.

Thanks,
-acbegen