[codec] AD Evaluation of draft-ietf-codec-oggopus-09

["Ben Campbell" <[e-mail]>]

(oops, sent from wrong address, and got bounced from a lot of lists. 
Apologies for the repeat. Please send replies to this address.)

Hi,

Here is my AD evaluation of draft-ietf-codec-oggopus-09. Codec 
encapsulation is not my strong suit, so I suspect many of my comments 
can be answered with "this is the way we normally do this thing". But 
I'd still like to discuss these before going to IETF last call.

Thanks!

Ben.

----------------


Substantive:
============

- section 2, last paragraph:

Is this paragraph necessary or useful? (And is it really impossible to 
create a non-compliant implementation that meets all the MUSTs?) I 
suggest removing it.

- section 3:
This section seems to use 2119 keywords to describe what I _think_ are 
existing Ogg requirements. If so, they should use descriptive text, 
unless they are in the form of attributed quotes.

- 3, last paragraph:
-- "implementations need to be prepared":
  maybe that should be a MUST or SHOULD?

-- "last page SHOULD NOT be a continued packet":
Why not MUST NOT? Can you describe reasons someone might reasonably 
violate this?

- 4.2, 2nd paragraph:
-- "samples which SHOULD be skipped": Why not MUST?  (also, 
s/which/that)
-- "SHOULD NOT be played": Is this redundant with SHOULD be skipped?

- 4.3: Should the reference to [vorbis-trim] be normative? Can the 
feature be implemented without understanding the referenced doc?

- 4.6: Should the reference to [seeking] be normative? Can the section 
be implemented without understanding it?

- 5.1, list entry 5: What is the unit for "Input Sample Rate"?

- 5.1, entry 6, sentence starting with "Virtually all players..."
Is this a normative statement, or a statement of fact? If the former, 
consider dropping “Virtually all”

- 5.1, entry 6, last paragraph:
-- "which MUST be omitted when the channel mapping family is 0": That 
seems redundant to the previous paragraph.
-- "Implementations SHOULD reject ID headers"
What does it mean, in context, to "reject" headers?

- 5.1.1.4: Why not MUST?

- 5.2, last two paragraph: Why are the two SHOULDs not MUSTs?

- 5.2.1, 2nd to last paragraph, last sentence:
I don’t think muxers assume things—what is the required concrete 
behavior?

- 5.2.1, last paragraph: Why is the SHOULD NOT not MUST NOT?

-6, first paragraph:
Why are the two SHOULDs not MUSTs? What does it mean to reject a packet?

-7: "reference implementation" could use a citation. Should 
[linear-predection] be normative? Can this be implemented without 
understanding that? If it does need to be normative, can you find a more 
stable reference?

- 9, first paragraph:
This seems to defer at least some security considerations to 4732. If 
so, this should be a normative reference. If, on the other hand, 4732 
just provides additional information that is not necessary to understand 
this section, then the wording should be adjusted.

- 13:
Is this needed? Are the IETF BCP 78 and 79 provisions incorrect for 
this?



Editorial
=========
- Section 1: Consider spelling out multiplexer (muxer) and demultiplexer 
(demuxer) on first mention.

- 3: Please expand TOC, SILK, and CELT on first mention.

- 5, steps 2 and 3:
These steps in combination seem to say “ decode at the highest 
available supported rate that the hardware can handle.” If so, the 
wording could be simplified.

- 7, 2nd paragraph:
I don’t think that’s literally true, you’ve defined 0, 1, and 255. 
The rest are reserved. (I guess one could argue the guidance to treat 
reserved values as 255 supports this statement, but I think that’s a 
stretch.

-5.1.1.2: Do you expect the reader to know what Vorbis channel order is? 
Also, please expand FLAC on first mention.

- 5.2.1, paragraph starting with " An Ogg Opus stream MUST NOT have more 
than one of each tag, and if present their values MUST be an integer"

each of these two new tags, or all tags? (I assume the former from the 
numeric values) Also, singular/plural mismatch in "their values MUST be 
an integer"

-10, last paragraph:

Updates, or extends? If the former, the draft needs an “Updates RFC 
5334” field in the initial heading.