On Thu, Jan 21, 2016 at 1:50 PM, wm4 <nfx...@googlemail.com> wrote: > On Thu, 21 Jan 2016 11:46:03 +0100 > Hendrik Leppkes <h.lepp...@gmail.com> wrote: > >> On Fri, Jan 15, 2016 at 6:12 AM, James Almer <jamr...@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. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
