Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On Sun, Jan 25, 2015 at 11:53:26PM +0530, arwa arif wrote: > I have updated the patch. > > I checked the output with many combinations of parameters. It is bitexact > now. merged with pauls changes and applied > I am facing problems in rebasing against the latest master. i asked stefano to help/explain git rebase tell me in case he doesnt have time Thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The real ebay dictionary, page 2 "100% positive feedback" - "All either got their money back or didnt complain" "Best seller ever, very honest" - "Seller refunded buyer after failed scam" signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On Mon, Jan 19, 2015 at 01:19:00PM +0100, Stefano Sabatini wrote:
> On date Monday 2015-01-19 04:04:45 +0530, Arwa Arif encoded:
[...]
> > +{ "gamma_b","gamma value for the luma plane",
> > +OFFSET(gamma_b),AV_OPT_TYPE_DOUBLE, {.dbl = 1.0}, 0.1, 10.0,
> > FLAGS },
> > +{ "gamma_g","gamma value for the 1st chroma plane",
> > +OFFSET(gamma_g),AV_OPT_TYPE_DOUBLE, {.dbl = 1.0}, 0.1, 10.0,
> > FLAGS },
> > +{ "gamma_r","gamma value for the 2st chroma plane",
> > +OFFSET(gamma_r),AV_OPT_TYPE_DOUBLE, {.dbl = 1.0}, 0.1, 10.0,
> > FLAGS },
>
> gamma_{y,u,v} are probably better names.
gamma is in RGB space
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
I have updated the patch.
I checked the output with many combinations of parameters. It is bitexact
now.
I am facing problems in rebasing against the latest master.
From f6c6a66b306475e3bc7977f59287c920f5e867a7 Mon Sep 17 00:00:00 2001
From: Arwa Arif
Date: Mon, 19 Jan 2015 03:56:48 +0530
Subject: [PATCH] Port mp=eq/eq2 to FFmpeg
---
configure| 38 +++
doc/filters.texi | 43 +++
libavfilter/Makefile |1 +
libavfilter/allfilters.c |1 +
libavfilter/vf_eq.c | 285 ++
libavfilter/vf_eq.h | 63 ++
libavfilter/x86/Makefile |1 +
libavfilter/x86/vf_eq.c | 96
8 files changed, 503 insertions(+), 25 deletions(-)
create mode 100644 libavfilter/vf_eq.c
create mode 100644 libavfilter/vf_eq.h
create mode 100644 libavfilter/x86/vf_eq.c
diff --git a/configure b/configure
index c73562b..d122720 100755
--- a/configure
+++ b/configure
@@ -60,7 +60,6 @@ show_help(){
cat <= 1.4" xcb/xcb.h xcb_connect || {
+enabled libxcb && die "ERROR: libxcb >= 1.4 not found";
} && disable x11grab && enable libxcb
if enabled libxcb; then
@@ -5694,7 +5684,7 @@ cat > $TMPH <
+ * Copyright (c) 2015 Arwa Arif
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with FFmpeg; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+/**
+ * @file
+ * very simple video equalizer
+ */
+
+/**
+ * TODO:
+ * - Add support to process_command
+ */
+
+#include "libavfilter/internal.h"
+#include "libavutil/common.h"
+#include "libavutil/imgutils.h"
+#include "libavutil/opt.h"
+#include "libavutil/pixdesc.h"
+#include "vf_eq.h"
+
+static void create_lut(EQParameters *param)
+{
+unsigned i;
+double g, v;
+double lw, gw;
+
+g = param->gamma;
+gw = param->gamma_weight;
+lw = 1.0 - gw;
+
+g = 1.0 / g;
+
+for (i = 0; i < 256; i++) {
+v = (double) i / 255.0;
+v = param->contrast * (v - 0.5) + 0.5 + param->brightness;
+
+if (v <= 0.0)
+param->lut[i] = 0;
+
+else {
+v = v*lw + pow(v, g)*gw;
+if (v >= 1.0)
+param->lut[i] = 255;
+else
+param->lut[i] = (unsigned char) (256.0 * v);
+}
+}
+
+for (i = 0; i < 256 * 256; i++)
+param->lut16[i] = param->lut[i & 0xFF] + (param->lut[i >> 8] << 8);
+
+param->lut_clean = 1;
+}
+
+static void apply_lut(EQParameters *param, uint8_t *dst, int dst_stride,
+ uint8_t *src, int src_stride, int w, int h)
+{
+int x, y;
+
+if (!param->lut_clean)
+create_lut(param);
+
+for (y = 0; y < h; y++) {
+for (x = 0; x < w; x++) {
+dst[y * dst_stride + x] = param->lut[src[y * src_stride + x]];
+}
+}
+}
+
+static void process_c(EQParameters *param, uint8_t *dst, int dst_stride,
+ uint8_t *src, int src_stride, int w, int h)
+{
+int x, y, pel;
+
+int contrast = (int) (param->contrast * 256 * 16);
+int brightness = ((int) (100.0 * param->brightness + 100.0) * 511) / 200 - 128 - contrast / 32;
+
+for (y = 0; y < h; y++) {
+for (x = 0; x < w; x++) {
+pel = ((src[y * src_stride + x] * contrast) >> 16) + brightness;
+
+if (pel & 768)
+pel = (-pel) >> 31;
+
+dst[y * dst_stride + x] = pel;
+}
+}
+}
+
+static void check_values(EQParameters *param, EQContext *eq)
+{
+if (param->contrast == 1.0 && param->brightness == 0.0 && param->gamma == 1.0)
+param->adjust = NULL;
+else if (param->gamma == 1.0)
+param->adjust = eq->process;
+else
+param->adjust = apply_lut;
+}
+
+static void set_contrast(EQContext *eq)
+{
+eq->param[0].contrast = eq->contrast;
+eq->param[0].lut_clean = 0;
+check_values(&eq->param[0], eq);
+}
+
+static void set_brightness(EQContext *eq)
+{
+eq->param[0].brightness = eq->brightness;
+eq->param[0].lut_clean = 0;
+check_values(&eq->param[0], eq);
+}
+
+static void set_gamma(EQContext *eq)
+{
+int i;
+eq->param[0].gamma = eq->gamma * eq->gamma_u;
+eq->param[1].gamma = sqrt(eq->gamma_v / eq->gamma_u);
+eq->param[2].gamma = sqrt(eq->gamma_y / eq->gamma_u);
+
+for (i = 0; i < 3; i++) {
+eq->param[i].gamma_weight = eq->gamm
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On 1/23/15, Stefano Sabatini wrote: > On date Friday 2015-01-23 20:44:01 +0530, Arwa Arif encoded: > [...] >> > Looks good otherwise, assuming it is bitexact with the mp=eq2. >> > >> >> The default is bit-exact with mp=eq2, I can't check it with other values, >> because the range of values in mp is different from the range of values >> in >> this code. > > What's wrong with testing with some random in-range values? Also why > do they differ? > Ranges of values are same, but order is different. I taken this code and fixed order and at least saturation does not work, also it is not using same code as eq2. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On date Friday 2015-01-23 20:44:01 +0530, Arwa Arif encoded:
[...]
> > Looks good otherwise, assuming it is bitexact with the mp=eq2.
> >
>
> The default is bit-exact with mp=eq2, I can't check it with other values,
> because the range of values in mp is different from the range of values in
> this code.
What's wrong with testing with some random in-range values? Also why
do they differ?
>
>
> >
> > Also, what about adding expressions support (like done in hue)? (this
> > will go of course to a separate patch).
> >
>
> What is meant by expressions support?
See for example:
75d34864d16fbf02928c319134bd94a7a7752092
> From f9610db3c8ce209b57e55e2bf778b03da8d3d4a1 Mon Sep 17 00:00:00 2001
> From: Arwa Arif
> Date: Mon, 19 Jan 2015 03:56:48 +0530
> Subject: [PATCH] Port mp=eq/eq2 to FFmpeg
>
> Code adapted from James Darnley's port
Nit: incomplete sentence, final dot missing.
> ---
> configure| 38 +++
> doc/filters.texi | 43 +++
> libavfilter/Makefile |1 +
> libavfilter/allfilters.c |1 +
> libavfilter/vf_eq.c | 284
> ++
> libavfilter/vf_eq.h | 63 ++
> libavfilter/x86/Makefile |1 +
> libavfilter/x86/vf_eq.c | 94 +++
> 8 files changed, 500 insertions(+), 25 deletions(-)
> create mode 100644 libavfilter/vf_eq.c
> create mode 100644 libavfilter/vf_eq.h
> create mode 100644 libavfilter/x86/vf_eq.c
>
> diff --git a/configure b/configure
> index c73562b..d122720 100755
> --- a/configure
> +++ b/configure
> @@ -60,7 +60,6 @@ show_help(){
> cat < Usage: configure [options]
> Options: [defaults in brackets after descriptions]
> -
unrelated change, here and below
[...]
> +@section eq
> +Set brightness, contrast, saturation and gamma adjustment.
> +
> +The filter accepts the following options:
> +
> +@table @option
> +@item brightness
> +Set the brightness value. It accepts a float value in range @code{-1.0} to
> +@code{1.0}. The default value is @code{0.0}.
> +
> +@item contrast
> +Set the contrast value. It accepts a float value in range @code{-2.0} to
> +@code{2.0}. The default value is @code{0.0}.
> +
> +@item gamma
> +Set the gamma value. It accepts a float value in range @code{0.1} to
> @code{10.0}.
> +The default value is @code{1.0}.
> +
> +@item gamma_y
> +Set the gamma value for the luma plane. It accepts a float value in range
> +@code{0.1} to @code{10.0}. The default value is @code{1.0}.
> +
> +@item gamma_u
> +Set the gamma value for 1st chroma plane. It accepts a float value in range
> +@code{0.1} to @code{10.0}. The default value is @code{1.0}.
> +
> +@item gamma_v
> +Set the gamma value for 2nd chroma plane. It accepts a float value in range
> +@code{0.1} to @code{10.0}. The default value is @code{1.0}.
> +
> +@item saturation
> +Set the saturation value. It accepts a float value in range @code{0.0} to
> +@code{3.0}. The default value is @code{1.0}.
> +
> +@item gamma_weight
> +Can be used to reduce the effect of a high gamma value on bright image areas,
> +e.g. keep them from getting overamplified and just plain white. It accepts a
> +float value in range @code{0.0} to @code{1.0}.A value of @code{0.0} turns the
> +gamma correction all the way down while @code{1.0} leaves it at its full
> strength.
> +Default is @code{1.0}.
> +
> +@end table
> +
> @section extractplanes
>
> Extract color channel components from input video stream into
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index e43d76d..8e94033 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -116,6 +116,7 @@ OBJS-$(CONFIG_DRAWGRID_FILTER) +=
> vf_drawbox.o
> OBJS-$(CONFIG_DRAWTEXT_FILTER) += vf_drawtext.o
> OBJS-$(CONFIG_ELBG_FILTER) += vf_elbg.o
> OBJS-$(CONFIG_EDGEDETECT_FILTER) += vf_edgedetect.o
> +OBJS-$(CONFIG_EQ_FILTER) += vf_eq.o
> OBJS-$(CONFIG_EXTRACTPLANES_FILTER) += vf_extractplanes.o
> OBJS-$(CONFIG_FADE_FILTER) += vf_fade.o
> OBJS-$(CONFIG_FIELD_FILTER) += vf_field.o
> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> index 381da4f..db34cb9 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -132,6 +132,7 @@ void avfilter_register_all(void)
> REGISTER_FILTER(DRAWTEXT, drawtext, vf);
> REGISTER_FILTER(EDGEDETECT, edgedetect, vf);
> REGISTER_FILTER(ELBG, elbg, vf);
> +REGISTER_FILTER(EQ, eq, vf);
> REGISTER_FILTER(EXTRACTPLANES, extractplanes, vf);
> REGISTER_FILTER(FADE, fade, vf);
> REGISTER_FILTER(FIELD, field, vf);
> diff --git a/libavfilter/vf_eq.c b/libavfilter/vf_eq.c
> new file mode 100644
> index 000..494eb63
> --- /dev/null
> +++ b/libavfilter/vf_eq.c
> @@ -0,0 +1,284 @@
> +/*
> + * Original MPlayer filters
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On Fri, Jan 23, 2015 at 8:05 PM, Stefano Sabatini
wrote:
> On date Thursday 2015-01-22 01:38:11 +0530, Arwa Arif encoded:
> > On Thu, Jan 22, 2015 at 12:09 AM, Paul B Mahol wrote:
> [...]
> > From 703cc1887903c2868537e19e99b76927bec07884 Mon Sep 17 00:00:00 2001
> > From: Arwa Arif
> > Date: Mon, 19 Jan 2015 03:56:48 +0530
> > Subject: [PATCH] Port mp=eq/eq2 to FFmpeg
> >
>
> > Code adapted from James Darnley's previous commits
>
> Code adapted from James Darnley's port.
>
> There are no related commits in the FFmpeg master repo.
>
> > ---
> > configure|1 +
> > doc/filters.texi | 44
> > libavfilter/Makefile |1 +
> > libavfilter/allfilters.c |1 +
> > libavfilter/vf_eq.c | 282
> ++
> > libavfilter/vf_eq.h | 63 +++
> > libavfilter/x86/Makefile |1 +
> > libavfilter/x86/vf_eq.c | 94
> > 8 files changed, 487 insertions(+)
> > create mode 100644 libavfilter/vf_eq.c
> > create mode 100644 libavfilter/vf_eq.h
> > create mode 100644 libavfilter/x86/vf_eq.c
> >
> > diff --git a/configure b/configure
> > index c73562b..138852e 100755
> > --- a/configure
> > +++ b/configure
> > @@ -2579,6 +2579,7 @@ delogo_filter_deps="gpl"
> > deshake_filter_select="pixelutils"
> > drawtext_filter_deps="libfreetype"
> > ebur128_filter_deps="gpl"
> > +eq_filter_deps="gpl"
> > flite_filter_deps="libflite"
> > frei0r_filter_deps="frei0r dlopen"
> > frei0r_src_filter_deps="frei0r dlopen"
> > diff --git a/doc/filters.texi b/doc/filters.texi
> > index d7b2273..70e0557 100644
> > --- a/doc/filters.texi
> > +++ b/doc/filters.texi
> > @@ -4320,6 +4320,50 @@ edgedetect=mode=colormix:high=0
> > @end example
> > @end itemize
> >
> > +@section eq
>
> > +Equalizer that uses lookup tables (very slow), allowing gamma correction
> > +in addition to simple brightness and contrast adjustment.
>
> This is not fitting (it's not always using LUTs). Also tells what the
> filter does, not what the filter is, as in the rest of the filters
> documentation. Something like this:
>
> Set brightness, contrast, saturation and gamma adjustment.
>
> > +
> > +The filter accepts the following options:
> > +
> > +@table @option
> > +@item brightness
> > +Set the brightness value. It accepts a float value in range @code{-1.0}
> to
> > +@code{1.0}. The default value is @code{0.0}.
> > +
> > +@item contrast
> > +Set the contrast value. It accepts a float value in range @code{-2.0} to
> > +@code{2.0}. The default value is @code{0.0}.
> > +
> > +@item gamma
> > +Set the gamma value. It accepts a float value in range @code{0.1} to
> @code{10.0}.
> > +The default value is @code{1.0}.
> > +
> > +@item gamma_y
> > +Set the gamma value for the luma plane. It accepts a float value in
> range
> > +@code{0.1} to @code{10.0}. The default value is @code{1.0}.
> > +
> > +@item gamma_u
> > +Set the gamma value for 1st chroma plane. It accepts a float value in
> range
> > +@code{0.1} to @code{10.0}. The default value is @code{1.0}.
> > +
> > +@item gamma_v
> > +Set the gamma value for 2nd chroma plane. It accepts a float value in
> range
> > +@code{0.1} to @code{10.0}. The default value is @code{1.0}.
> > +
> > +@item saturation
> > +Set the saturation value. It accepts a float value in range @code{0.0}
> to
> > +@code{3.0}. The default value is @code{1.0}.
> > +
> > +@item weight
>
> probably gamma_weight it's better, to make clear it is related to
> gamma correction.
>
> > +Can be used to reduce the effect of a high gamma value on bright image
> areas,
> > +e.g. keep them from getting overamplified and just plain white. It
> accepts a
> > +float value in range @code{0.0} to @code{1.0}.A value of @code{0.0}
> turns the
> > +gamma correction all the way down while @code{1.0} leaves it at its
> full strength.
> > +Default is @code{1.0}.
> > +
> > +@end table
> > +
> > @section extractplanes
> >
> > Extract color channel components from input video stream into
> > diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> > index e43d76d..8e94033 100644
> > --- a/libavfilter/Makefile
> > +++ b/libavfilter/Makefile
> > @@ -116,6 +116,7 @@ OBJS-$(CONFIG_DRAWGRID_FILTER) +=
> vf_drawbox.o
> > OBJS-$(CONFIG_DRAWTEXT_FILTER) += vf_drawtext.o
> > OBJS-$(CONFIG_ELBG_FILTER) += vf_elbg.o
> > OBJS-$(CONFIG_EDGEDETECT_FILTER) += vf_edgedetect.o
> > +OBJS-$(CONFIG_EQ_FILTER) += vf_eq.o
> > OBJS-$(CONFIG_EXTRACTPLANES_FILTER) += vf_extractplanes.o
> > OBJS-$(CONFIG_FADE_FILTER) += vf_fade.o
> > OBJS-$(CONFIG_FIELD_FILTER) += vf_field.o
> > diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> > index 381da4f..db34cb9 100644
> > --- a/libavfilter/allfilters.c
> > +++ b/libavfilter/allfilters.c
> > @@ -132,6 +132,7 @@ void avfilter_register_all(void)
> > REGISTER_FILTER(DRAWTEXT, drawtext,
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On date Thursday 2015-01-22 01:38:11 +0530, Arwa Arif encoded:
> On Thu, Jan 22, 2015 at 12:09 AM, Paul B Mahol wrote:
[...]
> From 703cc1887903c2868537e19e99b76927bec07884 Mon Sep 17 00:00:00 2001
> From: Arwa Arif
> Date: Mon, 19 Jan 2015 03:56:48 +0530
> Subject: [PATCH] Port mp=eq/eq2 to FFmpeg
>
> Code adapted from James Darnley's previous commits
Code adapted from James Darnley's port.
There are no related commits in the FFmpeg master repo.
> ---
> configure|1 +
> doc/filters.texi | 44
> libavfilter/Makefile |1 +
> libavfilter/allfilters.c |1 +
> libavfilter/vf_eq.c | 282
> ++
> libavfilter/vf_eq.h | 63 +++
> libavfilter/x86/Makefile |1 +
> libavfilter/x86/vf_eq.c | 94
> 8 files changed, 487 insertions(+)
> create mode 100644 libavfilter/vf_eq.c
> create mode 100644 libavfilter/vf_eq.h
> create mode 100644 libavfilter/x86/vf_eq.c
>
> diff --git a/configure b/configure
> index c73562b..138852e 100755
> --- a/configure
> +++ b/configure
> @@ -2579,6 +2579,7 @@ delogo_filter_deps="gpl"
> deshake_filter_select="pixelutils"
> drawtext_filter_deps="libfreetype"
> ebur128_filter_deps="gpl"
> +eq_filter_deps="gpl"
> flite_filter_deps="libflite"
> frei0r_filter_deps="frei0r dlopen"
> frei0r_src_filter_deps="frei0r dlopen"
> diff --git a/doc/filters.texi b/doc/filters.texi
> index d7b2273..70e0557 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -4320,6 +4320,50 @@ edgedetect=mode=colormix:high=0
> @end example
> @end itemize
>
> +@section eq
> +Equalizer that uses lookup tables (very slow), allowing gamma correction
> +in addition to simple brightness and contrast adjustment.
This is not fitting (it's not always using LUTs). Also tells what the
filter does, not what the filter is, as in the rest of the filters
documentation. Something like this:
Set brightness, contrast, saturation and gamma adjustment.
> +
> +The filter accepts the following options:
> +
> +@table @option
> +@item brightness
> +Set the brightness value. It accepts a float value in range @code{-1.0} to
> +@code{1.0}. The default value is @code{0.0}.
> +
> +@item contrast
> +Set the contrast value. It accepts a float value in range @code{-2.0} to
> +@code{2.0}. The default value is @code{0.0}.
> +
> +@item gamma
> +Set the gamma value. It accepts a float value in range @code{0.1} to
> @code{10.0}.
> +The default value is @code{1.0}.
> +
> +@item gamma_y
> +Set the gamma value for the luma plane. It accepts a float value in range
> +@code{0.1} to @code{10.0}. The default value is @code{1.0}.
> +
> +@item gamma_u
> +Set the gamma value for 1st chroma plane. It accepts a float value in range
> +@code{0.1} to @code{10.0}. The default value is @code{1.0}.
> +
> +@item gamma_v
> +Set the gamma value for 2nd chroma plane. It accepts a float value in range
> +@code{0.1} to @code{10.0}. The default value is @code{1.0}.
> +
> +@item saturation
> +Set the saturation value. It accepts a float value in range @code{0.0} to
> +@code{3.0}. The default value is @code{1.0}.
> +
> +@item weight
probably gamma_weight it's better, to make clear it is related to
gamma correction.
> +Can be used to reduce the effect of a high gamma value on bright image areas,
> +e.g. keep them from getting overamplified and just plain white. It accepts a
> +float value in range @code{0.0} to @code{1.0}.A value of @code{0.0} turns the
> +gamma correction all the way down while @code{1.0} leaves it at its full
> strength.
> +Default is @code{1.0}.
> +
> +@end table
> +
> @section extractplanes
>
> Extract color channel components from input video stream into
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index e43d76d..8e94033 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -116,6 +116,7 @@ OBJS-$(CONFIG_DRAWGRID_FILTER) +=
> vf_drawbox.o
> OBJS-$(CONFIG_DRAWTEXT_FILTER) += vf_drawtext.o
> OBJS-$(CONFIG_ELBG_FILTER) += vf_elbg.o
> OBJS-$(CONFIG_EDGEDETECT_FILTER) += vf_edgedetect.o
> +OBJS-$(CONFIG_EQ_FILTER) += vf_eq.o
> OBJS-$(CONFIG_EXTRACTPLANES_FILTER) += vf_extractplanes.o
> OBJS-$(CONFIG_FADE_FILTER) += vf_fade.o
> OBJS-$(CONFIG_FIELD_FILTER) += vf_field.o
> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> index 381da4f..db34cb9 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -132,6 +132,7 @@ void avfilter_register_all(void)
> REGISTER_FILTER(DRAWTEXT, drawtext, vf);
> REGISTER_FILTER(EDGEDETECT, edgedetect, vf);
> REGISTER_FILTER(ELBG, elbg, vf);
> +REGISTER_FILTER(EQ, eq, vf);
> REGISTER_FILTER(EXTRACTPLANES, extractplanes, vf);
> REGISTER_FILTER(FADE, fade, vf
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On date Wednesday 2015-01-21 20:38:20 +0530, Arwa Arif encoded: > > > > I still expect that eq and eq2 should have the same performances, > > since the adjust callback is set depending on the parameter values. So > > we should have a single eq filter. > > > > Please investigate about why you get different benchmark values. > > > > I used this command: ffmpeg -benchmark -i matrixbench_mpeg2.mpg -vf mp=eq2 > -f null - > And everytime I am running this command, I am getting a different result > even for the same input and same filter. > > So, I tried using the time.h library for getting the time, the results for > eq and eq2 are 37.71 and 35.56 seconds respectively. Please show the patch adding time.h benchmarks (in general making things easily reproduce helps *a lot* the tester and the reviewer). time.h usually works by creating log messages about the specific code segment called performance, it doesn't affect global execution time, so it looks like your benchmark was probably misguided. > I don't know why eq is coming out to be slower than eq2. (In fact, eq2 is > accessing more functions than eq). -- FFmpeg = Fascinating and Fundamental Miracolous Philosofic Enlightened Guru ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
So...
2015-01-21 21:08 GMT+01:00 arwa arif :
> Updated the patch.
There are trailing spaces, and the patch does not apply here (error on
libavfilter/x86/Makefile)
Furthemore, I think that hunk is incorrect:
+set_gamma(eq);
+set_contrast(eq);
+set_brightness(eq);
+set_saturation(eq);
+
+for (i = 0; i < 3; i++) {
+eq->param[i].c = (eq->param[i].contrast) * 65536.0;
+eq->param[i].b = (eq->param[i].brightness + 1.0) * 255.5 -
128.0 - (eq->param[i].contrast) * 128.0;
+}
+
+eq->process = process_c;
+
+if (ARCH_X86)
+ff_eq_init_x86(eq);
The adjust function pointer may be set to eq->process potentially, but
eq->process won't ever be set at that moment, as it is set in the last
part.
Once fixed, I get with gcc 4.9.2, Mingw64 (not msys2) on a core i5-460
(up to sse4.2)
C: 16098398 decicycles in eq, 2048 runs, 0 skips CRC=0xb4a713e1
MMX: 5912547 decicycles in eq, 2048 runs, 0 skips CRC=0x1dfc0418
vect: 8663446 decicycles in eq, 2048 runs, 0 skips CRC=0xb4a713e1
Note: vect is -O3 -msse4.2 -ftree-vectorize
--
Christophe
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On Wed, 21 Jan 2015 20:38:20 +0530 arwa arif wrote: > > > > I still expect that eq and eq2 should have the same performances, > > since the adjust callback is set depending on the parameter values. So > > we should have a single eq filter. > > > > Please investigate about why you get different benchmark values. > > > > I used this command: ffmpeg -benchmark -i matrixbench_mpeg2.mpg -vf mp=eq2 > -f null - > And everytime I am running this command, I am getting a different result > even for the same input and same filter. It could be that this command line doesn't actually run any of the actual filter code, because the parameters (brightness etc.) are set to "neutral" by default. Did you make sure the C and ASM filter code is actually run? > > So, I tried using the time.h library for getting the time, the results for > eq and eq2 are 37.71 and 35.56 seconds respectively. > > I don't know why eq is coming out to be slower than eq2. (In fact, eq2 is > accessing more functions than eq). > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On Thu, Jan 22, 2015 at 09:45:04PM +0530, arwa arif wrote: [...] > I checked the runtime of the codes with and without asm, it turns out that > there is not much difference. The difference is coming out to be in > milliseconds. > > 26.014s with asm and > 26.129 without asm. > > So, should I remove the asm part? > If you want to compare the speed of the ASM itself, you need to compare with START_TIMER and STOP_TIMER macros. But before you do that, I'd like to raise an issue (which might be kind of off topic, sorry about that). In 2009, this was committed: 973859f5230e77beea7bb59dc081870689d6d191 It disables the tree-vectorize optimizations (we compile FFmpeg at -O3 where it's enabled by default, but we explicitly disables it). In this particular case, it seems that GCC is able to vectorize the C version using SSE* code, which means it's actually potentially faster¹ than the home written MMX version we have² I'd suggest we try to revert that commit, and next time we write or port optimizations we make a fair comparison. That is, with code generated by a modern compiler without such option. If the old MMX code is actually making the code slower, we should not waste our time with it. Benchmarks welcome. [1] http://pastie.org/pastes/9852329/text (-O3) [2] http://pastie.org/pastes/9852328/text (-O3 -fno-tree-vectorize / current) -- Clément B. pgp8e9FOxf5WX.pgp Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On Thu, 22 Jan 2015 16:59:24 -0300 James Almer wrote: > On 22/01/15 4:52 PM, wm4 wrote: > > On Thu, 22 Jan 2015 16:43:16 -0300 > > James Almer wrote: > > > >> On 22/01/15 4:27 PM, wm4 wrote: > >>> Then I'd definitely vote for remove. > >>> > >>> The asm probably mattered on ancient CPUs and ancient compilers, but > >>> there's no reason to keep it anymore. > >> > >> No. If the handwritten asm is better than the C code, even if slightly, > >> then > >> it should not be removed. > >> And if someone dislikes its inline asm nature then they are free to port > >> it, > >> like i did with a couple other filters before. > > > > For such a small difference, your statement is ridiculous. > > > > No, really. > > Grab any audio file and try to decode it, manually disabling different audio > dsp > functions it uses from libavcodec/libswresample and recompiling, and see how > much > each of them affect overall decoding speed. > You'll find that many don't even seem to have any effect if you only check > with > time, yet are still 2 to 4 times faster than their C counterparts. > > Do you want to remove them as well? That's hard to tell; if they're not bottlenecks (and can never be), then this asm would have been a case of premature optimization. In this case, it seems unlikely that vf_eq will ever be a bottleneck, or that the asm would help if it was. (You can still add back the asm if this changes... it's part of the git history, and won't run away.) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On 22/01/15 4:52 PM, wm4 wrote: > On Thu, 22 Jan 2015 16:43:16 -0300 > James Almer wrote: > >> On 22/01/15 4:27 PM, wm4 wrote: >>> Then I'd definitely vote for remove. >>> >>> The asm probably mattered on ancient CPUs and ancient compilers, but >>> there's no reason to keep it anymore. >> >> No. If the handwritten asm is better than the C code, even if slightly, then >> it should not be removed. >> And if someone dislikes its inline asm nature then they are free to port it, >> like i did with a couple other filters before. > > For such a small difference, your statement is ridiculous. > > No, really. Grab any audio file and try to decode it, manually disabling different audio dsp functions it uses from libavcodec/libswresample and recompiling, and see how much each of them affect overall decoding speed. You'll find that many don't even seem to have any effect if you only check with time, yet are still 2 to 4 times faster than their C counterparts. Do you want to remove them as well? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On Thu, 22 Jan 2015 16:43:16 -0300 James Almer wrote: > On 22/01/15 4:27 PM, wm4 wrote: > > Then I'd definitely vote for remove. > > > > The asm probably mattered on ancient CPUs and ancient compilers, but > > there's no reason to keep it anymore. > > No. If the handwritten asm is better than the C code, even if slightly, then > it should not be removed. > And if someone dislikes its inline asm nature then they are free to port it, > like i did with a couple other filters before. For such a small difference, your statement is ridiculous. No, really. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On 22/01/15 4:27 PM, wm4 wrote: > Then I'd definitely vote for remove. > > The asm probably mattered on ancient CPUs and ancient compilers, but > there's no reason to keep it anymore. No. If the handwritten asm is better than the C code, even if slightly, then it should not be removed. And if someone dislikes its inline asm nature then they are free to port it, like i did with a couple other filters before. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On Thu, 22 Jan 2015 21:45:04 +0530
arwa arif wrote:
> On Thu, Jan 22, 2015 at 2:59 PM, wm4 wrote:
>
> > On Thu, 22 Jan 2015 01:38:11 +0530
> > arwa arif wrote:
> >
> > > From 703cc1887903c2868537e19e99b76927bec07884 Mon Sep 17 00:00:00 2001
> > > From: Arwa Arif
> > > Date: Mon, 19 Jan 2015 03:56:48 +0530
> > > Subject: [PATCH] Port mp=eq/eq2 to FFmpeg
> > >
> > > Code adapted from James Darnley's previous commits
> >
> > > [...]
> >
> > > +#if HAVE_MMX && HAVE_6REGS
> > > +static void process_MMX(EQParameters *param, uint8_t *dst, int
> > dst_stride,
> > > +uint8_t *src, int src_stride, int w, int h)
> > > +{
> > > +int i;
> > > +int pel;
> > > +int dstep = dst_stride - w;
> > > +int sstep = src_stride - w;
> > > +short brvec[4];
> > > +short contvec[4];
> > > +
> > > +brvec[0] = brvec[1] = brvec[2] = brvec[3] = param->b;
> > > +contvec[0] = contvec[1] = contvec[2] = contvec[3] = param->c;
> > > +
> > > +while (h--) {
> > > +__asm__ volatile (
> > > +"movq (%5), %%mm3 \n\t"
> > > +"movq (%6), %%mm4 \n\t"
> > > +"pxor %%mm0, %%mm0 \n\t"
> > > +"movl %4, %%eax\n\t"
> > > +//ASMALIGN(4)
> > > +"1:\n\t"
> > > +"movq (%0), %%mm1 \n\t"
> > > +"movq (%0), %%mm2 \n\t"
> > > +"punpcklbw %%mm0, %%mm1\n\t"
> > > +"punpckhbw %%mm0, %%mm2\n\t"
> > > +"psllw $4, %%mm1 \n\t"
> > > +"psllw $4, %%mm2 \n\t"
> > > +"pmulhw %%mm4, %%mm1 \n\t"
> > > +"pmulhw %%mm4, %%mm2 \n\t"
> > > +"paddw %%mm3, %%mm1\n\t"
> > > +"paddw %%mm3, %%mm2\n\t"
> > > +"packuswb %%mm2, %%mm1 \n\t"
> > > +"add $8, %0\n\t"
> > > +"movq %%mm1, (%1) \n\t"
> > > +"add $8, %1\n\t"
> > > +"decl %%eax\n\t"
> > > +"jnz 1b\n\t"
> > > +: "=r" (src), "=r" (dst)
> > > +: "0" (src), "1" (dst), "r" (w>>3), "r"
> > (brvec), "r" (contvec)
> > > +: "%eax"
> > > +);
> > > +
> > > +for (i = w&7; i; i--) {
> > > +pel = ((*src++ * param->c) >> 12) + param->b;
> > > +if (pel & 768)
> > > +pel = (-pel) >> 31;
> > > +*dst++ = pel;
> > > +}
> > > +
> > > +src += sstep;
> > > +dst += dstep;
> > > +}
> > > +__asm__ volatile ( "emms \n\t" ::: "memory" );
> > > +}
> > > +#endif
> >
> > IMO the asm shouldn't be added at all. It's not important enough for a
> > ported video filter, but has high maintenance overhead - there's a
> > reason the FFmpeg loathes inline asm, right?
> >
> > How much does the inline asm help with speed? When I tested it on a
> > reasonably modern CPU a while ago, there was barely any advantage.
> >
> >
> I checked the runtime of the codes with and without asm, it turns out that
> there is not much difference. The difference is coming out to be in
> milliseconds.
>
> 26.014s with asm and
> 26.129 without asm.
>
> So, should I remove the asm part?
Then I'd definitely vote for remove.
The asm probably mattered on ancient CPUs and ancient compilers, but
there's no reason to keep it anymore.
>
> > > +av_cold void ff_eq_init_x86(EQContext *eq)
> > > +{
> > > +#if HAVE_MMX_INLINE
> > > +int cpu_flags = av_get_cpu_flags();
> > > +
> > > +if (cpu_flags & AV_CPU_FLAG_MMX) {
> > > +eq->process = process_MMX;
> > > +}
> > > +#endif
> > > +}
> >
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On 22/01/15 6:29 AM, wm4 wrote: > IMO the asm shouldn't be added at all. It's not important enough for a > ported video filter, but has high maintenance overhead - there's a > reason the FFmpeg loathes inline asm, right? Unless the asm is slower than the C code as compiled by recent versions of gcc, or its output not bitexact to the C version while at the same time looking worse, then i see no reason to remove it. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On Thu, Jan 22, 2015 at 2:59 PM, wm4 wrote:
> On Thu, 22 Jan 2015 01:38:11 +0530
> arwa arif wrote:
>
> > From 703cc1887903c2868537e19e99b76927bec07884 Mon Sep 17 00:00:00 2001
> > From: Arwa Arif
> > Date: Mon, 19 Jan 2015 03:56:48 +0530
> > Subject: [PATCH] Port mp=eq/eq2 to FFmpeg
> >
> > Code adapted from James Darnley's previous commits
>
> > [...]
>
> > +#if HAVE_MMX && HAVE_6REGS
> > +static void process_MMX(EQParameters *param, uint8_t *dst, int
> dst_stride,
> > +uint8_t *src, int src_stride, int w, int h)
> > +{
> > +int i;
> > +int pel;
> > +int dstep = dst_stride - w;
> > +int sstep = src_stride - w;
> > +short brvec[4];
> > +short contvec[4];
> > +
> > +brvec[0] = brvec[1] = brvec[2] = brvec[3] = param->b;
> > +contvec[0] = contvec[1] = contvec[2] = contvec[3] = param->c;
> > +
> > +while (h--) {
> > +__asm__ volatile (
> > +"movq (%5), %%mm3 \n\t"
> > +"movq (%6), %%mm4 \n\t"
> > +"pxor %%mm0, %%mm0 \n\t"
> > +"movl %4, %%eax\n\t"
> > +//ASMALIGN(4)
> > +"1:\n\t"
> > +"movq (%0), %%mm1 \n\t"
> > +"movq (%0), %%mm2 \n\t"
> > +"punpcklbw %%mm0, %%mm1\n\t"
> > +"punpckhbw %%mm0, %%mm2\n\t"
> > +"psllw $4, %%mm1 \n\t"
> > +"psllw $4, %%mm2 \n\t"
> > +"pmulhw %%mm4, %%mm1 \n\t"
> > +"pmulhw %%mm4, %%mm2 \n\t"
> > +"paddw %%mm3, %%mm1\n\t"
> > +"paddw %%mm3, %%mm2\n\t"
> > +"packuswb %%mm2, %%mm1 \n\t"
> > +"add $8, %0\n\t"
> > +"movq %%mm1, (%1) \n\t"
> > +"add $8, %1\n\t"
> > +"decl %%eax\n\t"
> > +"jnz 1b\n\t"
> > +: "=r" (src), "=r" (dst)
> > +: "0" (src), "1" (dst), "r" (w>>3), "r"
> (brvec), "r" (contvec)
> > +: "%eax"
> > +);
> > +
> > +for (i = w&7; i; i--) {
> > +pel = ((*src++ * param->c) >> 12) + param->b;
> > +if (pel & 768)
> > +pel = (-pel) >> 31;
> > +*dst++ = pel;
> > +}
> > +
> > +src += sstep;
> > +dst += dstep;
> > +}
> > +__asm__ volatile ( "emms \n\t" ::: "memory" );
> > +}
> > +#endif
>
> IMO the asm shouldn't be added at all. It's not important enough for a
> ported video filter, but has high maintenance overhead - there's a
> reason the FFmpeg loathes inline asm, right?
>
> How much does the inline asm help with speed? When I tested it on a
> reasonably modern CPU a while ago, there was barely any advantage.
>
>
I checked the runtime of the codes with and without asm, it turns out that
there is not much difference. The difference is coming out to be in
milliseconds.
26.014s with asm and
26.129 without asm.
So, should I remove the asm part?
> > +av_cold void ff_eq_init_x86(EQContext *eq)
> > +{
> > +#if HAVE_MMX_INLINE
> > +int cpu_flags = av_get_cpu_flags();
> > +
> > +if (cpu_flags & AV_CPU_FLAG_MMX) {
> > +eq->process = process_MMX;
> > +}
> > +#endif
> > +}
>
> ___
> 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
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On Thu, 22 Jan 2015 01:38:11 +0530
arwa arif wrote:
> From 703cc1887903c2868537e19e99b76927bec07884 Mon Sep 17 00:00:00 2001
> From: Arwa Arif
> Date: Mon, 19 Jan 2015 03:56:48 +0530
> Subject: [PATCH] Port mp=eq/eq2 to FFmpeg
>
> Code adapted from James Darnley's previous commits
> [...]
> +#if HAVE_MMX && HAVE_6REGS
> +static void process_MMX(EQParameters *param, uint8_t *dst, int dst_stride,
> +uint8_t *src, int src_stride, int w, int h)
> +{
> +int i;
> +int pel;
> +int dstep = dst_stride - w;
> +int sstep = src_stride - w;
> +short brvec[4];
> +short contvec[4];
> +
> +brvec[0] = brvec[1] = brvec[2] = brvec[3] = param->b;
> +contvec[0] = contvec[1] = contvec[2] = contvec[3] = param->c;
> +
> +while (h--) {
> +__asm__ volatile (
> +"movq (%5), %%mm3 \n\t"
> +"movq (%6), %%mm4 \n\t"
> +"pxor %%mm0, %%mm0 \n\t"
> +"movl %4, %%eax\n\t"
> +//ASMALIGN(4)
> +"1:\n\t"
> +"movq (%0), %%mm1 \n\t"
> +"movq (%0), %%mm2 \n\t"
> +"punpcklbw %%mm0, %%mm1\n\t"
> +"punpckhbw %%mm0, %%mm2\n\t"
> +"psllw $4, %%mm1 \n\t"
> +"psllw $4, %%mm2 \n\t"
> +"pmulhw %%mm4, %%mm1 \n\t"
> +"pmulhw %%mm4, %%mm2 \n\t"
> +"paddw %%mm3, %%mm1\n\t"
> +"paddw %%mm3, %%mm2\n\t"
> +"packuswb %%mm2, %%mm1 \n\t"
> +"add $8, %0\n\t"
> +"movq %%mm1, (%1) \n\t"
> +"add $8, %1\n\t"
> +"decl %%eax\n\t"
> +"jnz 1b\n\t"
> +: "=r" (src), "=r" (dst)
> +: "0" (src), "1" (dst), "r" (w>>3), "r" (brvec), "r"
> (contvec)
> +: "%eax"
> +);
> +
> +for (i = w&7; i; i--) {
> +pel = ((*src++ * param->c) >> 12) + param->b;
> +if (pel & 768)
> +pel = (-pel) >> 31;
> +*dst++ = pel;
> +}
> +
> +src += sstep;
> +dst += dstep;
> +}
> +__asm__ volatile ( "emms \n\t" ::: "memory" );
> +}
> +#endif
IMO the asm shouldn't be added at all. It's not important enough for a
ported video filter, but has high maintenance overhead - there's a
reason the FFmpeg loathes inline asm, right?
How much does the inline asm help with speed? When I tested it on a
reasonably modern CPU a while ago, there was barely any advantage.
> +av_cold void ff_eq_init_x86(EQContext *eq)
> +{
> +#if HAVE_MMX_INLINE
> +int cpu_flags = av_get_cpu_flags();
> +
> +if (cpu_flags & AV_CPU_FLAG_MMX) {
> +eq->process = process_MMX;
> +}
> +#endif
> +}
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On 21/01/15 5:08 PM, arwa arif wrote:
> diff --git a/libavfilter/x86/Makefile b/libavfilter/x86/Makefile
> index b93154e..8222e3f 100644
> --- a/libavfilter/x86/Makefile
> +++ b/libavfilter/x86/Makefile
> @@ -1,3 +1,4 @@
> +OBJS-$(CONFIG_EQ_FILTER) += x86/vf_eq.o
> OBJS-$(CONFIG_FSPP_FILTER) += x86/vf_fspp.o
> OBJS-$(CONFIG_GRADFUN_FILTER)+= x86/vf_gradfun_init.o
> OBJS-$(CONFIG_HQDN3D_FILTER) += x86/vf_hqdn3d_init.o
> diff --git a/libavfilter/x86/vf_eq.c b/libavfilter/x86/vf_eq.c
> new file mode 100644
> index 000..3396c33
> --- /dev/null
> +++ b/libavfilter/x86/vf_eq.c
> @@ -0,0 +1,94 @@
> +/*
> + *
> + * Original MPlayer filters by Richard Felker.
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with FFmpeg; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + */
> +
> +#include "libavutil/attributes.h"
> +#include "libavutil/cpu.h"
> +#include "libavutil/mem.h"
> +#include "libavutil/x86/asm.h"
> +#include "libavfilter/vf_eq.h"
> +
> +#if HAVE_MMX && HAVE_6REGS
Again, "HAVE_MMX_INLINE && HAVE_6REGS".
Otherwise, compilation will fail with compilers like msvc where HAVE_MMX is
true but
HAVE_MMX_INLINE isn't.
> +static void process_MMX(EQParameters *param, uint8_t *dst, int dst_stride,
> +uint8_t *src, int src_stride, int w, int h)
> +{
> +int i;
> +int pel;
> +int dstep = dst_stride - w;
> +int sstep = src_stride - w;
> +short brvec[4];
> +short contvec[4];
> +
> +brvec[0] = brvec[1] = brvec[2] = brvec[3] = param->b;
> +contvec[0] = contvec[1] = contvec[2] = contvec[3] = param->c;
> +
> +while (h--) {
> +__asm__ volatile (
> +"movq (%5), %%mm3 \n\t"
> +"movq (%6), %%mm4 \n\t"
> +"pxor %%mm0, %%mm0 \n\t"
> +"movl %4, %%eax\n\t"
> +//ASMALIGN(4)
".p2align 4 \n\t", or copy the define line for ASMALIGN() from
libavfilter/libmpcodecs/mp_image.h
to libavutil/x86/asm.h and uncomment this line.
> +"1:\n\t"
> +"movq (%0), %%mm1 \n\t"
> +"movq (%0), %%mm2 \n\t"
> +"punpcklbw %%mm0, %%mm1\n\t"
> +"punpckhbw %%mm0, %%mm2\n\t"
> +"psllw $4, %%mm1 \n\t"
> +"psllw $4, %%mm2 \n\t"
> +"pmulhw %%mm4, %%mm1 \n\t"
> +"pmulhw %%mm4, %%mm2 \n\t"
> +"paddw %%mm3, %%mm1\n\t"
> +"paddw %%mm3, %%mm2\n\t"
> +"packuswb %%mm2, %%mm1 \n\t"
> +"add $8, %0\n\t"
> +"movq %%mm1, (%1) \n\t"
> +"add $8, %1\n\t"
> +"decl %%eax\n\t"
> +"jnz 1b\n\t"
> +: "=r" (src), "=r" (dst)
> +: "0" (src), "1" (dst), "r" (w>>3), "r" (brvec), "r"
> (contvec)
> +: "%eax"
> +);
> +
> +for (i = w&7; i; i--) {
> +pel = ((*src++ * param->c) >> 12) + param->b;
> +if (pel & 768)
> +pel = (-pel) >> 31;
> +*dst++ = pel;
> +}
> +
> +src += sstep;
> +dst += dstep;
> +}
> +__asm__ volatile ( "emms \n\t" ::: "memory" );
> +}
> +#endif
> +
> +av_cold void ff_eq_init_x86(EQContext *eq)
> +{
> +#if HAVE_MMX_INLINE
Check for HAVE_6REGS here as well.
> +int cpu_flags = av_get_cpu_flags();
> +
> +if (cpu_flags & AV_CPU_FLAG_MMX) {
> +eq->process = process_MMX;
> +}
> +#endif
> +}
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On Thu, Jan 22, 2015 at 12:09 AM, Paul B Mahol wrote:
> On 1/21/15, arwa arif wrote:
> >>
> >> I still expect that eq and eq2 should have the same performances,
> >> since the adjust callback is set depending on the parameter values. So
> >> we should have a single eq filter.
> >>
> >> Please investigate about why you get different benchmark values.
> >>
> >
> > I used this command: ffmpeg -benchmark -i matrixbench_mpeg2.mpg -vf
> mp=eq2
> > -f null -
> > And everytime I am running this command, I am getting a different result
> > even for the same input and same filter.
> >
> > So, I tried using the time.h library for getting the time, the results
> for
> > eq and eq2 are 37.71 and 35.56 seconds respectively.
> >
> > I don't know why eq is coming out to be slower than eq2. (In fact, eq2 is
> > accessing more functions than eq).
>
> Just remove eq code and rename eq2 to eq.
>
>
Updated the patch.
> >
> >
> >> --
> >> FFmpeg = Fancy Fancy Multipurpose Pacific Elitist Game
> >> ___
> >> 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
> >
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
From 703cc1887903c2868537e19e99b76927bec07884 Mon Sep 17 00:00:00 2001
From: Arwa Arif
Date: Mon, 19 Jan 2015 03:56:48 +0530
Subject: [PATCH] Port mp=eq/eq2 to FFmpeg
Code adapted from James Darnley's previous commits
---
configure|1 +
doc/filters.texi | 44
libavfilter/Makefile |1 +
libavfilter/allfilters.c |1 +
libavfilter/vf_eq.c | 282 ++
libavfilter/vf_eq.h | 63 +++
libavfilter/x86/Makefile |1 +
libavfilter/x86/vf_eq.c | 94
8 files changed, 487 insertions(+)
create mode 100644 libavfilter/vf_eq.c
create mode 100644 libavfilter/vf_eq.h
create mode 100644 libavfilter/x86/vf_eq.c
diff --git a/configure b/configure
index c73562b..138852e 100755
--- a/configure
+++ b/configure
@@ -2579,6 +2579,7 @@ delogo_filter_deps="gpl"
deshake_filter_select="pixelutils"
drawtext_filter_deps="libfreetype"
ebur128_filter_deps="gpl"
+eq_filter_deps="gpl"
flite_filter_deps="libflite"
frei0r_filter_deps="frei0r dlopen"
frei0r_src_filter_deps="frei0r dlopen"
diff --git a/doc/filters.texi b/doc/filters.texi
index d7b2273..70e0557 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -4320,6 +4320,50 @@ edgedetect=mode=colormix:high=0
@end example
@end itemize
+@section eq
+Equalizer that uses lookup tables (very slow), allowing gamma correction
+in addition to simple brightness and contrast adjustment.
+
+The filter accepts the following options:
+
+@table @option
+@item brightness
+Set the brightness value. It accepts a float value in range @code{-1.0} to
+@code{1.0}. The default value is @code{0.0}.
+
+@item contrast
+Set the contrast value. It accepts a float value in range @code{-2.0} to
+@code{2.0}. The default value is @code{0.0}.
+
+@item gamma
+Set the gamma value. It accepts a float value in range @code{0.1} to @code{10.0}.
+The default value is @code{1.0}.
+
+@item gamma_y
+Set the gamma value for the luma plane. It accepts a float value in range
+@code{0.1} to @code{10.0}. The default value is @code{1.0}.
+
+@item gamma_u
+Set the gamma value for 1st chroma plane. It accepts a float value in range
+@code{0.1} to @code{10.0}. The default value is @code{1.0}.
+
+@item gamma_v
+Set the gamma value for 2nd chroma plane. It accepts a float value in range
+@code{0.1} to @code{10.0}. The default value is @code{1.0}.
+
+@item saturation
+Set the saturation value. It accepts a float value in range @code{0.0} to
+@code{3.0}. The default value is @code{1.0}.
+
+@item weight
+Can be used to reduce the effect of a high gamma value on bright image areas,
+e.g. keep them from getting overamplified and just plain white. It accepts a
+float value in range @code{0.0} to @code{1.0}.A value of @code{0.0} turns the
+gamma correction all the way down while @code{1.0} leaves it at its full strength.
+Default is @code{1.0}.
+
+@end table
+
@section extractplanes
Extract color channel components from input video stream into
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index e43d76d..8e94033 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -116,6 +116,7 @@ OBJS-$(CONFIG_DRAWGRID_FILTER) += vf_drawbox.o
OBJS-$(CONFIG_DRAWTEXT_FILTER) += vf_drawtext.o
OBJS-$(CONFIG_ELBG_FILTER) += vf_elbg.o
OBJS-$(CONFIG_EDGEDETECT_FILTER) += vf_edgedetect.o
+OBJS-$(CONFIG_EQ_FILTER) += vf_eq.o
OBJS-$(CONFIG_EXTRACTPLANES_FILTER)
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On 1/21/15, arwa arif wrote: >> >> I still expect that eq and eq2 should have the same performances, >> since the adjust callback is set depending on the parameter values. So >> we should have a single eq filter. >> >> Please investigate about why you get different benchmark values. >> > > I used this command: ffmpeg -benchmark -i matrixbench_mpeg2.mpg -vf mp=eq2 > -f null - > And everytime I am running this command, I am getting a different result > even for the same input and same filter. > > So, I tried using the time.h library for getting the time, the results for > eq and eq2 are 37.71 and 35.56 seconds respectively. > > I don't know why eq is coming out to be slower than eq2. (In fact, eq2 is > accessing more functions than eq). Just remove eq code and rename eq2 to eq. > > >> -- >> FFmpeg = Fancy Fancy Multipurpose Pacific Elitist Game >> ___ >> 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 > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
> > I still expect that eq and eq2 should have the same performances, > since the adjust callback is set depending on the parameter values. So > we should have a single eq filter. > > Please investigate about why you get different benchmark values. > I used this command: ffmpeg -benchmark -i matrixbench_mpeg2.mpg -vf mp=eq2 -f null - And everytime I am running this command, I am getting a different result even for the same input and same filter. So, I tried using the time.h library for getting the time, the results for eq and eq2 are 37.71 and 35.56 seconds respectively. I don't know why eq is coming out to be slower than eq2. (In fact, eq2 is accessing more functions than eq). > -- > FFmpeg = Fancy Fancy Multipurpose Pacific Elitist Game > ___ > 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
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On date Tuesday 2015-01-20 22:30:54 +0530, Arwa Arif encoded:
[...]
> > Add also an entry to add support to .process_command(). You can
> > implement it in a later patch.
> >
> >
> What is meant by adding support to process_command?
grep for .process_command in the libavfilter dir, and you will know.
eq is meant to be used interactively, that's why having a
process_command callback is particuarly useful in this case.
[...]
> > > +static void set_saturation(EQ2Context *eq2)
> > > +{
> > > +int i;
> > > +/* saturation already set as AVOpt */
> > > +
> > > +for (i = 1; i < 3; i++) {
> > > +eq2->param[i].contrast = eq2->saturation;
> > > +eq2->param[i].lut_clean = 0;
> > > +check_values(&eq2->param[i]);
> > > +}
> >
> > Is this really working with gray8 or crashing (like mp=eq does)?
> >
>
> Yes, I checked the formats, it is working with gray8.
>
> > You should store in the context the number of planes.
> >
> > Also, please add a fate test.
> >
>
> I am not able to run rsync, maybe because of proxy settings. I tried
> various ways of bypassing the proxy, but I am still unable to use it. Is
> there any other way of adding the fate test?
You don't need to have the complete fate repository to run the
test. Give me some time and try to come with a patch (it's been a
while I'm not hacking in tests, and things moved).
> Updated the patch.
> From b54fd33b1d4ad68487ce20480c7865ad95ac19d8 Mon Sep 17 00:00:00 2001
> From: Arwa Arif
> Date: Mon, 19 Jan 2015 03:56:48 +0530
> Subject: [PATCH] Port mp=eq/eq2 to FFmpeg Code
> adapted from James Darnley's
> previous commits
This can stay in a separate line.
>
> ---
> configure|2 +
> doc/filters.texi | 65 ++
> libavfilter/Makefile |2 +
> libavfilter/allfilters.c |2 +
> libavfilter/vf_eq.c | 325
> ++
> libavfilter/vf_eq.h | 63 +
> libavfilter/x86/Makefile |1 +
> libavfilter/x86/vf_eq.c | 94 ++
> 8 files changed, 554 insertions(+)
> create mode 100644 libavfilter/vf_eq.c
> create mode 100644 libavfilter/vf_eq.h
> create mode 100644 libavfilter/x86/vf_eq.c
>
> diff --git a/configure b/configure
> index c73562b..a8042b2 100755
> --- a/configure
> +++ b/configure
> @@ -2579,6 +2579,8 @@ delogo_filter_deps="gpl"
> deshake_filter_select="pixelutils"
> drawtext_filter_deps="libfreetype"
> ebur128_filter_deps="gpl"
> +eq_filter_deps="gpl"
> +eq2_filter_deps="gpl"
> flite_filter_deps="libflite"
> frei0r_filter_deps="frei0r dlopen"
> frei0r_src_filter_deps="frei0r dlopen"
> diff --git a/doc/filters.texi b/doc/filters.texi
> index d7b2273..ded154a 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -4320,6 +4320,71 @@ edgedetect=mode=colormix:high=0
> @end example
> @end itemize
>
> +@anchor{eq}
> +@section eq
> +Control brightness and contrast. It can be used for fixing poorly captured
> movies,
> +or for slightly reducing contrast to mask artifacts and get by with lower
> bitrates.
> +
> +The filter accepts the following options:
> +
> +@table @option
> +
> +@item brightness
> +Set the brightness value. It accepts a float value in range @code{-1.0} to
> +@code{1.0}. The default value is @code{0.0}.
> +
> +@item contrast
> +Set the contrast value. It accepts a float value in range @code{-1.0} to
> +@code{1.0}. The default value is @code{0.0}.
> +@end table
> +
> +@section eq2
> +Equalizer that uses lookup tables (very slow), allowing gamma correction
> +in addition to simple brightness and contrast adjustment.
> +
> +Note that it uses the same optimized code as @ref{eq} if all gamma values
> +are 1.0. The parameters are given as floating point values.
> +
> +The filter accepts the following options:
> +
> +@table @option
> +@item brightness
> +Set the brightness value. It accepts a float value in range @code{-1.0} to
> +@code{1.0}. The default value is @code{0.0}.
> +
> +@item contrast
> +Set the contrast value. It accepts a float value in range @code{-2.0} to
> +@code{2.0}. The default value is @code{0.0}.
> +
> +@item gamma
> +Set the gamma value. It accepts a float value in range @code{0.1} to
> @code{10.0}.
> +The default value is @code{1.0}.
> +
> +@item gamma_y
> +Set the gamma value for the luma plane. It accepts a float value in range
> +@code{0.1} to @code{10.0}. The default value is @code{1.0}.
> +
> +@item gamma_u
> +Set the gamma value for 1st chroma plane. It accepts a float value in range
> +@code{0.1} to @code{10.0}. The default value is @code{1.0}.
> +
> +@item gamma_v
> +Set the gamma value for 2nd chroma plane. It accepts a float value in range
> +@code{0.1} to @code{10.0}. The default value is @code{1.0}.
> +
> +@item saturation
> +Set the saturation value. It accepts a float value in range @code{0.0} to
> +@code{3.0}. The default value is @code{1.0}.
> +
> +@item weight
> +Can be used to reduce the effect of a high gamma value on bright image
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
>
> > @@ -0,0 +1,342 @@
>
> +/*
> > + * Original MPlayer filters by Richard Felker, Hampa Hug, Daniel Moreno,
> > + * and Michael Niedermeyer.
> > + *
> > + * Copyright (c) 2014 James Darnley
> > + * Copyright (c) 2015 Arwa Arif
> > + *
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> along
> > + * with FFmpeg; if not, write to the Free Software Foundation, Inc.,
> > + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> > + */
> > +
> > +/**
> > + * @file
> > + * very simple video equalizer
> > + */
> > +
> > +/* TODO:
>
> > + * - copy plane pointers rather than data
>
> uh? drop this comment unless it is clear to you what it means
>
> > + * - support alpha channels
>
> Add also an entry to add support to .process_command(). You can
> implement it in a later patch.
>
>
What is meant by adding support to process_command?
>
>
> > +
> > +eq2->param[0].contrast = eq2->contrast;
> > +eq2->param[0].lut_clean = 0;
> > +check_values(&eq2->param[0]);
> > +}
> > +
> > +static void set_brightness(EQ2Context *eq2)
> > +{
> > +/* brightness already set as AVOpt */
> > +
> > +eq2->param[0].brightness = eq2->brightness;
> > +eq2->param[0].lut_clean = 0;
> > +check_values(&eq2->param[0]);
> > +}
> > +
> > +static void set_gamma(EQ2Context *eq2)
> > +{
> > +int i;
> > +/* gamma already set as AVOpt */
> > +
> > +eq2->param[0].gamma = eq2->gamma * eq2->gamma_g;
> > +eq2->param[1].gamma = sqrt(eq2->gamma_b / eq2->gamma_g);
> > +eq2->param[2].gamma = sqrt(eq2->gamma_r / eq2->gamma_g);
> > +
> > +for (i = 0; i < 3; i++) {
> > +eq2->param[i].weight = eq2->weight;
> > +eq2->param[i].lut_clean = 0;
> > +check_values(&eq2->param[i]);
> > +}
> > +}
> > +
> > +static void set_saturation(EQ2Context *eq2)
> > +{
> > +int i;
> > +/* saturation already set as AVOpt */
> > +
> > +for (i = 1; i < 3; i++) {
> > +eq2->param[i].contrast = eq2->saturation;
> > +eq2->param[i].lut_clean = 0;
> > +check_values(&eq2->param[i]);
> > +}
>
> Is this really working with gray8 or crashing (like mp=eq does)?
>
Yes, I checked the formats, it is working with gray8.
> You should store in the context the number of planes.
>
> Also, please add a fate test.
>
I am not able to run rsync, maybe because of proxy settings. I tried
various ways of bypassing the proxy, but I am still unable to use it. Is
there any other way of adding the fate test?
Updated the patch.
From b54fd33b1d4ad68487ce20480c7865ad95ac19d8 Mon Sep 17 00:00:00 2001
From: Arwa Arif
Date: Mon, 19 Jan 2015 03:56:48 +0530
Subject: [PATCH] Port mp=eq/eq2 to FFmpeg Code adapted from James Darnley's
previous commits
---
configure|2 +
doc/filters.texi | 65 ++
libavfilter/Makefile |2 +
libavfilter/allfilters.c |2 +
libavfilter/vf_eq.c | 325 ++
libavfilter/vf_eq.h | 63 +
libavfilter/x86/Makefile |1 +
libavfilter/x86/vf_eq.c | 94 ++
8 files changed, 554 insertions(+)
create mode 100644 libavfilter/vf_eq.c
create mode 100644 libavfilter/vf_eq.h
create mode 100644 libavfilter/x86/vf_eq.c
diff --git a/configure b/configure
index c73562b..a8042b2 100755
--- a/configure
+++ b/configure
@@ -2579,6 +2579,8 @@ delogo_filter_deps="gpl"
deshake_filter_select="pixelutils"
drawtext_filter_deps="libfreetype"
ebur128_filter_deps="gpl"
+eq_filter_deps="gpl"
+eq2_filter_deps="gpl"
flite_filter_deps="libflite"
frei0r_filter_deps="frei0r dlopen"
frei0r_src_filter_deps="frei0r dlopen"
diff --git a/doc/filters.texi b/doc/filters.texi
index d7b2273..ded154a 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -4320,6 +4320,71 @@ edgedetect=mode=colormix:high=0
@end example
@end itemize
+@anchor{eq}
+@section eq
+Control brightness and contrast. It can be used for fixing poorly captured movies,
+or for slightly reducing contrast to mask artifacts and get by with lower bitrates.
+
+The filter accepts the following options:
+
+@table @option
+
+@item brightness
+Set the brightness value. It accepts a float value in range @code{-1.0} to
+@code{1.0}. The default value is @code{0.0}.
+
+@item contrast
+Set the contrast value. It accepts a float value in range @code{-1.0} to
+@code{1.0}. The default value is @code{0.0}.
+@end tab
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On 1/19/15, arwa arif wrote: > On Mon, Jan 19, 2015 at 8:53 PM, Stefano Sabatini > wrote: > >> On date Monday 2015-01-19 15:20:54 +0100, Clement Boesch encoded: >> > On Mon, Jan 19, 2015 at 02:09:33PM +, Paul B Mahol wrote: >> > > On 1/18/15, arwa arif wrote: >> > > > Attached the patch. >> > > > >> > > >> > > I'm for dropping eq code and rename eq2 to eq. >> > >> > Yes please let's not add 2 filters for this, it's insane. Also, "eq" is >> > quite a bad name, but well... >> > >> >> > What happened to the idea of having the feature in hue instead? >> >> I'm not against that if we agree it's a better path. >> >> About eq/eq2, are there really performance concerns for having both of >> them? >> >> Arwa, can you show some benchmarks? >> > > The benchmark result for a demo video for > > 1.) eq filter: > > frame= 4690 fps=120 q=31.0 Lsize= 16788kB time=00:03:07.64 bitrate= > 732.9kbits/s > video:7828kB audio:8796kB subtitle:0kB other streams:0kB global headers:0kB > muxing overhead: 0.983091% > bench: utime=45.871s > bench: maxrss=19420kB > > > 2.) eq2 filter: > > frame= 4690 fps=110 q=31.0 Lsize= 16788kB time=00:03:07.64 bitrate= > 732.9kbits/s > video:7828kB audio:8796kB subtitle:0kB other streams:0kB global headers:0kB > muxing overhead: 0.983091% > bench: utime=51.475s > bench: maxrss=19920kB That is strange considering they share same code. > > >> -- >> FFmpeg = Fiendish Free Majestic Philosophical Ermetic Gem >> ___ >> 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 > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On Mon, Jan 19, 2015 at 8:53 PM, Stefano Sabatini wrote: > On date Monday 2015-01-19 15:20:54 +0100, Clément Bœsch encoded: > > On Mon, Jan 19, 2015 at 02:09:33PM +, Paul B Mahol wrote: > > > On 1/18/15, arwa arif wrote: > > > > Attached the patch. > > > > > > > > > > I'm for dropping eq code and rename eq2 to eq. > > > > Yes please let's not add 2 filters for this, it's insane. Also, "eq" is > > quite a bad name, but well... > > > > > What happened to the idea of having the feature in hue instead? > > I'm not against that if we agree it's a better path. > > About eq/eq2, are there really performance concerns for having both of > them? > > Arwa, can you show some benchmarks? > The benchmark result for a demo video for 1.) eq filter: frame= 4690 fps=120 q=31.0 Lsize= 16788kB time=00:03:07.64 bitrate= 732.9kbits/s video:7828kB audio:8796kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: 0.983091% bench: utime=45.871s bench: maxrss=19420kB 2.) eq2 filter: frame= 4690 fps=110 q=31.0 Lsize= 16788kB time=00:03:07.64 bitrate= 732.9kbits/s video:7828kB audio:8796kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: 0.983091% bench: utime=51.475s bench: maxrss=19920kB > -- > FFmpeg = Fiendish Free Majestic Philosophical Ermetic Gem > ___ > 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
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On Mon, 19 Jan 2015 16:23:46 +0100 Stefano Sabatini wrote: > On date Monday 2015-01-19 15:20:54 +0100, Clément Bœsch encoded: > > On Mon, Jan 19, 2015 at 02:09:33PM +, Paul B Mahol wrote: > > > On 1/18/15, arwa arif wrote: > > > > Attached the patch. > > > > > > > > > > I'm for dropping eq code and rename eq2 to eq. > > > > Yes please let's not add 2 filters for this, it's insane. Also, "eq" is > > quite a bad name, but well... > > > > > What happened to the idea of having the feature in hue instead? > > I'm not against that if we agree it's a better path. > > About eq/eq2, are there really performance concerns for having both of > them? > > Arwa, can you show some benchmarks? Why are you questioning this again? vf_eq2 is vf_eq copy&pasted, with exactly the same asm, and an additional _optional_ code path using a LUT. This extra code path is not taken when using it like vf_eq. (Did you think mplayer development made sense?) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On date Monday 2015-01-19 15:20:54 +0100, Clément Bœsch encoded: > On Mon, Jan 19, 2015 at 02:09:33PM +, Paul B Mahol wrote: > > On 1/18/15, arwa arif wrote: > > > Attached the patch. > > > > > > > I'm for dropping eq code and rename eq2 to eq. > > Yes please let's not add 2 filters for this, it's insane. Also, "eq" is > quite a bad name, but well... > > What happened to the idea of having the feature in hue instead? I'm not against that if we agree it's a better path. About eq/eq2, are there really performance concerns for having both of them? Arwa, can you show some benchmarks? -- FFmpeg = Fiendish Free Majestic Philosophical Ermetic Gem ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On Mon, Jan 19, 2015 at 02:09:33PM +, Paul B Mahol wrote: > On 1/18/15, arwa arif wrote: > > Attached the patch. > > > > I'm for dropping eq code and rename eq2 to eq. Yes please let's not add 2 filters for this, it's insane. Also, "eq" is quite a bad name, but well... What happened to the idea of having the feature in hue instead? -- Clément B. pgpku0gdw4Kcu.pgp Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On 1/18/15, arwa arif wrote: > Attached the patch. > I'm for dropping eq code and rename eq2 to eq. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On date Monday 2015-01-19 04:04:45 +0530, Arwa Arif encoded:
> Attached the patch.
> From 79298b4f6d08abacb387dbd3f75fabe329d96772 Mon Sep 17 00:00:00 2001
> From: Arwa Arif
> Date: Mon, 19 Jan 2015 03:56:48 +0530
> Subject: [PATCH] Port mp=eq/eq2 to FFmpeg
Mention James Darnley in the log message. Something like: this code is
adapted from a port by James Darnley.
>
> ---
> configure|2 +
> doc/filters.texi | 68 +
> libavfilter/Makefile |2 +
> libavfilter/allfilters.c |2 +
> libavfilter/vf_eq.c | 342
> ++
> libavfilter/vf_eq.h | 63 +
> libavfilter/x86/Makefile |1 +
> libavfilter/x86/vf_eq.c | 94 +
> 8 files changed, 574 insertions(+)
> create mode 100644 libavfilter/vf_eq.c
> create mode 100644 libavfilter/vf_eq.h
> create mode 100644 libavfilter/x86/vf_eq.c
>
> diff --git a/configure b/configure
> index c73562b..a8042b2 100755
> --- a/configure
> +++ b/configure
> @@ -2579,6 +2579,8 @@ delogo_filter_deps="gpl"
> deshake_filter_select="pixelutils"
> drawtext_filter_deps="libfreetype"
> ebur128_filter_deps="gpl"
> +eq_filter_deps="gpl"
> +eq2_filter_deps="gpl"
> flite_filter_deps="libflite"
> frei0r_filter_deps="frei0r dlopen"
> frei0r_src_filter_deps="frei0r dlopen"
> diff --git a/doc/filters.texi b/doc/filters.texi
> index d7b2273..5eff0b5 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -4320,6 +4320,74 @@ edgedetect=mode=colormix:high=0
> @end example
> @end itemize
>
> +@anchor{eq}
> +@section eq
> +Software equalizer with interactive controls just like the hardware
> equalizer,
> +for cards/drivers that do not support brightness and contrast controls in
> hardware.
Drop the redundant reference to software/hardware (this is obviously a
software equalizer). Something like:
Control brightness and contrast.
should be enough.
> +
> +Might also be useful with MEncoder, either for fixing poorly captured
> movies, or
> +for slightly reducing contrast to mask artifacts and get by with lower
> bitrates.
Drop the reference to MEncoder.
> +
> +The filter accepts the following options:
> +
> +@table @option
> +
> +@item brightness
> +Set the brightness value. It accepts a float value in range @code{-1.0} to
> +@code{1.0}. The default value is @code{0.0}.
> +
> +@item contrast
> +Set the contrast value. It accepts a float value in range @code{-1.0} to
> +@code{1.0}. The default value is @code{0.0}.
> +@end table
> +
> +@section eq2
> +Alternative software equalizer that uses lookup tables (very slow), allowing
> +gamma correction in addition to simple brightness and contrast adjustment.
> +
> +Note that it uses the same MMX optimized code as @ref{eq} if all gamma values
Note that it uses the same optimizied code ...
> +are 1.0. The parameters are given as floating point values.
I wonder if it still makes sense to get two distinct filters. Can you
show benhmarks?
> +
> +The filter accepts the following options:
> +
> +@table @option
> +@item brightness
> +Set the brightness value. It accepts a float value in range @code{-1.0} to
> +@code{1.0}. The default value is @code{0.0}.
> +
> +@item contrast
> +Set the contrast value. It accepts a float value in range @code{-2.0} to
> +@code{2.0}. The default value is @code{0.0}.
> +
> +@item gamma
> +Set the gamma value. It accepts a float value in range @code{0.1} to
> @code{10.0}.
> +The default value is @code{1.0}.
> +
> +@item gamma_b
> +Set the gamma value for blue component. It accepts a float value in range
> +@code{0.1} to @code{10.0}. The default value is @code{1.0}.
> +
> +@item gamma_g
> +Set the gamma value for green component. It accepts a float value in range
> +@code{0.1} to @code{10.0}. The default value is @code{1.0}.
> +
> +@item gamma_r
> +Set the gamma value for red component. It accepts a float value in range
> +@code{0.1} to @code{10.0}. The default value is @code{1.0}.
> +
> +@item saturation
> +Set the saturation value. It accepts a float value in range @code{0.0} to
> +@code{3.0}. The default value is @code{1.0}.
> +
> +@item weight
> +Can be used to reduce the effect of a high gamma value on bright image areas,
> +e.g. keep them from getting overamplified and just plain white. It accepts a
> +float value in range @code{0.0} to @code{1.0}.A value of @code{0.0} turns the
> +gamma correction all the way down while @code{1.0} leaves it at its full
> strength.
> +Default is @code{1.0}.
> +
> +@end table
> +
> @section extractplanes
>
> Extract color channel components from input video stream into
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index e43d76d..8dab587 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -116,6 +116,8 @@ OBJS-$(CONFIG_DRAWGRID_FILTER) +=
> vf_drawbox.o
> OBJS-$(CONFIG_DRAWTEXT_FILTER) += vf_drawtext.o
> OBJS-$(CONFIG_ELBG_FILTER) += vf_elbg.o
> OBJS-$(C
Re: [FFmpeg-devel] [PATCH] Port mp=eq/eq2 to FFmpeg
On 18/01/15 7:34 PM, arwa arif wrote:
> +static int initialize(AVFilterContext *ctx)
> +{
> +EQ2Context *eq2 = ctx->priv;
> +int i;
> +
> +set_gamma(eq2);
> +set_contrast(eq2);
> +set_brightness(eq2);
> +set_saturation(eq2);
> +
> +if (IS_FILTER_EQ(ctx->filter)) {
> +eq2->param[0].contrast += 1.0;
> +eq2->param[1].adjust = NULL;
> +eq2->param[2].adjust = NULL;
> +}
> +
> +for (i = 0; i < 3; i++) {
> +eq2->param[i].c = (eq2->param[i].contrast) * 65536.0;
> +eq2->param[i].b = (eq2->param[i].brightness + 1.0) * 255.5 - 128.0 -
> (eq2->param[i].contrast) * 128.0;
> +}
> +
> +eq2->param->process = process_c;
This doesn't look right.
> +
> +if (ARCH_X86)
> +{
> +ff_eq_init_x86;
> +}
if (ARCH_X86)
ff_eq_init_x86(eq2);
> +
> +return 0;
> +}
[...]
> diff --git a/libavfilter/x86/vf_eq.c b/libavfilter/x86/vf_eq.c
> new file mode 100644
> index 000..ac693bb
> --- /dev/null
> +++ b/libavfilter/x86/vf_eq.c
> @@ -0,0 +1,94 @@
> +/*
> + *
> + * Original MPlayer filters by Richard Felker.
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with FFmpeg; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + */
> +
> +#include "libavutil/attributes.h"
> +#include "libavutil/cpu.h"
> +#include "libavutil/mem.h"
> +#include "libavutil/x86/asm.h"
> +#include "libavfilter/vf_eq.h"
> +
> +#if HAVE_MMX && HAVE_6REGS
#if HAVE_MMX_INLINE && HAVE_6REGS
> +static void process_MMX(EQ2Parameters *param, uint8_t *dst, int dst_stride,
> +uint8_t *src, int src_stride, int w, int h)
> +{
> +int i;
> +int pel;
> +int dstep = dst_stride - w;
> +int sstep = src_stride - w;
> +short brvec[4];
> +short contvec[4];
> +
> +brvec[0] = brvec[1] = brvec[2] = brvec[3] = param->b;
> +contvec[0] = contvec[1] = contvec[2] = contvec[3] = param->c;
> +
> +while (h--) {
> +__asm__ volatile (
> +"movq (%5), %%mm3 \n\t"
> +"movq (%6), %%mm4 \n\t"
> +"pxor %%mm0, %%mm0 \n\t"
> +"movl %4, %%eax\n\t"
> +//ASMALIGN(4)
Why is this commented out? If the ASMALIGN define isn't available here, then
use
".p2align 4 \n\t"
> +"1:\n\t"
> +"movq (%0), %%mm1 \n\t"
> +"movq (%0), %%mm2 \n\t"
> +"punpcklbw %%mm0, %%mm1\n\t"
> +"punpckhbw %%mm0, %%mm2\n\t"
> +"psllw $4, %%mm1 \n\t"
> +"psllw $4, %%mm2 \n\t"
> +"pmulhw %%mm4, %%mm1 \n\t"
> +"pmulhw %%mm4, %%mm2 \n\t"
> +"paddw %%mm3, %%mm1\n\t"
> +"paddw %%mm3, %%mm2\n\t"
> +"packuswb %%mm2, %%mm1 \n\t"
> +"add $8, %0\n\t"
> +"movq %%mm1, (%1) \n\t"
> +"add $8, %1\n\t"
> +"decl %%eax\n\t"
> +"jnz 1b\n\t"
> +: "=r" (src), "=r" (dst)
> +: "0" (src), "1" (dst), "r" (w>>3), "r" (brvec), "r"
> (contvec)
> +: "%eax"
> +);
> +
> +for (i = w&7; i; i--) {
> +pel = ((*src++ * param->c) >> 12) + param->b;
> +if (pel & 768)
> +pel = (-pel) >> 31;
> +*dst++ = pel;
> +}
> +
> +src += sstep;
> +dst += dstep;
> +}
> +__asm__ volatile ( "emms \n\t" ::: "memory" );
> +}
> +#endif
> +
> +av_cold void ff_eq_init_x86(EQ2Context *eq2)
> +{
> +#if HAVE_MMX_INLINE
Check for HAVE_6REGS here as well.
> +int cpu_flags = av_get_cpu_flags();
> +
> +if (cpu_flags & AV_CPU_FLAG_MMX) {
> +eq2->param->process = process_MMX;
Again, this looks wrong.
> +}
> +#endif
> +}
___
ffmpeg-devel mai
