On 11/10/16, Hendrik Leppkes <h.lepp...@gmail.com> wrote: > On Thu, Nov 10, 2016 at 5:32 PM, Kyle Swanson <k...@ylo.ph> wrote: >> On Tue, Nov 8, 2016 at 9:39 AM, Kyle Swanson <k...@ylo.ph> wrote: >>> On Mon, Nov 7, 2016 at 6:00 PM, Marton Balint <c...@passwd.hu> wrote: >>>> >>>> On Fri, 4 Nov 2016, Marton Balint wrote: >>>> >>>>> >>>>> On Thu, 3 Nov 2016, Hendrik Leppkes wrote: >>>>> >>>>>> On Mon, Oct 17, 2016 at 5:20 PM, Moritz Barsnick <barsn...@gmx.net> >>>>>> wrote: >>>>>>> >>>>>>> On Mon, Oct 17, 2016 at 17:09:15 +0200, wm4 wrote: >>>>>>>> >>>>>>>> Does this copy parts of libebur128 to FFmpeg? >>>>>>>> Why? >>>>>>> >>>>>>> >>>>>>> There was a long discussion regarding this patch: >>>>>>> >>>>>>> http://lists.ffmpeg.org/pipermail/ffmpeg-devel/2016-April/192668.html >>>>>>> >>>>>>> (in summary: "please don't require yet another small external >>>>>>> library, >>>>>>> rather port it to ffmpeg and maintain it") leading to this one: >>>>>>> >>>>>> >>>>>> The generic idea was not to just copy/paste an external library into >>>>>> internal code, but extend the ebur128 code we already have - at least >>>>>> that way we get code written by one of our maintainers, code he knows >>>>>> and can properly maintain. >>>>> >>>>> >>>>> I copied the external library because we needed an API. The way the >>>>> internals work, I used the library code because it was simply easier, >>>>> than >>>>> factoring out f_ebur128 stuff, also there are some features which >>>>> f_ebur128.c does not have (variable sample rate support), and there was >>>>> the >>>>> licensing issue GPL v.s. LGPL. >>>>> >>>>>> If you just copy the implementation of a library, you might as well >>>>>> just use that library - thats what it exists for. Why do we want to >>>>>> increase the maintenance burden of our project when other people (ie. >>>>>> the authors of libebur128) are already doing it as well? >>>>> >>>>> >>>>> In general I agree with people who think that for small code, it is >>>>> better >>>>> to integrate it into our codebase, because >>>>> - it can benefit from features we already have (e.g. resampling) >>>>> - makes it easier for developers to work on features based on this >>>>> - code gets more review than code in a small 3rd party library >>>>> - code and/or improvements have stronger requirements performance-wise >>>>> - code is better audited (Coverity, etc) >>>>> >>>>> Yes, some additional maintenance burden is the price we pay for this, >>>>> which is IMHO in this case is acceptable. >>>>> >>>> >>>> Is it fine to apply, or we should put this to a vote? >>> >>> Give me another day to review the patch. Meant look at this last weekend. >> >> These patches look good to me. If we're going to do this, we really >> need to keep the true peak mode in the libebur128 port. This is a huge >> part of the R128 spec, and it's important that it stays in. Of course >> redundant code is bad, so we'll need to update f_ebur128 as well >> (which has a true peak requirement.) I already have a patch for >> f_ebur128. Marton, it might be easier for you to update the patch to >> include true peak mode but I could do it as well. It'd be interesting >> to benchmark the libebur128 FIR resampler vs. swresample. > > If this gets re-added, please make it use our resampler unless there > are good technical reasons against that (algorithm wise). > Keeping an extra small resampler just for some measurements seems like > something that can be surely avoided.
Yes, I also agree. Is such small resampler really needed? _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
