[FFmpeg-devel] [PATCH 0/7] Replace native DCA decoder with libdcadec based one

[Hendrik Leppkes]

On Thu, Jan 21, 2016 at 1:50 PM, wm4 <nfxjfg at googlemail.com> wrote:
> On Thu, 21 Jan 2016 11:46:03 +0100
> Hendrik Leppkes <h.leppkes at gmail.com> wrote:
>
>> On Fri, Jan 15, 2016 at 6:12 AM, James Almer <jamrial at gmail.com> wrote:
>> > On 1/14/2016 9:59 PM, Andreas Cadhalpun wrote:
>> >> On 14.01.2016 17:25, foo86 wrote:
>> >>> Full diff output has been omitted for deleted files. If git complains about
>> >>> applying the first patch, this can be also pulled from dca-replace branch at
>> >>> [1].
>> >>
>> >> I'd prefer if you would post full patches here, i.e. including deleted files.
>> >
>> > For future, smaller patches if anything. Doing so here would mean thousands of
>> > extra lines for no reason or benefit. And i remember there being a problem with
>> > git send-email regarding huge emails.
>> >
>> >>
>> >> That aside, the new decoder seems fine from a security point of view, with
>> >> only some rare overflows in the dsp functions left.
>> >>
>> >> However, this series breaks FATE.
>> >> The fate-dca-xll test should probably be disabled, as the reference was created
>> >> with the old, not bitexact decoder. (It currently fails due to the disable_xll
>> >> option being gone, but fixing that reveals the changed output.)
>> >>
>> >> The checkasm test needs to be updated:
>> >
>> > Removed, actually. The new lfe_fir dsp functions don't yet have arch optimized
>> > versions, so even if you adapt this code now it will not be really tested and
>> > reported as part of the checkasm output since there's nothing to compare the C
>> > version with.
>> > It can be readded and adapted once said arch optimized functions are written.
>> >
>>
>> Other than the comments already made here, this set looks fine to me.
>> The missing changes are trivial removals of (now broken) tests, so I
>> could do that when pushing as well if we all agree.
>>
>> New tests should then be added afterwards.
>>
>> Everyone else, if you still have outstanding comments on this set,
>> please do speak up soon, otherwise I would say we give it another
>> couple days and then just push it, so we can move forward.
>
>> Anyone any opinions if this should be squashed to avoid leaving us
>> without a dca decoder for a few commits? Probably should be, right?
>
> Why would this be important? Except maybe that it would be nice if
> FATE worked at any point in the history (to make bisecting easier).

That would mean removing and re-adding some stuff, which seems
somewhat annoying. But I don't really care either way.