[FFmpeg-devel] [PATCH] Add crop filter

[Stefano Sabatini]

On date Tuesday 2009-10-06 15:01:32 +0200, Michael Niedermayer encoded:
> On Tue, Oct 06, 2009 at 12:58:41AM +0200, Stefano Sabatini wrote:
> > Hi,
> > 
> > let's start to add some filter to the main repo.
> > 
> > As long as I'll add them, I plan to remove them from the soc repo, so
> > that further changes will be directly applied to the main repo.
> > 
> > In the meaningwhile though regression tests will be yet performed in
> > soc.
> > 
> > In attachment there is the exact copy of the crop filter you can find
> > in the soc repo.
> > 
> > Regards.
> > -- 
> > FFmpeg = Fundamental & Friendly Merciless Portable Epic Geisha
> 
> >  Makefile     |    2 
> >  allfilters.c |    3 -
> >  vf_crop.c    |  160 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 162 insertions(+), 3 deletions(-)
> > 32cd7ccf67a2827a81eb82007922aee682411066  add-crop-filter.patch
> > Index: ffmpeg/libavfilter/Makefile
> > ===================================================================
> > --- ffmpeg.orig/libavfilter/Makefile	2009-10-06 00:37:21.000000000 +0200
> > +++ ffmpeg/libavfilter/Makefile	2009-10-06 00:37:45.000000000 +0200
> > @@ -11,6 +11,6 @@
> >         defaults.o                                                       \
> >         formats.o                                                        \
> >  
> > -#OBJS-$(CONFIG_XXX_FILTER)    += vf_xxx.o
> > +OBJS-$(CONFIG_CROP_FILTER)    += vf_crop.o
> >  
> >  include $(SUBDIR)../subdir.mak
> > Index: ffmpeg/libavfilter/allfilters.c
> > ===================================================================
> > --- ffmpeg.orig/libavfilter/allfilters.c	2009-10-06 00:37:55.000000000 +0200
> > +++ ffmpeg/libavfilter/allfilters.c	2009-10-06 00:38:06.000000000 +0200
> > @@ -34,6 +34,5 @@
> >          return;
> >      initialized = 1;
> >  
> > -//    REGISTER_FILTER (CROP,crop,vf);
> > -
> > +    REGISTER_FILTER (CROP,crop,vf);
> >  }
> > Index: ffmpeg/libavfilter/vf_crop.c
> > ===================================================================
> > --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> > +++ ffmpeg/libavfilter/vf_crop.c	2009-10-06 00:37:11.000000000 +0200
> > @@ -0,0 +1,160 @@
> > +/*
> > + * video crop filter
> > + * copyright (c) 2007 Bobby Bingham
> > + *
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * 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
> > + */
> > +
> > +#include <stdio.h>
> > +
> > +#include "avfilter.h"
> > +
> > +typedef struct
> > +{
> 
> > +    int  x,  y,  w,  h;
> > +    int cx, cy, cw, ch;
> 
> these are missing doxygen comments explaining what they are

Removed cx, cy, cw, ch which seem are not required. 

> > +
> > +    int bpp;                //< bytes per pixel
> > +    int hsub, vsub;         //< chroma subsampling
> > +} CropContext;
> > +
> 
> > +static av_cold int init(AVFilterContext *ctx, const char *args, void *opaque)
> > +{
> > +    CropContext *crop = ctx->priv;
> > +
> > +    /* default parameters */
> > +    crop->x = 0;
> > +    crop->y = 0;
> 
> i would assume that the context is zeroed when its allocated, is that not so?

Yes, so I skipped the initialization step (also w and h are now
initialized to 0, so no need to init their values).
 
> 
> > +    crop->w = -1;
> > +    crop->h = -1;
> > +
> > +    if(args)
> > +        sscanf(args, "%d:%d:%d:%d", &crop->x, &crop->y, &crop->w, &crop->h);
> > +
> > +    return 0;
> > +}
> > +
> 
> > +static int config_input(AVFilterLink *link)
> > +{
> > +    CropContext *crop = link->dst->priv;
> > +
> > +    crop->cx = FFMIN(crop->x, link->w - 1);
> > +    crop->cy = FFMIN(crop->y, link->h - 1);
> > +    crop->cw = FFMIN(crop->w, link->w - crop->cx);
> > +    crop->ch = FFMIN(crop->h, link->h - crop->cy);
> > +
> > +    if(crop->cw <= 0) crop->cw = link->w - crop->cx;
> > +    if(crop->ch <= 0) crop->ch = link->h - crop->cy;
> 
> this seems to check for w/h < 0 while x/y are not checked ...

Added checks and changed the semantics of the values when their value
is negative, does it make sense to you? (this is also
documented in the docs).

> > +
> > +    switch(link->format) {
> > +    case PIX_FMT_RGB32:
> > +    case PIX_FMT_BGR32:
> > +        crop->bpp = 4;
> > +        break;
> > +    case PIX_FMT_RGB24:
> > +    case PIX_FMT_BGR24:
> > +        crop->bpp = 3;
> > +        break;
> > +    case PIX_FMT_RGB565:
> > +    case PIX_FMT_RGB555:
> > +    case PIX_FMT_BGR565:
> > +    case PIX_FMT_BGR555:
> > +    case PIX_FMT_GRAY16BE:
> > +    case PIX_FMT_GRAY16LE:
> > +        crop->bpp = 2;
> > +        break;
> > +    default:
> > +        crop->bpp = 1;
> > +    }
> > +
> 
> > +    avcodec_get_chroma_sub_sample(link->format, &crop->hsub, &crop->vsub);
> > +    crop->cx &= ~((1 << crop->hsub) - 1);
> > +    crop->cw &= ~((1 << crop->hsub) - 1);
> > +    crop->ch &= ~((1 << crop->vsub) - 1);
> 
> and cy?

Added.

Added the missing docs for the filters, depending on the vfilter.texi
patch.

Regards.
-- 
FFmpeg = Frenzy & Furious Murdering Practical Exxagerate Guru
-------------- next part --------------
A non-text attachment was scrubbed...
Name: add-crop-filter.patch
Type: text/x-diff
Size: 8070 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091007/a52f54b3/attachment.patch>