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

["Timothy B. Terriberry" <[e-mail]>]

Ben Campbell wrote:
> That makes me wonder why the end-of-stream flag part is a SHOULD and not
> a MUST, though. Are there reasonable circumstances to violate that SHOULD?

The usual case is again, saving a live stream that gets interrupted 
before it ends.

> This might be better stated as non-normative guidance.

Given that this is a general Ogg requirement, and not really 
Opus-specific, I think you are right.

Here are the changes I propose:

-The first audio data page SHOULD NOT have the 'continued packet' flag set
- (which would indicate the first audio data packet is continued from a 
previous
- page).
-Packets MUST be placed into Ogg pages in order until the end of stream.
-Audio packets MAY span page boundaries.
+Packets are placed into Ogg pages in order until the end of stream.
+Audio data packets might span page boundaries.
+The first audio data page could have the 'continued packet' flag set
+ (indicating the first audio data packet is continued from a previous 
page) if,
+ for example, it was a live stream joined mid-broadcast, with the headers
+ pasted on the front.
+A demuxer SHOULD NOT attempt to decode the data for the first packet on 
a page
+ with the 'continued packet' flag set if the previous page with packet data
+ does not end in a continued packet (i.e., did not end with a lacing 
value of
+ 255) or if the page sequence numbers are not consecutive, unless the 
demuxer
+ has some special knowledge that would allow it to interpret this data
+ despite the missing pieces.

Along with

-The last page SHOULD have the 'end of stream' flag set, but implementations
- need to be prepared to deal with truncated streams that do not have a page
- marked 'end of stream'.
-The final packet on the last page SHOULD NOT be a continued packet, 
i.e., the
- final lacing value SHOULD be less than 255.
+A logical stream ends with a page with the 'end of stream' flag set, but
+ implementations need to be prepared to deal with truncated streams 
that do not
+ have a page marked 'end of stream'.
+There is no reason for the final packet on the last page to be a continued
+ packet, i.e., for the final lacing value to be less than 255.
+However, demuxers might encounter such streams, possibly as the result of a
+ transfer that did not complete or of corruption.
+A demuxer SHOULD NOT attempt to decode the data from a packet that 
continues
+ onto a subsequent page (i.e., when the page ends with a lacing value 
of 255)
+ if the next page with packet data does not have the 'continued packet' 
flag
+ set or does not exist, or if the page sequence numbers are not 
consecutive,
+ unless the demuxer has some special knowledge that would allow it to 
interpret
+ this data despite the missing pieces.

That gives specific guidance on what a demuxer needs to do to avoid 
corrupting the Opus state machine with garbage data (which is generally 
worse for Opus than simply skipping decode of a packet). It is also what 
is most broadly implemented.

> It would be helpful to mention that in the text.

I added something about that.

