[FFmpeg-devel] [RFC] avcodec: Add native DCA decoder based on libdcadec.

[James Almer]

On 1/5/2016 11:35 PM, Michael Niedermayer wrote:
> On Tue, Jan 05, 2016 at 11:27:25PM -0300, James Almer wrote:
>> On 1/5/2016 11:21 PM, Michael Niedermayer wrote:
>>> On Tue, Jan 05, 2016 at 11:38:00PM +0300, 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 [1] (will be rebased
>>>> frequently against FFmpeg master).
>>>>
>>>
>>>> [1]: https://github.com/foo86/FFmpeg/tree/dcadec2
>>>
>>> breaks "make fate", something needs to be updated
>>> or a new reference sample uploaded if teh one we have is wrong
>>>
>>> stddev:  297.72 PSNR: 46.85 MAXDIFF: 3474 bytes:  8994816/  9601024
>>> MAXDIFF: |3474 - 0| >= 1
>>> size: |8994816 - 9601024| >= 0
>>> Test dca-xll failed. Look at tests/data/fate/dca-xll.err for details.
>>> make: *** [fate-dca-xll] Error 1
>>> make: *** Waiting for unfinished jobs....
>>
>> Was this run using foo86's decoder, or the current one? If the former then it's
>> not unexpected since the xll test was made for the current decoder, which is not
>> bitexact.
> 
> i just locally merged the branch and did a make -j12 fate

I see dca2 is above dca in allcodecs.c on this patch so i guess it's using the
former, which would explain why it fails.

> 
> 
>>
>> Ideally, once this decoder is committed replacing the current one we'd replace
>> the samples for the set available here: https://github.com/foo86/dcadec-samples
> 
> we can add new fate samples, we cannot replace fate samples
> replacing would break previous checkouts and releases
> not sure you actually meant to replace any ...

Yeah, forgot about that. Kinda makes for a bloated fate suit in the long run...

Since the lossy tests (fate-dca-core and fate-dts_es) apparently still work with
this decoder then there's no need to remove them, but the new extensions still
need the above sample suit.

> 
> [...]
> 
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>