[FFmpeg-devel] [PATCH v2 00/16] Replace native DCA decoder with libdcadec based one

[James Almer]

On 1/22/2016 7:34 PM, Michael Niedermayer wrote:
> On Fri, Jan 22, 2016 at 06:43:15PM -0300, James Almer wrote:
>> On 1/22/2016 6:40 PM, Michael Niedermayer wrote:
>>> On Thu, Jan 21, 2016 at 09:44:06PM +0300, foo86 wrote:
>>>> Updated version of the patch. I choose to split it into even smaller commits to
>>>> make reviewing easier; you may prefer to squash it as needed.
>>>>
>>>> Changes since the first version:
>>>>
>>>> * Removed checkasm test for dcadsp
>>>> * Removed FATE test for dca-xll (didn't check if this works)
>>>> * Core decoder now uses butterflies_fixed() for sumdiff decoding
>>>> * avpriv_request_sample() is now used for reporting missing features
>>>> * Core decoder now stays in fixed point mode during intermittent XLL decoding errors
>>>> * X96 extension is no longer parsed (and left unused) when decoding XLL
>>>> * Minor code refactoring
>>>>
>>>> foo86 (16):
>>>>   avcodec/dca: remove old decoder
>>>>   avcodec/dca: remove unused assembly
>>>>   avcodec/dca: remove unused data
>>>>   tests/fate/audio: remove dca-xll test
>>>>   tests/checkasm: remove dcadsp test
>>>>   avcodec/dca: add REV1AUX sync word
>>>>   avcodec/dca: add more tables
>>>>   avcodec/dca: add math helpers and fixed point DCT
>>>>   avcodec/synth_filter: fix whitespace
>>>>   avcodec/synth_filter: add more filters
>>>>   avcodec/dca: add DSP implementation
>>>>   avcodec/dca: add generic defines
>>>>   avcodec/dca: add core decoder
>>>>   avcodec/dca: add EXSS parser
>>>>   avcodec/dca: add XLL decoder
>>>>   avcodec/dca: add new decoder based on libdcadec
>>>
>>> intermediates dont seem to build
>>> "tests/fate/acodec.mak:102: *** No such config: CONFIG_DCA_DECODER.  Stop."
>>>
>>> also i would have thoght that the new decoder would be added and
>>> the old removed once the new was in, so that all intermedate versions
>>> work with all files
>>>
>>> having intermedates work could help with potential future debug/
>>> git bisect of bugs
>>
>> Adding the new first then removing the old after is a bad idea.
>>
>> This patchset is meant to be squashed at least in part, anyway. And i agree
>> that whatever is committed should not break fate, so the patch that removes
>> the old decoder should at the same time remove/disable the fate tests, for
>> example, which can be done by squashing patches 1 to 5.
> 
> after 1-5 i get
> 
> tests/fate/acodec.mak:102: *** No such config: CONFIG_DCA_DECODER.  Stop.
> 
> with make fate
> 
> also
> git grep CONFIG_DCA_DECODER
> has plenty of matches but its missing in config.mak/h

Mmh, yeah, looks like it's because the synth_filter stuff and the fate-dca
and fate-dts_es tests were left intact.
It should be a matter of removing the CONFIG_DCA_DECODER references in all
the Makefiles and checkasm without actually removing the asm files, and
commenting the tests out as part of the removal patch/es.

I'd very much like if we don't squash the removal and addition into a single
commit, since it will be messy and hard to read from within git's history.
Ideally it should be a single removal patch, followed by misc patches that
make changes to existing files, then a single addition made with most of the
second half of this patchset.