[FFmpeg-cvslog] r25118 - in trunk: Changelog configure doc/filters.texi libavfilter/Makefile libavfilter/allfilters.c libavfilter/avfilter.h libavfilter/vf_libopencv.c

[Diego Biurrun]

On Tue, Sep 14, 2010 at 03:21:14PM +0200, stefano wrote:
> 
> Log:
> Implement libopencv smooth filter.
> 
> --- trunk/Changelog	Tue Sep 14 02:17:58 2010	(r25117)
> +++ trunk/Changelog	Tue Sep 14 15:21:13 2010	(r25118)
> @@ -35,6 +35,7 @@ version <next>:
>  - MMS-HTTP support
>  - G.722 ADPCM audio decoder
>  - R10k video decoder
> +- ocv_smooth filter

The readers of the Changelog should not have to guess what ocv_
stands for.  I would not have guessed correctly without reading
the commit.
 
> --- trunk/doc/filters.texi	Tue Sep 14 02:17:58 2010	(r25117)
> +++ trunk/doc/filters.texi	Tue Sep 14 15:21:13 2010	(r25118)
> @@ -112,6 +112,33 @@ input to the vflip filter.
>  
> +It accepts the following parameters:

s/It/The filter/

> + at var{type}:@var{param1}:@var{param2}:@var{param3}:@var{param4}.
> +
> + at var{type} is the type of smooth filter to apply, and can be one of
> +the following value: "blur", "blur_no_scale", "median", "gaussian",

valueS

> +These parameters corresponds to the parameters assigned to the

correspond

> +libopencv function @code{cvSmooth}. Refer the official libopencv

Refer to

Please doublecheck your plural/singular uses before committing.

> --- /dev/null	00:00:00 1970	(empty, because file is newly added)
> +++ trunk/libavfilter/vf_libopencv.c	Tue Sep 14 15:21:13 2010	(r25118)
> @@ -0,0 +1,156 @@
> +/*
> + * copyright Stefano Sabatini 2010

This looks slightly weird, compare the other copyright lines in the
code.

> +#include "opencv/cv.h"
> +#include "opencv/cxtypes.h"

System headers should use <> instead of "".

> +static void fill_iplimage_from_picref(IplImage *img, const AVFilterBufferRef *picref, enum PixelFormat pixfmt)

Here (and in lots of other places) you could easily break this long
line for extra readability.

> +static void null_draw_slice(AVFilterLink *link, int y, int h, int slice_dir) { }

Hmmm?

> +#if CONFIG_OCV_SMOOTH_FILTER

This is pointless and thus harmful: The file is only ever compiled
if that condition is true...

> +typedef struct {
> +    int type;
> +    int    param1, param2;
> +    double param3, param4;
> +} SmoothContext;

What's with the typedef mania that everybody has?  Just write 'struct'...

> +static void smooth_end_frame(AVFilterLink *inlink)
> +{
> +    SmoothContext *smooth = inlink->dst->priv;
> +    AVFilterLink *outlink= inlink->dst->outputs[0];

align :)

Diego