[FFmpeg-devel] [PATCH 3/4] oggdec: add support for Opus codec.

Gregory Maxwell gmaxwell at gmail.com
Wed Jul 11 16:13:07 CEST 2012

Nicolas George <nicolas.george at normalesup.org> wrote:
> 16 (rejected) confirm that. But you wrote about major!=1: is it a typo from
> you or something else?

Indeed, just a typo.

> This is not a problem with Opus, this is a problem I fear with the ffmpeg
> command-line tool: I have noticed at a time that audio timestamps were often
> neglected. There will be need for careful checks.

For round-tripping durations and phase shifts this should be easy to
check, and I have scripts for this that should help.

For AV sync, you've got me. Thats harder.

> Please forgive me, I spoke way too harshly.

Not a problem.

> Thanks a lot. But looking at the diff, it looks like it was done as
> post-processing, exactly like an application could have done: I am a bit
> disappointed because I hoped it could be injected directly somewhere in the
> decoding process with a much smaller cost.

It's somewhat more complex codewise to lower it, as it will need to be
done in four places— in different ways because of different
units/scaling used a the different points (e.g. the correct way to
efficiently do it in MDCT mode is by scaling the log domain band

I would have done so except we're currently working on making the 1.0
release and complicated changes need to be done carefully, and because
I was in a big hurry because I was hoping avoid a compatibility mess.

But sure, it will be eventually be done at the cost of only 25 adds
per frame, at least in the MDCT modes and the PLC, the LP modes will
likely need to retain a multiply add pass over the signal because the
scaling of the data in the predictive loop is important.

Fortunately this won't change the API. It will just become faster for
those using it.

> For that reason, plus the fact that this API enhancement will not make it
> into the next Debian Stable, I am hesitating to use it. Especially for
> floats, where vector_fmul_scalar+vector_clipf can do the trick really in two
> lines, possibly faster. I shall have to profile a bit.

Debian stable is actually shipping a snapshot of this branch rather
than our "releases". The addition is forwards and backwards ABI
compatible. (or at least backwards assuming you handle it throwing
unimplemented at runtime)

This interface will be in our first stable library release in a few
weeks and should make its way into Debian in the fullness of time.
Had someone brought this up four weeks ago it would have already been
in Debian stable.

In float, on arches with SSE in the default platform GCC vectorizes that loop.
(Though with some irrelevant fluff and I'm sure less reliably than
something done directly)

I think you may be falling into a shed-painting trap by worrying about
a single normalization loop which will be a small fraction of the
execution time of the entire codec. (None of which has a single line
of platform optimizations because our focus has been on finishing the
format and bitstream algorithmic complexity, the current
implementation is slow and brain-dead in a lot of ways)

> I will rework my patches to take everything into consideration.

I look forward to it.

More information about the ffmpeg-devel mailing list