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

[Rostislav Pehlivanov]

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.

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

>+/* 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.

>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