[FFmpeg-devel] [PATCH] Add pad filter

[Stefano Sabatini]

On date Tuesday 2009-11-10 12:02:53 +0100, Diego Biurrun encoded:
> On Sun, Nov 08, 2009 at 09:50:13PM +0100, Stefano Sabatini wrote:
> > 
> > --- ffmpeg.orig/libavfilter/allfilters.c	2009-11-08 21:38:43.000000000 +0100
> > +++ ffmpeg/libavfilter/allfilters.c	2009-11-08 21:38:47.000000000 +0100
> > @@ -38,6 +38,7 @@
> >      REGISTER_FILTER (NOFORMAT, noformat, vf);
> >      REGISTER_FILTER (NULL, null, vf);
> > +    REGISTER_FILTER (PAD,  pad, vf);
> >      REGISTER_FILTER (SCALE, scale, vf);
> >      REGISTER_FILTER (SLICIFY, slicify, vf);
> 
> What is the 'vf' for anyway?

Video Filter, it's for avoiding name clashes between
filter/sources/sink of different types, for example vf_null != af_null
(Audio Filter) != vsink_null. 
> 
> > --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> > +++ ffmpeg/libavfilter/vf_pad.c	2009-11-08 21:38:47.000000000 +0100
> > @@ -0,0 +1,329 @@
> > +
> > +/**
> > + * @file libavfilter/vf_pad.c
> > + * video pad filter
> 
> "video padding" sounds better to my ears.

Fixed.

> > +static AVFilterPicRef *get_video_buffer(AVFilterLink *link, int perms, int w, int h)
> 
> needlessly long line (in other places as well)

Fixed.
 
> > +    picref->data[0] +=  pad->x             + (pad->y * picref->linesize[0]);
> > +    picref->data[1] += (pad->x>>pad->hsub) + (pad->y * picref->linesize[1]>>pad->vsub);
> > +    picref->data[2] += (pad->x>>pad->hsub) + (pad->y * picref->linesize[2]>>pad->vsub);
> 
> I think spaces around >> would greatly aid readability here (and in
> other places).

Done like that.
 
> > --- ffmpeg.orig/doc/libavfilter.texi	2009-11-08 21:38:43.000000000 +0100
> > +++ ffmpeg/doc/libavfilter.texi	2009-11-08 21:38:47.000000000 +0100
> > @@ -145,6 +145,59 @@
> >  
> > + at section pad
> > +
> > +Add pads at the unscaled border and places the unscaled original at
> 
> s/pads/padding/
> 
> > +size. Default expressions are respectively ``(out_w-in_w)/2'' and
> 
> I think "are respectively" is always wrong word ordering.  You can flag
> this mentally as a mistake.  Also, I think you can drop most uses of the
> word "respectively" from your sentences.
> 
> > +All the expressions can contains the constants "PI", and "E".
> 
> contain
> 
> > +adds a 50 pixels red border to the bottom of the picture.
>
> pixel

These don't apply anymore to the incoming patch, as I simplified the
syntax.

Regards.
-- 
FFmpeg = Foolish and Faithless Majestic Portentous Ecstatic Game