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

[Hendrik Leppkes]

On Sat, Jan 30, 2016 at 11:02 PM, James Almer <jamrial at gmail.com> wrote:
> On 1/30/2016 6:45 PM, Hendrik Leppkes wrote:
>> On Sat, Jan 30, 2016 at 10:41 PM, James Almer <jamrial at gmail.com> wrote:
>>> On 1/30/2016 6:15 PM, Hendrik Leppkes wrote:
>>>> On Sat, Jan 30, 2016 at 7:05 PM, Hendrik Leppkes <h.leppkes at gmail.com> wrote:
>>>>> On Sat, Jan 30, 2016 at 1:43 AM, Andreas Cadhalpun
>>>>> <andreas.cadhalpun at googlemail.com> wrote:
>>>>>> On 25.01.2016 23:47, Hendrik Leppkes wrote:
>>>>>>> The decoder in itself looks fine to me, short of the regression michael found.
>>>>>>> If you could look at that, that would be great.
>>>>>>>
>>>>>>> I can squash and re-shuffle the commits appropriately for pushing and
>>>>>>> make sure all intermediate steps still build, so don't worry about
>>>>>>> that as much.
>>>>>>> I would love to get this in soon, we can do further improvements and
>>>>>>> more FATE coverage after.
>>>>>>
>>>>>> I agree that it's time to apply these patches. Resending this huge
>>>>>> patch set for tiny improvements is just not practical.
>>>>>>
>>>>>>> There doesn't seem to be much agreement yet of the order of pushing.
>>>>>>> I would argue that since we're replacing the decoder entirely anyway,
>>>>>>> a tiny period in between where we don't actually have a dca decoder
>>>>>>> wouldn't break any bisect flow, since it would probably end there
>>>>>>> anyway.
>>>>>>> So considering that, it feels cleaner to me to push the removal first,
>>>>>>> and then the additions for the new decoder for "prettier" history.
>>>>>>
>>>>>> That seems fine to me, but I don't have a strong opinion about this.
>>>>>>
>>>>>
>>>>> I have started to rebase, update and squash it appropriately,
>>>>> including the patch for Michaels issue I posted earlier.
>>>>> Once its all ready for pushing, I'll post my GitHub link for a final
>>>>> review if anyone wants to, and otherwise push it in the next day or
>>>>> two, so we finally get this done.
>>>>>
>>>>
>>>> Here is the repository, rebased and partially squashed:
>>>> https://github.com/Nevcairiel/FFmpeg/tree/dca
>>>>
>>>> All individual steps build and pass FATE, version bump and Changelog
>>>> entry in the commit with the new decoder.
>>>
>>> It doesn't really need a minor bump. It's not a "new" decoder that wasn't
>>> available before. Just bump micro because of the avoption changes. This
>>> change is completely transparent from an API PoV otherwise.
>>
>> Suppose micro is fine as well.
>>
>>>
>>>>
>>>> - Hendrik
>>>
>>> IMO, merge "avcodec/dca: add math helpers and fixed point DCT" into the
>>> last patch. It's no different than the dsp patch you already squashed.
>>>
>>
>> The new synth filter functions depend on those (fixed point synth
>> filter needs fixed point DCT), so its either squash those as well, or
>> keep them split.
>>
>> - Hendrik
>
> Ah, i see. Ok, keep them split then. Thanks for merging and applying all
> this.

The decoder has been pushed now.