On Sun, Jun 12, 2016 at 4:32 PM, Rostislav Pehlivanov <atomnu...@gmail.com> wrote: > On 12 June 2016 at 22:14, Kyle Swanson <k...@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@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@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