> 2119 :-) It's not so much how big of a stick you need to get people to
> do the right thing as it is how bad do things break if people do the
> wrong thing. (I do agree with the sentiment to not put in MUSTs that you

Given that historically, many audio formats (including MP3) did not 
support sample-accurate durations at all, I don't think the world ends. 
But it certainly would break features like gapless playback that some 
users find important.

> The current wording says "see [seeking] for general implementation
> guidelines." If those are optional guidelines, it would help to say so,
> or describe it as "some example guidelines". OTOH, if you really prefer
> the approach in [seeking], then it still makes sense for it to be
> normative.

I've added the optional/example qualifier.

I also updated the claim on the number of bisections required, as our 
implementations have improved since the draft was first written.

>>> What does it mean, in context, to "reject" headers?
>>
>> The same thing as rejecting a stream (i.e., don't try to play/decode
>> it). Changed to say that instead.
>
> Which "that"?

To quote the attached diff:

-Implementations SHOULD reject ID headers which do not contain enough 
data for
- these fields, even if they contain a valid Magic Signature.
+Implementations SHOULD reject streams with ID headers that do not contain
+ enough data for these fields, even if they contain a valid Magic 
Signature.

> Maybe this is a conventional way to say things in the codec community,
> but when I see "reject" I usually think that means you generate some
> sort of error back to the source of the thing you reject. (As opposed to
> "ignore")

Well, the most likely behavior here is to tell the user that the file is 
not a valid Ogg Opus file, so your sense of "generating some sort of 
error back" is in fact correct. But that makes some application-level 
assumptions I'm not sure I'm comfortable writing normative text around. 
Any ideas for a better way to phrase this?

> If this spec reserves those values, then people still need to update it
> to assign them. If you want to make it extensible, then perhaps it needs
> an IANA registry, which, depending on the assignment policy, may not
> require an RFC at all (seems like "expert review" or "specification
> required" might make sense).

An IANA registry actually does not seem like a terrible idea. Reading 
RFC 5226 it doesn't seem too hard to set one up. I attempted to draft 
some text.

> it's what our "UPDATES" process is for. The idea that you might want to
> informally experiment with different approaches is more convincing. But
> if that is the case, it would be helpful to have a sentence to that
> effect in the text.

Okay, I will add one.

> (I do agree "Excessive" is a bit vague, so if there were a way to state
> this more concretely it would improve things. But that's true for a
> SHOULD as much as for a MUST).

The problem is that it depends very much on context. 60 kB might already 
be seen as excessive for an embedded system, but would never be noticed 
on a desktop.

>> In some contexts, there may be no confusion with multiple
>> normalization schemes. The language in this section was debated a good
>> [...]
>
> I agree that SHOULD NOT may be appropriate here. My point is the text
> should explain why. Almost anytime a SHOULD occurs, it's helpful to

I agree with your point very much in principle. We just may not have 
been as careful about it in practice as we should have been (and I 
appreciate you forcing us to clarify our thinking now). I'll add 
something to the effect of what I said above.

> Did 6716 use a SHOULD?

It imposed no restrictions on the amount of padding at all.

> attributed). But in any case, I'm looking for justification in the text.
> (I do consider preventing the use of excessive bandwidth a good enough
> reason for a MUST

I attempted to add some.

>> For the second, this is the same as the previous comment about
>> excessive allocations. I have no objection to changing this one, either.
>
> I realize now there were 3 SHOULDs in that paragraph. I mean the one
> about avoiding excessive allocations. It sounds like

You may have forgotten to complete your thought here.

> Please consider saying something like "For more information on linear
> prediction, see [XXX]." As it's currently stated, it looks like it says
> MAY do X, where the reference specified X, which would require a
> normative reference.

That's a good suggestion. Done.

> That language is a bit of a problem for a standards track RFC. If people

You mean the current text, or the actual text that wound up in RFC 6716?

> are really attached to it, we can pursue allowing it. But I expect
> objections down the line. Keep in mind "NOTE WELL" says that any
> contributions are subject to the rules of BCP 78 and 79.

Yes, I think this has more to do with letting people use such 
contributions _outside_ the context of the IETF (i.e., it grants 
additional rights to those already granted in BCP 78 and 79, which I 
understand is allowed).

You are probably right about people objecting down the line (again), but 
I am happy to address those objections when they come, as we did for RFC 
6716.

> In any case, 6716 is a bit more code centric , so the discussion made
> more sense there.

Well, from our point of view, the XML source for both RFCs is included 
in the repository with libopus, which is distributed in Debian. If we 
didn't have some kind of grant like this, they would have to somehow 
strip those documents out. So it is not so much the code being in the 
RFC as the draft being distributed with the code.

In any case, I have attempted to draft an RFC Editor's note requesting 
the same text that RFC 6716 used. Since that text grants a license from 
the IETF Trust, instead of the authors, I assume that someone from the 
Trust will have to approve of it, however (and reading 
<http://www.ietf.org/proceedings/85/slides/slides-85-iesg-opsandtech-15.ppt> 
it seems that the current opinion of the Trust is that this approach is 
legally required).

I have also gone through and finished removing the 2119 language for 
general Ogg requirements.

The diff of all changes is attached. If these look good (or if it's 
simply helps in reviewing them) I can publish a new version.