[FFmpeg-devel] [PATCH] lavfi: port MP noise filter
Clément Bœsch
ubitux at gmail.com
Thu Feb 14 22:09:36 CET 2013
On Thu, Feb 14, 2013 at 11:40:27AM +0000, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol <onemda at gmail.com>
> ---
> configure | 1 +
> doc/filters.texi | 52 ++++++-
> libavfilter/Makefile | 1 +
> libavfilter/allfilters.c | 1 +
> libavfilter/vf_noise.c | 355 +++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 403 insertions(+), 7 deletions(-)
> create mode 100644 libavfilter/vf_noise.c
>
> diff --git a/configure b/configure
> index 4a8d85f..0033e13 100755
> --- a/configure
> +++ b/configure
> @@ -2012,6 +2012,7 @@ movie_filter_deps="avcodec avformat"
> mp_filter_deps="gpl avcodec swscale inline_asm"
> mptestsrc_filter_deps="gpl"
> negate_filter_deps="lut_filter"
> +noise_filter_deps="gpl"
LICENSE update required as well.
> resample_filter_deps="avresample"
> ocv_filter_deps="libopencv"
> pan_filter_deps="swresample"
> diff --git a/doc/filters.texi b/doc/filters.texi
> index ebb9ffa..efd5ecb 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -3631,7 +3631,6 @@ The list of the currently supported filters follows:
> @item ivtc
> @item kerndeint
> @item mcdeint
> - at item noise
> @item ow
> @item perspective
> @item phase
> @@ -3659,12 +3658,6 @@ Adjust gamma, brightness, contrast:
> @example
> mp=eq2=1.0:2:0.5
> @end example
> -
> - at item
> -Add temporal noise to input video:
> - at example
> -mp=noise=20t
> - at end example
> @end itemize
>
> See also mplayer(1), @url{http://www.mplayerhq.hu/}.
> @@ -3694,6 +3687,51 @@ noformat=yuv420p,vflip
> noformat=yuv420p:yuv444p:yuv410p
> @end example
>
> + at section noise
> +
> +Add noise on video input frame.
> +
> +This filter accepts a list of options in the form of @var{key}=@var{value}
> +pairs separated by ":". A description of the accepted options follows.
> +
> + at table @option
> + at item all_seed
> + at item c0_seed
> + at item c1_seed
> + at item c2_seed
> + at item c3_seed
> +Set noise seed for pixel component or all of them for @var{all_seed}.
> +Default value is @code{1234567}.
> +
> + at item all_strength, as
> + at item c0_strength, c0s
> + at item c1_strength, c1s
> + at item c2_strength, c2s
> + at item c3_strength, c3s
> +Set noise strength for pixel component or all of them for @var{all_strength}.
> +Default value is @code{0}. Allowed range is [0, 100].
> +
> + at item all_flags, af
> + at item c0_flags, c0f
> + at item c1_flags, c1f
> + at item c2_flags, c2f
> + at item c3_flags, c3f
> +Set pixel component flags or set flags for all components if @var{all_flags}.
> +Available values for component flags are:
> + at table @samp
> + at item a
> +averaged temporal noise (smoother)
> + at item p
> +mix random noise with a (semi)regular pattern
> + at item q
> +higher quality (slightly better looking, slightly slower)
> + at item t
> +temporal noise (noise pattern changes between frames)
> + at item u
> +uniform noise (gaussian otherwise)
> + at end table
> + at end table
> +
An example would be welcome, especially since you removed the only one
available.
> diff --git a/libavfilter/vf_noise.c b/libavfilter/vf_noise.c
[...]
> +typedef struct {
> + int strength;
> + int flags;
might make sense to have it unsigned and eventually explicit size
[...]
> +#define NOISE_PARAMS(name, x, param) \
> + {#name"_seed", "set component #"#x" noise seed", OFFSET(param.seed), AV_OPT_TYPE_INT, {.i64=1234567}, 0, INT_MAX, FLAGS}, \
> + {#name"_strength", "set component #"#x" strength", OFFSET(param.strength), AV_OPT_TYPE_INT, {.i64=0}, 0, 100, FLAGS}, \
> + {#name"s", "set component #"#x" strength", OFFSET(param.strength), AV_OPT_TYPE_INT, {.i64=0}, 0, 100, FLAGS}, \
> + {#name"_flags", "set component #"#x" flags", OFFSET(param.flags), AV_OPT_TYPE_FLAGS, {.i64=0}, 0, 31, FLAGS, #name"_flags"}, \
> + {#name"f", "set component #"#x" flags", OFFSET(param.flags), AV_OPT_TYPE_FLAGS, {.i64=0}, 0, 31, FLAGS, #name"_flags"}, \
> + {"a", "averaged noise", 0, AV_OPT_TYPE_CONST, {.i64=NOISE_AVERAGED}, 0, 0, FLAGS, #name"_flags"}, \
> + {"p", "(semi)regular pattern", 0, AV_OPT_TYPE_CONST, {.i64=NOISE_PATTERN}, 0, 0, FLAGS, #name"_flags"}, \
> + {"q", "high quality", 0, AV_OPT_TYPE_CONST, {.i64=NOISE_QUALITY}, 0, 0, FLAGS, #name"_flags"}, \
> + {"t", "temporal noise", 0, AV_OPT_TYPE_CONST, {.i64=NOISE_TEMPORAL}, 0, 0, FLAGS, #name"_flags"}, \
> + {"u", "uniform noise", 0, AV_OPT_TYPE_CONST, {.i64=NOISE_UNIFORM}, 0, 0, FLAGS, #name"_flags"},
Trailing whitespaces
[...]
> + for (i = 0, j = 0; i < MAX_NOISE; i++,j++) {
> + if (flags & NOISE_UNIFORM) {
> + if (flags & NOISE_AVERAGED) {
> + if (flags & NOISE_PATTERN) {
> + noise[i] = (RAND_N(strength) - strength/2)/6
> + +patt[j%4]*strength*0.25/3;
> + } else {
> + noise[i]= (RAND_N(strength) - strength/2)/3;
> + }
> + } else {
> + if (flags & NOISE_PATTERN) {
> + noise[i]= (RAND_N(strength) - strength/2)/2
> + + patt[j%4]*strength*0.25;
> + } else {
> + noise[i]= RAND_N(strength) - strength/2;
> + }
> + }
> + } else {
> + double x1, x2, w, y1;
> + do {
> + x1 = 2.0 * rand()/(float)RAND_MAX - 1.0;
> + x2 = 2.0 * rand()/(float)RAND_MAX - 1.0;
> + w = x1 * x1 + x2 * x2;
> + } while ( w >= 1.0 );
> +
> + w = sqrt( (-2.0 * log( w ) ) / w );
> + y1= x1 * w;
> + y1*= strength / sqrt(3.0);
> + if (flags & NOISE_PATTERN) {
> + y1 /= 2;
> + y1 += patt[j%4]*strength*0.35;
> + }
> + if (y1 < -128)
> + y1 = -128;
> + else if (y1 > 127)
> + y1 = 127;
av_clipf?
> + if (flags & NOISE_AVERAGED)
> + y1 /= 3.0;
> + noise[i]= (int)y1;
> + }
> + if (RAND_N(6) == 0)
> + j--;
> + }
> +
nit: you have miscellaneous style issues in this code block ('=' positions
and other weird spaces after functions etc) that you could fix.
[...]
> +static int filter_frame(AVFilterLink *inlink, AVFilterBufferRef *inpicref)
> +{
> + NoiseContext *n = inlink->dst->priv;
> + AVFilterLink *outlink = inlink->dst->outputs[0];
> + AVFilterBufferRef *out;
> + int ret, i;
> +
> + if (inpicref->perms & AV_PERM_WRITE) {
> + out = inpicref;
> + } else {
> + out = ff_get_video_buffer(outlink, AV_PERM_WRITE, outlink->w, outlink->h);
> + if (!out) {
> + avfilter_unref_bufferp(&inpicref);
> + return AVERROR(ENOMEM);
> + }
> + avfilter_copy_buffer_ref_props(out, inpicref);
> + }
> +
> + for (i = 0; i < n->nb_planes; i++)
> + noise(out->data[i], inpicref->data[i], out->linesize[i],
> + inpicref->linesize[i], n->linesize[i], n->height[i], n, i);
> +
> + ret = ff_filter_frame(outlink, out);
> + if (inpicref != out)
> + avfilter_unref_buffer(inpicref);
> + return ret;
> +}
> +
From a quick look, it doesn't seem the code has some kind of fast path
when src == dst, so I think you could just request the write perm as
minimal permissions and simplify this function.
> +static av_cold void uninit(AVFilterContext *ctx)
> +{
> + NoiseContext *n = ctx->priv;
> + int i;
> +
> + for (i = 0; i < 4; i++)
> + av_freep(&n->param[i].noise);
> + av_opt_free(n);
> +}
> +
> +static const AVFilterPad inputs[] = {
> + {
> + .name = "default",
> + .type = AVMEDIA_TYPE_VIDEO,
> + .get_video_buffer = ff_null_get_video_buffer,
> + .filter_frame = filter_frame,
> + .config_props = config_input,
> + .min_perms = AV_PERM_READ,
> + },
> + { NULL }
> +};
> +
> +static const AVFilterPad outputs[] = {
> + {
> + .name = "default",
> + .type = AVMEDIA_TYPE_VIDEO,
> + },
> + { NULL }
> +};
> +
We tend to use the filter name as prefix for inputs and outputs (it can be
useful when debugging). We rarely do it for init and uninit (historical +
copy/paste between filters) so it doesn't matter for those, but I'd like
to keep the inputs and outputs one consistent if you don't mind.
> +AVFilter avfilter_vf_noise = {
> + .name = "noise",
> + .description = NULL_IF_CONFIG_SMALL("Add noise."),
> + .priv_size = sizeof(NoiseContext),
> + .init = init,
> + .uninit = uninit,
> + .query_formats = query_formats,
> + .inputs = inputs,
> + .outputs = outputs,
> + .priv_class = &noise_class,
> +};
LGTM otherwise
--
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130214/308e4c5a/attachment.asc>
More information about the ffmpeg-devel
mailing list