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

[Stefano Sabatini]

On date Thursday 2010-09-16 10:04:05 +0200, Diego Biurrun wrote:
> 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?

I noticed this helps readability, and also this makes it reusable by
many ocv filters (that was the original idea).

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

Yes but my idea was to put here more than one ocv filters, so it's
useful for enabling just one of these.
 
> > +typedef struct {
> > +    int type;
> > +    int    param1, param2;
> > +    double param3, param4;
> > +} SmoothContext;
> 
> What's with the typedef mania that everybody has?  Just write 'struct'...

Save some typing, you know FFmpeg devs are *lazy*...
 
> > +static void smooth_end_frame(AVFilterLink *inlink)
> > +{
> > +    SmoothContext *smooth = inlink->dst->priv;
> > +    AVFilterLink *outlink= inlink->dst->outputs[0];
> 
> align :)

Fixed.

Regards.