Subject: [AVT] Review of draft-ietf-avt-rtp-atrac-family-13

From: Tom Taylor <[email-masked]>
Date: Thu, 06 Mar 2008 22:15:11 -0500

The Working Group Last Call period expired for 
draft-ietf-avt-rtp-atrac-family-13 without comment on the AVT list. There was 
one objection on the Media Types list, that it is unacceptable to register a 
media type in the IETF tree without giving a reference to the source 
documentation. The submitting organization is discussing the matter.

Here are my own comments. There are a few fixes that are essential, followed by 
a long list of trivial editorial changes.

Issues that need fixing:

1. Numbering of fragments

There is an inconsistency in the numbering of fragments. Section 5.3.1 says it 
starts from 0. Section 5.3.2.2, first sentence of the second paragraph, says 
that it starts from 1. The example in section 6.2 also shows it starting from 1. 
Before deciding too quickly what is the right answer, you should also think 
about and indicate in section 5.3.1 what the value should be for an unfragmented 
frame.

2. Normative language in section 7.5.2

There is a "should", a "must", and a "recommended" (mis-spelled) in section 
7.5.2 that should probably be capitalized. There is also a lower-case 
"recommended" near the end of section 7.5.3.

3. Cut-and-paste error in section 7.9?

First example offer shows the same rtpmap description for payload types 98 and 
99. Should payload type 99 be the same as in the answer?

4. Obsolete reference

Your final normative reference [8] should now be to 
draft-ietf-mmusic-decoding-dependency-01.

Trivial:

- most I-Ds have the authors' organization (e.g., Sony Corporation) in the draft 
header. You're turning down a chance to put your organization's name in front of 
the public :)

- section 1, first para first line: are designed -> is designed
[to agree with "family"]

- section 1, second para, fourth line: add comma at the end after "codecs"

- section 4.1 third line: octect -> octet

- section 4.4 second line: "decoding gap" by -> "decoding gap" at

- section 4.4 again: I suggest adding a sentence: "For further details, see 
section 5.3.2.1." You could also consider adding a reference to RFC 2198 as a 
more sophisticated approach (implying a new informative reference).

- I assume section 4.5.2 has multiple paragraphs, but the blank lines between 
them are missing. Anyway, in what appears to be the third para:
   can not -> cannot
   only with ignoring -> only, ignoring

- section 5.1, second para (first after Figure 3), last sentence:
   In case of using redundant mechanism, -> When using the redundancy mechanism 
described in section 5.3.2.1,

- section 5.2:
   Extention -> Extension
   In last line of timestamp description: sigalling -> signalling

- section 5.3.1, Continuous flag:
   Might make more sense to call this the "Continuation flag", here and 
elsewhere in the document.
   fragmentaion -> fragmentation

- section 5.3.2: is it too obvious to say that if multiple frames in a packet 
cover multiple sampling times, the frames must be ordered by increasing sample 
time? And as a consequence, when the redundancy mechanism of section 5.3.2.1 is 
used, the redundant frames come first and the timestamp corresponds to the 
sampling time of the oldest redundant frame. Also, probably should say that the 
frames in a packet, including the redundant ones, must cover a continuous time 
interval. The calculation in section 5.3.2.1 depends on these assumptions.

- section 5.3.2 first para third line: preceeded -> preceded

- section 5.3.2, block length description: final sentence could be confusing and 
should be deleted. The first para of the section already says that each frame is 
preceded by the Layer Type flag as well as the Block Length.

- section 5.3.2.1 first para, second-last line:
   and to expand -> and warns it to expand

- section 5.3.2.1, Figure 7:
   reciever -> receiver
   Exmaple -> Example

- section 5.3.2.2, first to second lines of second para:
   packet, its -> packet with its

- section 5.3.2.2, third para has its ideas in the wrong order, I think. I 
suggest that the para be rewritten to read:

   "Packets containing related fragmented frames MUST have identical
    timestamps. Thus, while the Continuous bit and Fragment Number fields
    indicate fragmentation and a means to reorder the packets, the
    timestamp can be used to determine which packets go together."

- section 7.3, channelID description, second-last line:
   could be used -> could be used in this case

- section 7.3, maxptime description:
   a multiple frames can not -> multiple frames cannot
   reciever -> receiver

- section 7.4 first para: The Table 1. is explaining -> Table 1 explains

- section 7.5.1, Media subtype:
   channels SHALL also be included -> channels (0 or 1) SHALL also be specified

- section 7.5.3, remaining parameters bullet, fifth line:
   MUST be existing -> MUST be present

- section 7.6, second line: either -> either of

- section 7.7 first bullet:
   configuration(s) that is provided -> configuration(s) provided

