[FFmpeg-devel] [RFC] avcodec: Add native DCA decoder based on libdcadec.
h.leppkes at gmail.com
Wed Jan 6 00:42:54 CET 2016
On Wed, Jan 6, 2016 at 12:40 AM, Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:
> On Tue, Jan 5, 2016 at 3:28 PM, Hendrik Leppkes <h.leppkes at gmail.com> wrote:
>> On Tue, Jan 5, 2016 at 10:46 PM, Andreas Cadhalpun
>> <andreas.cadhalpun at googlemail.com> wrote:
>>> On 05.01.2016 21:38, foo86 wrote:
>>>> On Tue, Jan 05, 2016 at 08:45:22PM +0100, Andreas Cadhalpun wrote:
>>>>> On 03.01.2016 18:49, foo86 wrote:
>>>>>> +// 5.3.1 - Bit stream header
>>>>>> +static int parse_frame_header(DCA2CoreDecoder *s)
>>>>>> + // Source PCM resolution
>>>>>> + s->source_pcm_res = ff_dca_bits_per_sample[pcmr_index = get_bits(&s->gb, 3)];
>>>>> This can cause an out-of-bounds read if get_bits returns 7, because ff_dca_bits_per_sample
>>>>> only has 7 elements.
>>>> Fixed locally, thanks.
>>>> P.S. To avoid resending this huge patch, I've put the fixes accumulated
>>>> so far in a private dcadec2 branch on github  (will be rebased
>>>> frequently against FFmpeg master).
>>>> : https://github.com/foo86/FFmpeg/tree/dcadec2
>>> OK. This decoder seems to be quite robust in handling fuzzed samples,
>>> so from a security point of view it should be fine to replace the
>>> old dca decoder with this one.
>> So that leaves us with a bunch of positive comments, on this side
>> anyway, and noone opposed yet.
>> Arguments for a switch include:
>> - Nearly complete coverage of all DTS features, well tested and
>> confirmed bit-exact (only DTS Express is missing, which is technically
>> its own little codec using DTS EXSS headers)
>> - Slightly faster (~5%)
>> - Active maintainer
>> - Andreas seal of security approval ;)
>> If anyone thinks we should not replace our decoder, speak now or
>> forever hold your peace (and bring proper arguments).
>> I will try to do a code review to the best of my abilities in the upcoming days.
> For now, I definitely think we should replace our decoder.
> Just a clarification: in the long run, isn't it a good idea to get
> this into the repo and not use an external library? For instance, if
> and when asm code gets written for this, isn't it easier to work
> within FFmpeg's codebase, which has some infrastructure for writing
The whole idea of this patch is to integrate the decoder fully into
our codebase, and no longer use the external library.
What I forgot to mention in my last mail, foo86 if you have any
questions about ripping out the old dca decoder, feel free to contact
More information about the ffmpeg-devel