[FFmpeg-devel] [PATCH] Unsharp filter
Bobby Bingham
uhmmmm
Fri Mar 19 07:05:20 CET 2010
On Thu, 18 Mar 2010 13:14:36 -0400
"Daniel G. Taylor" <dan at programmer-art.org> wrote:
> Hey,
>
> I'm interested in contributing to libavfilter. I have ported the
> unsharp filter from MPlayer, which is attached. I still don't fully
Great! It's always good to have more filters, both for using and to
serve as examples for future filter writers.
> understand how everything works, so please go easy on me in the
> review.
>
> The attached patch is against FFmpeg + SoC libavfilter. Usage
> examples are in the updated documentation.
>
> Take care,
> diff --git a/doc/libavfilter.texi b/doc/libavfilter.texi
> index 3f90a23..2e961d7 100644
> --- a/doc/libavfilter.texi
> +++ b/doc/libavfilter.texi
> @@ -344,6 +344,21 @@ The input video is passed on to two outputs.
>
> Transpose (line => column) input video to next video filter.
>
> + at section unsharp
> +
> +Sharpen or blur the input video. It takes four parameters: the effect type, width, height, and amount. The type can be one of 'l' for luma, 'c' for chroma, or 'b' for both. The width and height are integers defining how large the affected area is, while the amount defines the intensity. A negative amount blurs while a positive amount sharpens. Passing no arguments to the filter is the same as passing l:3:3:1.5.
> +
> + at example
> +# Use the default values
> +./ffmpeg -i in.avi -vfilters "[in] unsharp [out]"
> +
> +# Specify a strong sharpen effect
> +./ffmpeg -i in.avi -vfilters "[in] unsharp=l:5:5:1.5 [out]" out.avi
> +
> +# Specify a strong blur effect
> +./ffmpeg -i in.avi -vfilters "[in] unsharp=l:5:5:-1.5 [out]" out.avi
> + at end example
> +
> @section vflip
>
> Flip the input video vertically.
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index 9e59b0b..041bef0 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -32,6 +32,7 @@ OBJS-$(CONFIG_SLICIFY_FILTER) += vf_slicify.o
> OBJS-$(CONFIG_SPLIT_FILTER) += vf_split.o
> OBJS-$(CONFIG_TRANSPOSE_FILTER) += vf_transpose.o
> OBJS-$(CONFIG_VFLIP_FILTER) += vf_vflip.o
> +OBJS-$(CONFIG_UNSHARP_FILTER) += vf_unsharp.o
>
> OBJS-$(CONFIG_BUFFER_FILTER) += vsrc_buffer.o
> OBJS-$(CONFIG_MOVIE_FILTER) += vsrc_movie.o
You'll need to recreate this patch from current SVN. Spacing has
changed in this file recently. Please keep the list in alphabetic
order as well.
> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> index 73e5fbe..b925960 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -51,6 +51,7 @@ void avfilter_register_all(void)
> REGISTER_FILTER(SPLIT,split,vf);
> REGISTER_FILTER(TRANSPOSE,transpose,vf);
> REGISTER_FILTER(VFLIP,vflip,vf);
> + REGISTER_FILTER(UNSHARP,unsharp,vf);
>
> REGISTER_FILTER(BUFFER,buffer,vsrc);
> REGISTER_FILTER(MOVIE,movie,vsrc);
> diff --git a/libavfilter/vf_unsharp.c b/libavfilter/vf_unsharp.c
> new file mode 100644
> index 0000000..1cd033b
> --- /dev/null
> +++ b/libavfilter/vf_unsharp.c
> @@ -0,0 +1,244 @@
> +/*
> + * Ported to FFmpeg from MPlayer libmpcodecs/unsharp.c
> + * Original copyright (C) 2002 Remi Guyomarch <rguyom at pobox.com>
> + * Port copyright (C) 2010 Daniel G. Taylor <dan at programmer-art.org>
> + *
> + * 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.
I'm sure some developers would appreciate it if you could ask the
original developer about the possibility of relicensing under the LGPL
instead. If not, I believe there is some configure magic required to
make this only build when the user specified --enable-gpl.
> + *
> + * 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
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser 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
> + */
This is not the same license mentioned above.
> +
> +/**
> + * @file libavfilter/vf_unsharp.c
> + * blur / sharpen filter
> + */
> +
> +#include "avfilter.h"
> +#include "libavutil/common.h"
> +#include "libavutil/mem.h"
> +
> +#define MIN_MATRIX_SIZE 3
unused
> +#define MAX_MATRIX_SIZE 63
> +
> +typedef struct FilterParam
> +{
> + int msizeX, msizeY;
> + double amount;
> + uint32_t *SC[MAX_MATRIX_SIZE - 1];
> +} FilterParam;
> +
> +typedef struct
> +{
> + FilterParam luma; ///< luma parameters (width, height, amount)
> + FilterParam chroma; ///< chroma parameters (width, height, amount)
> +} UnsharpContext;
> +
> +//===========================================================================//
> +
> +/* This code is based on :
> +
> +An Efficient algorithm for Gaussian blur using finite-state machines
> +Frederick M. Waltz and John W. V. Miller
> +
> +SPIE Conf. on Machine Vision Systems for Inspection and Metrology VII
> +Originally published Boston, Nov 98
> +
> +https://docs.google.com/viewer?url=http://www.engin.umd.umich.edu/~jwvm/ece581/21_GBlur.pdf
> +
Why not link directly to the original PDF?
> +*/
> +
> +static void unsharpen( uint8_t *dst, uint8_t *src, int dstStride, int srcStride, int width, int height, FilterParam *fp )
> +{
> + uint32_t **SC = fp->SC;
> + uint32_t SR[MAX_MATRIX_SIZE - 1], Tmp1, Tmp2;
> + uint8_t* src2 = src; // avoid gcc warning
It looks like src2 always equals src, and is hence unnecessary.
> +
> + int32_t res;
> + int x, y, z;
> + int amount = fp->amount * 65536.0;
> + int stepsX = fp->msizeX / 2;
> + int stepsY = fp->msizeY / 2;
Instead of recalculating these same three values every from, calculate
them in init and store them in the context.
> + int scalebits = (stepsX + stepsY) * 2;
> + int32_t halfscale = 1 << ((stepsX + stepsY) * 2 - 1);
> +
> + if (!fp->amount)
> + {
> + if (src == dst)
> + return;
This cannot happen.
> +
> + if (dstStride == srcStride)
> + memcpy(dst, src, srcStride*height);
> + else
> + for (y = 0; y < height; y++, dst += dstStride, src += srcStride)
> + memcpy( dst, src, width );
> + return;
> + }
Tabs are forbidden in ffmpeg svn.
> +
> + for (y = 0; y < 2 * stepsY; y++)
> + {
> + memset(SC[y], 0, sizeof(SC[y][0]) * (width + 2 * stepsX));
> + }
> +
> + for (y =- stepsY; y < height + stepsY; y++)
> + {
> + if(y < height)
> + src2 = src;
> +
> + memset(SR, 0, sizeof(SR[0]) * (2 * stepsX - 1));
> +
> + for (x =- stepsX; x < width + stepsX; x++)
> + {
> + Tmp1 = x<=0 ? src2[0] : x>=width ? src2[width-1] : src2[x];
> + for (z = 0; z < stepsX * 2; z += 2)
> + {
> + Tmp2 = SR[z + 0] + Tmp1; SR[z + 0] = Tmp1;
> + Tmp1 = SR[z + 1] + Tmp2; SR[z + 1] = Tmp2;
> + }
> + for (z = 0; z < stepsY * 2; z += 2)
> + {
> + Tmp2 = SC[z + 0][x + stepsX] + Tmp1; SC[z + 0][x + stepsX] = Tmp1;
> + Tmp1 = SC[z + 1][x + stepsX] + Tmp2; SC[z + 1][x + stepsX] = Tmp2;
> + }
> + if (x >= stepsX && y >= stepsY)
> + {
> + uint8_t* srx = src - stepsY*srcStride + x - stepsX;
> + uint8_t* dsx = dst - stepsY*dstStride + x - stepsX;
> +
> + res = (int32_t)*srx + ((((int32_t) * srx - (int32_t)((Tmp1 + halfscale) >> scalebits)) * amount) >> 16);
> + *dsx = res > 255 ? 255 : res < 0 ? 0 : (uint8_t)res;
av_clip_uint8
> + }
> + }
> + if (y >= 0)
> + {
> + dst += dstStride;
> + src += srcStride;
> + }
> + }
> +}
I'll read the paper and come back and review this function. It does
look possible to support slices, rather than filtering the entire frame
at the end.
> +
> +static av_cold int init(AVFilterContext *ctx, const char *args, void *opaque)
> +{
> + UnsharpContext *unsharp = ctx->priv;
> + char type;
> + int msizeX, msizeY;
> + double amount;
> +
> + if (args)
> + sscanf(args, "%c:%d:%d:%lf", &type, &msizeX, &msizeY, &amount);
> + else
> + {
> + type = 'l';
> + msizeX = 3;
> + msizeY = 3;
> + amount = 1.5f;
> + }
> +
> + if (type == 'l' || type == 'b')
> + {
> + unsharp->luma.msizeX = msizeX;
> + unsharp->luma.msizeY = msizeY;
> + unsharp->luma.amount = amount;
> + }
> +
> + if (type == 'c' || type == 'b')
> + {
> + unsharp->chroma.msizeX = msizeX;
> + unsharp->chroma.msizeY = msizeY;
> + unsharp->chroma.amount = amount;
> + }
You should do some bounds checking on msize*.
> +
> + return 0;
> +}
> +
> +static int query_formats(AVFilterContext *ctx)
> +{
> + enum PixelFormat pix_fmts[] = {
> + PIX_FMT_YUV420P, PIX_FMT_NONE
> + };
Does this really only work for this format?
> +
> + avfilter_set_common_formats(ctx, avfilter_make_format_list(pix_fmts));
> +
> + return 0;
> +}
> +
> +static int config_props(AVFilterLink *link)
> +{
> + UnsharpContext *unsharp = link->dst->priv;
> + FilterParam *fp;
> + int z, stepsX, stepsY;
> + char *effect;
Either declare this const char * or put its value directly into the
av_log calls below to avoid warnings about discarding qualifiers.
> +
> + fp = &unsharp->luma;
> + effect = fp->amount == 0 ? "don't touch" : fp->amount < 0 ? "blur" : "sharpen";
> +
> + av_log(NULL, AV_LOG_ERROR, "unsharp: %dx%d:%0.2f (%s luma)\n", fp->msizeX, fp->msizeY, fp->amount, effect);
> +
> + memset(fp->SC, 0, sizeof(fp->SC));
> + stepsX = fp->msizeX / 2;
> + stepsY = fp->msizeY / 2;
> + for (z = 0; z < 2 * stepsY; z++)
> + {
> + fp->SC[z] = memalign(16, sizeof(*(fp->SC[z])) * (link->w + 2 * stepsX));
Use av_malloc instead of memalign. Also, you never free this memory.
> + }
> +
> + fp = &unsharp->chroma;
> + effect = fp->amount == 0 ? "don't touch" : fp->amount < 0 ? "blur" : "sharpen";
> +
> + av_log(NULL, AV_LOG_ERROR, "unsharp: %dx%d:%0.2f (%s chroma)\n", fp->msizeX, fp->msizeY, fp->amount, effect);
> +
> + memset(fp->SC, 0, sizeof(fp->SC));
> + stepsX = fp->msizeX / 2;
> + stepsY = fp->msizeY / 2;
> + for (z = 0; z < 2 * stepsY; z++)
> + {
> + fp->SC[z] = memalign(16, sizeof(*(fp->SC[z])) * (link->w + 2 * stepsX));
> + }
This is a duplicate of the above code. Factor it into a separate function please.
> +
> + return 0;
> +}
> +
> +static void end_frame(AVFilterLink *link)
> +{
> + UnsharpContext *unsharp = link->dst->priv;
> + AVFilterPicRef *in = link->cur_pic;
> + AVFilterPicRef *out = link->dst->outputs[0]->outpic;
> +
> + unsharpen(out->data[0], in->data[0], link->w, link->w, link->w, link->h, &unsharp->luma);
> + unsharpen(out->data[1], in->data[1], link->w, link->w, link->w, link->h, &unsharp->chroma);
> + unsharpen(out->data[2], in->data[2], link->w, link->w, link->w, link->h, &unsharp->chroma);
> +
You pass the width as the image strides. This is not correct. Use the
linesize member from AVFilterPicRef.
The actual dimensions of the chroma planes also do not match that of the
luma plane. Check out av_pix_fmt_descriptors in libavutil (someone
correct me if this isn't the current preferred way to get this
information).
> + avfilter_end_frame(link->dst->outputs[0]);
> +}
You need to call avfilter_unref_pic(in). You're leaking memory like mad.
> +
> +AVFilter avfilter_vf_unsharp = {
> + .name = "unsharp",
> + .description = NULL_IF_CONFIG_SMALL("Sharpen / blur input."),
> +
> + .priv_size = sizeof(UnsharpContext),
> +
> + .init = init,
> + .query_formats = query_formats,
> +
> + .inputs = (AVFilterPad[]) {{ .name = "default",
> + .type = CODEC_TYPE_VIDEO,
> + .end_frame = end_frame,
> + .config_props = config_props, },
You should specify .min_perms = AV_PERM_READ, because your code needs
permission to read from the source frame. I can't think of a reason
you wouldn't always get that permission, regardless, but it's better to
ask for it anyway.
> + { .name = NULL}},
> +
> + .outputs = (AVFilterPad[]) {{ .name = "default",
> + .type = CODEC_TYPE_VIDEO, },
> + { .name = NULL}},
> +};
--
Bobby Bingham
??????????????????????
More information about the ffmpeg-devel
mailing list