[FFmpeg-devel] [PATCH] add (e)ac3float decoders

[Justin Ruggles]

On 03/07/2011 04:20 PM, Reimar D?ffinger wrote:

> On 7 Mar 2011, at 21:01, Justin Ruggles <justin.ruggles at gmail.com> wrote:
>> On 03/07/2011 02:25 PM, Ronald S. Bultje wrote:
>>
>>> Hi,
>>>
>>> On Mon, Mar 7, 2011 at 2:18 PM, madshi <madshi at gmail.com> wrote:
>>>> attached you'll find a patch which adds "ac3float" and "eac3float" decoders.
>>>> The only difference to the "ac3" and "eac3" decoders is that the float
>>>> decoders output float, obviously (which is the native decoding format)
>>>> instead of rounded down 16bit integer samples. This is quite important,
>>>> IMHO, because rounding down digital audio/video data is a violation of
>>>> processing laws. When bitdepth is reduced, dithering must be used, to avoid
>>>> quantization errors, which libav does currently not do. So outputting float
>>>> is a better solution for audio quality.
>>>
>>> The idea is fine, but the execution isn't. IIUC, Justin has patches
>>> that move audio decoding to using something like an AVFrame, which we
>>> could eventually use to output "planar" audio formats, so each channel
>>> in its own buffer. This would prevent the loop in your patch and not
>>> cause the significant slowdown introduced here.
>>>
>>> I'd prefer to wait for Justin's patches to mature and use that approach.
>>
>>
>> 1) There is no need for separate int16 and float decoders. The AC3
>> decoder is a floating-point decoder. It just happens to output int16 at
>> the end.
> 
> Can FATE handle float?

sure. it's handling it just fine right now.

>> 2) Like Mans said, the goal needs to be to optimize the generic
>> conversion functions first so we don't slow things down, then change
>> decoders to output in their native format.
> 
> Hm, wouldn't it be possible to select which via codeccontet->sample_fmt?
> I mean we still have API-compatibility with versions where using anything but int16 is not possible as output.
> This would allow to have this feature and have it tested without having to wait for additional stuff that might take quite some time, and it avoids the compatibility issues that switching to float-only means (and yes I am sure the changes for it will be non-trivial for some applications).


If we do this as a temporary solution, I think it would be better to add
something like avctx->request_sample_fmt.  That way we have an easy way
to disable it when it is no longer needed without changing how
sample_fmt is used.

And why would it be non-trivial for applications if we change a single
decoder or even multiple decoders to output to float.  We already have
audio decoders that output to float, so applications need to handle
float samples anyway unless they selectively disable them all.

-Justin