Re: [codec] Fwd: New Version Notification for draft-terriberry-oggopus-00.txt

[Gregory Maxwell <[e-mail]>]

Timothy B. Terriberry [[e-mail]]:
>> One thing we should explicitly mention in the draft is that a decoder
>> MUST NOT reject a file because the header is longer than expected.
>Yeah, I hadn't touched on this because I wasn't sure what I thought yet.
>> This makes it possible to have a minor revision to the format that
>> adds a field to the header. As an exception, if the decoder knows the
>> exact length of header for the minor revision that the file uses, then
>> it MAY reject it. Any thoughts?
>The test would actually have to be: exact version is known (0 or 1) AND
>(mapping family is 0 AND length is not 19, OR mapping family is > 0 and

I'd prefer to just say— generally— that they SHOULD rejected headers
which are completely defined by this specification which can not be
completely parsed.

And that they also MUST accept new minor numbers which have 
additional data.  It's not like there is any complexity in this. You just
stop when you're done reading what you understand.

If an addition is made which can't be safely handled this way
then it should simply use a new major number.

Ron [[e-mail]]:
> and I get a version 2 header that is several MB in size, should I
> really just blindly accept that Nothing Is Wrong

Well—You'll blindly accept that nothing is wrong with
several MB of junk in the comments or in Opus padding.

But, see RFC 3533. The initial header packet must be on a single
page, so its limited by the container spec to 64K.

And because there is no explicit length field implementers won't
run into contradictions with the decoder or memory exhaustion
attacks where you malloc the signaled size which can be much
greater than the bits on the wire.

> It also doesn't really seem ideal to be appending extra fields after
> the variable length mapping table data anyway.

I'm not sure what you're thinking here. You just append them.

With this extensions must be monotonic— e.g. you couldn't
know about Y but not an extension added before it, but the
version numbers are also monotonic.  If there arose a need
for TLV style extensions in this header, that could be added
in some version.  But /currently/ I can only come up with
reasons why this would be a horrible idea.

A key point here is that there are not currently any anticipated
extensions.  But a byte there aligns the header, and having
it be explicit is superior to the header size sniffing I observed
implementers doing for version=0, before the pre-draft spec
defined how the version field should be used (when it was
just treated as a null terminator for "OpusHead")

I'm stridently opposed to implementing some deathstar
complexity binary-xml-extension-mechanism-whatever
when we don't have much of idea what it would be used for.

Experience in early implementations shows what we have in 
the document now is simple enough that people will implement
and validate the whole thing, even parts they don't really
care about.

No one who's contributed to the draft so far is new at this,
and, apparently, based on the collective experience with
vorbis, speex, flac, theora, (and the many non Ogg formats
we've all worked with), it doesn't appear that there will
ever need to be anything added to the header.

But, Opus is new and hits some new applications and it
would seem to be foolish to leave no graceful way out
in the case of currently unanticipated needs.

If it would be helpful, let me suggest a kind of thing
that would be done with this:

Say we find that through some astonishing
boneheaded flaw, the Opus bitstream spec results
in terrible quality for some important signal. Experts
mash their heads together and come up with a
bitstream modification which can be decoded by the
original decoder with no worse quality and fixes the 
decode when played with a new type decoder, but
only if it knows its playing a new-type bitstream. The
Opus RFC is updated.

But then an updated player needs to know that it
has a new-type file.  The header could just be
changed entirely but then the result wouldn't
play in old players.

So instead a v=2 header would just define some
field to signal this.  Because its a codec setup
header it would be available in the right layers
of applications to pass it on to the codec
implementation.

This is all terribly unlikely, not just because of all
the work that went into the opus spec but also
because no one would be eager to revise the bitstream,
but the availability of a simple compatible version
field prevents being cornered with ~zero complexity
in the likely case that it never happens.

> One is that such things be added to the comment header - which
> already defines a mechanism for arbitrary additional fields

Ugh.  The comment header is for stream metadata.
In theory it should all just be preserved if you transcode
the file between other formats.  It really shouldn't
contain anything codec specific.  

My experience in Vorbis indicates to me that using
the comment packets for less-metadata-ish use has 
had bad consequences:  It results in an application
layering violation (metadata doesn't get anywhere 
near the codec itself), it gets moved between
files where it doesn't belong, and editing tools invalidate
it without removing it (because they can 'parse' but
not understand it).  You'd have applications that
care nothing about metadata being forced to parse
it just to get whatever extension you need.  And then
you also get lovely results like your terminal getting
broken because something shoved a bunch of binary
data into the comments and a simplistic application
just splat it out to your terminal.