Subject: Re: [codec] I-D Action: draft-ietf-codec-oggopus-00.txt

From: Tina le Grand <[email-masked]>
Date: Thu, 13 Dec 2012 10:07:19 +0100

Hi,

I have reviewed draft-ietf-codec-oggopus-00, and I have some questions and
comments. The Ogg format is new to me, so some of my comments might not
require any changes to the specification.

Section 3:
- In RFC 3533 "packet" is defined to be "created by the encoder of the
logical bitstream and represent meaningful entities for the encoder only",
but in this section it is said that "the first packet in the logical Ogg
bitstream MUST contain the identification (ID) header...". This header is
not part of the regular Opus bitstream, and is defined for the Ogg Opus
format only. does "packet" have a different meaning in this specification?
Or should it say "the first page.."?

Section 5:
- Would be great if you could mention the name of the two header packets in
the first section.

Section 5.1, list number 4:
- What is a cropped Ogg Opus stream?

Section 5.1, list number 5 (page 13):
- What does this mean "The original sample rate of the encoder input is not
preserved by the lossy compression"? You don't get perfect reconstruction,
of course, but you'll get the same audio bandwidth except for the 48 kHz
mode, or is there any other filtering affecting the bandwidth?

Section 5.1, list number 5 (page 13):
- There is a new numbered list within list number 5, which makes
the document hard to read.
- List number 3: Can be more clear: "...decode at the highest supported
rate above the hardware's sample rate and resample."
- Right after the list of 4 ways of choosing decode rate there is a fifth,
that is not in the list. I think it should be merged into the list as one
option.

Section 5.1.1, page 17:
- Family 1: Would be better if the Vorbis channel order was described in
this spec as well.

Section 5.2, list number 3:
- NUL -> NULL? or "null" as in section 5.1 list number 2, page 12.

/Tina

On Tue, Nov 20, 2012 at 12:00 AM, Timothy B. Terriberry
<[email-masked]>wrote:

> [email-masked] wrote:
>
>> A New Internet-Draft is available from the on-line Internet-Drafts
>> directories.
>>   This draft is a work item of the Internet Wideband Audio Codec Working
>> Group of the IETF.
>>
>
> The (other) chairs declared there was consensus to adopt this draft as a
> WG item and asked me to upload a new version of it.
>
> I have made a few minor edits from draft-terriberry-oggopus-01:
> 1) Fixed up the references.
> 2) Fixed a couple of typos.
> 3) Replaced a "should" with a "SHOULD" in Section 4.1.
> 4) Clarified the language around the starting granule position in the case
> that a) there is more audio in packets that complete on the first audio
> data page with a completed packet than the granule position indicates and
> b) the EOS flag is set on the same page (in this case you should not count
> forwards from 0, but should work backwards to figure out the real starting
> granule position).
> 5) Updated the acknowledgments.
>
> I have not incorporated any of the proposals that have been made to the
> list since the initial draft was posted (e.g., seamless chaining,
> replaygain tags, etc.).
>
> ______________________________**_________________
> codec mailing list
> [email-masked]
> https://www.ietf.org/mailman/**listinfo/codec<https://www.ietf.org/mailman/listinfo/codec>
>

---

From: Ron <[email-masked]>
Date: Fri, 14 Dec 2012 15:57:41 +1030

> Or should it say "the first page.."?

It's actually probably "encoder" that might have the different meaning
there :)  Here it means the thing encoding the "ogg bitstream" which
includes both the codec (Opus) packets and the metadata in the bitstream.

Ogg pages contain packets, and all payload is contained in packets,
including metadata not produced by the codec itself.

If there's some simple text we could add to clarify this though, please
do suggest something.

> - Would be great if you could mention the name of the two header packets in

They are introduced and named in Section 3.  But the "tell them what you
are going to tell them" principle means repeating that in the Section 5
intro probably isn't a bad idea.

> - What is a cropped Ogg Opus stream?

That's mentioned in Section 4.4.  Ogg streams can generally be shortened by
trimming packets off the beginning or end of the stream without needing to
rewrite the entire stream portion that is retained.

If a few more words to explain this are warranted, then please suggest
some extra text (and the best place to put it) for that too.

> mode, or is there any other filtering affecting the bandwidth?

Once the stream is encoded as Opus, there is no longer any concept of
sample rate inherent in the data.  It is entirely a decision of the
decoder configuration what sample rate it will be reconstructed with.

Bandwidth will be preserved if it chooses a sufficiently high sample
rate, but it could also take a full-band 48k original signal and
reconstruct that at 8kHz.  It would just throw away extra information
that wasn't actually lost by the encoder.

If you want to know exactly what the sample rate of the original was,
(regardless of the actual bandwidth of the audio it contained), then
we need to store that as metadata, because Opus doesn't know it.

> the document hard to read.

That's not really a numbered list, it's a 4 step procedure.
That said, that procedure itself is also under review, since it is
arguably not actually the optimal decision for quality at present.

> that is not in the list. I think it should be merged into the list as one

That's not really a 5th step in the procedure for selecting a _playback_
sample rate.  It's the rationale for having this metadata field.

As the first sentence says:

 This field is _not_ the sample rate to use for playback of the
 encoded data.

But if there is confusion over this section, it would be good to figure
out where and clear that up too.  It does sort of sidetrack a little
before getting to the actual use of the field, but the sidetrack is
related to when a decoder should use the field and when it should just
ignore it completely.

> - Family 1: Would be better if the Vorbis channel order was described in

We had discussed that, but there was no clear guidance yet on whether it
was better as an external reference or should be repeated again here.

> - NUL -> NULL? or "null" as in section 5.1 list number 2, page 12.

NUL is the traditional name for the 0 octet in a character string,
as distinct from a NULL pointer value etc. -- though it's true that
it is also called a "null terminator" when it's not actually a part
of the string content ...

http://en.wikipedia.org/wiki/Null_character

Thanks for your feedback on all this.

  Ron

---

From: Ralph Giles <[email-masked]>
Date: Wed, 19 Dec 2012 12:25:30 -0800

On 12-12-13 1:07 AM, Tina le Grand wrote:

> I have reviewed draft-ietf-codec-oggopus-00, and I have some questions
> and comments.

Thanks, these are very helpful.

> Section 5:
> - Would be great if you could mention the name of the two header packets
> in the first section.

Done.

> - What is a cropped Ogg Opus stream?

Did Ron's explaination help? I don't think referencing section 4.4
directly is helpful. What about:

"When cropping the beginning of existing Ogg Opus streams, a pre-skip of
at least...to ensure complete convergence in the decoder."

> - What does this mean "The original sample rate of the encoder input is
> not preserved by the lossy compression"?

I didn't come up with a way to clarify this without adding three
paragraphs, like Ron did. Suggestions?

> the document hard to read.

I'll have to think about this one.

> - Right after the list of 4 ways of choosing decode rate there is a
> fifth, that is not in the list. I think it should be merged into the
> list as one option.
> 
> Section 5.1.1, page 17:

Agreed. I've added a full description of the channel order for this mapping.

> - NUL -> NULL? or "null" as in section 5.1 list number 2, page 12.

This is 'NUL' as in the ascii character code, but 'null-terminated' wins
the google fight, so I'll use 'null' in both locations.

 -r