[FFmpeg-devel] [PATCH 2/3] lavfi/loudnorm: add an internal libebur128 library

Kyle Swanson k at ylo.ph
Thu Nov 10 18:32:18 EET 2016


On Tue, Nov 8, 2016 at 9:39 AM, Kyle Swanson <k at ylo.ph> wrote:
> On Mon, Nov 7, 2016 at 6:00 PM, Marton Balint <cus at 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 <barsnick at 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.

Also, this port is only like ~670 lines of C plus a header. It makes
sense to port it to FFmpeg instead of linking it. Also, on my systems
(osx, linux) af_loudnorm is ~5x faster then it was when libebur128 was
dynamically linked.

Thanks,
Kyle

>
>>
>> Thanks,
>>
>> Marton
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


More information about the ffmpeg-devel mailing list