[FFmpeg-devel] [PATCH][WIP] avfilter: add libebur128 port

Kyle Swanson k at ylo.ph
Mon Jun 13 05:08:04 CEST 2016


On Sun, Jun 12, 2016 at 4:32 PM, Rostislav Pehlivanov
<atomnuker at gmail.com> wrote:
> On 12 June 2016 at 22:14, Kyle Swanson <k at ylo.ph> wrote:
>
>> Hi all,
>>
>> Here's three patches. These are still WIP and not ready to be pushed.
>>
>> 0001-avfilter-add-libebur128-port.patch
>> This first patch ports libebur128 to ffmpeg. I haven't re-indented
>> these yet, so please diff `ebur128.c' and `ebur128.h' with the
>> original libebur128 files[1][2] to see what has changed. Also included
>> is `queue.h' which comes from BSD, which AFAIK should be distributable
>> if we decide to go this route. All these files still need their
>> license header, as libebur128 is MIT licensed and needs its own
>> copyright message. One other thing to take a look at is the section
>> with the sse2 optimizations - does FFmpeg already have a macro we
>> could use for this?
>>
>>
>> 0002-avfilter-af_loudnorm-use-internal-ebur128-api.patch
>> This patch removes the libebur128 dependency for the loudnorm and uses
>> the new internal ebur128 API.
>>
>>
>> 0003-avfilter-af_astats-add-ebur128-stats.patch
>> This patch adds ebur128 stats to the astats filter. Because of the
>> extra computation required to calculate ebur128 stats, I decided that
>> these modes should be explicitly specified via a few new filter
>> parameters. From my perspective, it makes more sense for this to live
>> in the astats filter instead of a completely separate filter
>> (f_ebur128). I'd vote for removing the current ebur128 filter, but if
>> we wanted to keep it it should be ported to use the new ebur128 code.
>>
>> Thanks,
>> Kyle
>>
>> [1]
>> https://raw.githubusercontent.com/jiixyj/libebur128/master/ebur128/ebur128.h
>> [2]
>> https://raw.githubusercontent.com/jiixyj/libebur128/master/ebur128/ebur128.c
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>>
> Hi,
>
>>+#include <xmmintrin.h>
> Intrinsics aren't allowed in the codebase. Looking at the patch it's only
> used in a single place and I can't see how it would improve the
> performance, so it should be safe to remove it.

That's what I figured. I'll remove this part.

>
>>+#if CONFIG_SWRESAMPLE
> What would happen if CONFIG_SWRESAMPLE wasn't enabled?
>

Good point, the only exported function that needs this is
`ff_ebur128_true_peak'. Calls to this function should also include an
`#if CONFIG_SWRESAMPLE.' I'll fix this for the next patch.

>>+/* Those will be calculated when initializing the library */ >+static
> double relative_gate_factor; >+static double minus_twenty_decibels;
>>+static double histogram_energies[1000]; >+static double
> histogram_energy_boundaries[1001];
>
> Do you think it would be easy to put those inside the context?
> Also you should probably define the big double arrays using DECLARE_ALIGNED.

I'll take a look at this.

>
>>libebur128 is MIT licensed and needs its own copyright message
> Had this discussion half a year ago, the correct thing to do in this case
> is to use FFmpeg's license at the top, put a message saying "This file uses
> code from <project name>'s codebase which has the following license", and
> then paste libebur128's license beneath indented one level.
>
> So far apart from those things it looks good, will have time to look at
> this a bit better later this week.
>
> Thanks
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


More information about the ffmpeg-devel mailing list