- section 7.7, final para, first sentence:
   scaleble -> scalable
   the attribute -> the attributes

- section 9.1 first para: preferrable -> preferable

- section 9.1 second para:
   for encryption affecting -> that encryption will affect

- section 9.2 first line: due malicious -> due to malicious

Tom Taylor
_______________________________________________
Audio/Video Transport Working Group
[email-masked]
https://www.ietf.org/mailman/listinfo/avt

---

From: actech <[email-masked]>
Date: Fri, 07 Mar 2008 12:41:52 +0900

Dear Mr.Taylor,

Thank you for your review and comments on our draft.
We will fix the matter, and revise the draft as "...atrac-family-14"
in soon.

In that case, do we have to re-submit the draft to AVT group and
have reviewed again in the group, or is there any short cuts? :-)

Best Regards,

Jun Matsumoto

---

From: Tom Taylor <[email-masked]>
Date: Thu, 06 Mar 2008 23:07:58 -0500

The issues are small enough that a short review will be sufficient. The only 
difficult issue is the one Harald raised, about the requirement for a specification.

actech wrote:
>> draft-ietf-avt-rtp-atrac-family-13 without comment on the AVT list. 
>> There was one objection on the Media Types list, that it is 
>> unacceptable to register a media type in the IETF tree without giving 
>> a reference to the source documentation. The submitting organization 
>> is discussing the matter.
>>
>> Here are my own comments. There are a few fixes that are essential, 
>> followed by a long list of trivial editorial changes.
>>
>> Issues that need fixing:
>>
>> There is an inconsistency in the numbering of fragments. Section 5.3.1 
>> says it starts from 0. Section 5.3.2.2, first sentence of the second 
>> paragraph, says that it starts from 1. The example in section 6.2 also 
>> shows it starting from 1. Before deciding too quickly what is the 
>> right answer, you should also think about and indicate in section 
>> 5.3.1 what the value should be for an unfragmented frame.
>>
>> There is a "should", a "must", and a "recommended" (mis-spelled) in 
>> section 7.5.2 that should probably be capitalized. There is also a 
>> lower-case "recommended" near the end of section 7.5.3.
>>
>> First example offer shows the same rtpmap description for payload 
>> types 98 and 99. Should payload type 99 be the same as in the answer?
>>
>> 4. Obsolete reference
>>
>> - most I-Ds have the authors' organization (e.g., Sony Corporation) in 
>> the draft header. You're turning down a chance to put your 
>> organization's name in front of the public :)
>>
>> - section 4.4 again: I suggest adding a sentence: "For further 
>> details, see section 5.3.2.1." You could also consider adding a 
>> reference to RFC 2198 as a more sophisticated approach (implying a new 
>> informative reference).
>>
>> - I assume section 4.5.2 has multiple paragraphs, but the blank lines 
>> between them are missing. Anyway, in what appears to be the third para:
>>    can not -> cannot
>>    In case of using redundant mechanism, -> When using the redundancy 
>> mechanism described in section 5.3.2.1,
>>
>> - section 5.2:
>>    Extention -> Extension
>>    Might make more sense to call this the "Continuation flag", here 
>> and elsewhere in the document.
>> - section 5.3.2: is it too obvious to say that if multiple frames in a 
>> packet cover multiple sampling times, the frames must be ordered by 
>> increasing sample time? And as a consequence, when the redundancy 
>> mechanism of section 5.3.2.1 is used, the redundant frames come first 
>> and the timestamp corresponds to the sampling time of the oldest 
>> redundant frame. Also, probably should say that the frames in a 
>> packet, including the redundant ones, must cover a continuous time 
>> - section 5.3.2, block length description: final sentence could be 
>> confusing and should be deleted. The first para of the section already 
>> says that each frame is preceded by the Layer Type flag as well as the 
>> Block Length.
>>
>> - section 5.3.2.2, third para has its ideas in the wrong order, I 
>> think. I suggest that the para be rewritten to read:
>>

---

From: Tom Taylor <[email-masked]>
Date: Fri, 07 Mar 2008 11:43:54 -0500

Reading the update you sent me, I realized there is one more issue that has to 
be fixed. The first paragraph of section 5.1 says that the ATRAC header section 
MAY be empty. In first reading I took that to mean that the ATRAC header may be 
absent. Is that really what you meant? If it isn't, I suggest just deleting the 
  text:

    ", of which the second i.e ATRAC Header Section MAY be empty"

from this paragraph. The rules for population of the ATRAC header in section 
5.3.1 are clear enough now to cover all circumstances.

>> have reviewed again in the group, or is there any short cuts? :-)
...