[FFmpeg-devel] [PATCH] Add crop filter

[Stefano Sabatini]

On date Thursday 2009-10-15 16:11:20 +0200, Michael Niedermayer encoded:
> On Wed, Oct 14, 2009 at 08:49:16PM +0200, Stefano Sabatini wrote:
> > On date Wednesday 2009-10-14 01:37:46 +0200, Michael Niedermayer encoded:
> > > On Tue, Oct 13, 2009 at 08:47:09PM +0200, Stefano Sabatini wrote:
> > > > On date Tuesday 2009-10-13 13:16:29 +0200, Michael Niedermayer encoded:
> > > > > On Mon, Oct 12, 2009 at 08:41:08PM +0200, Stefano Sabatini wrote:
> > > > > > On date Monday 2009-10-12 09:28:29 +0200, Michael Niedermayer encoded:
> > > [...]
> > > > +    if (link->format == PIX_FMT_MONOWHITE || link->format == PIX_FMT_MONOBLACK) {
> > > > +        crop->x &= ~7;
> > > > +        crop->y &= ~7;
> > > > +        crop->w &= ~7;
> > > > +        crop->h &= ~7;
> > > 
> > > the y/h changes are wrong
> > 
> > Fixed, and factorized the following (ab)using the crop->hsub field.
> 
> hacks requires a comment in the code at least

After some thought I preferred to make the code slightly longer and
more explicit and avoid the hack but I won't bikeshed on this, just
tell what you prefer.

> [...]
> > +static int config_input(AVFilterLink *link)
> > +{
> > +    AVFilterContext *ctx = link->dst;
> > +    CropContext *crop = ctx->priv;
> > +
> > +    if (crop->w == 0)
> > +        crop->w = link->w - crop->x;
> > +    if (crop->h == 0)
> > +        crop->h = link->h - crop->y;
> > +
> > +    if (crop->x < 0 || crop->y < 0                      ||
> 
> > +        crop->w < 0 || crop->h < 0                      ||
> 
> <=

Fixed.

> > +        (unsigned)crop->x + (unsigned)crop->w > link->w ||
> > +        (unsigned)crop->y + (unsigned)crop->h > link->h) {
> > +        av_log(ctx, AV_LOG_ERROR,
> > +               "Output area %d:%d:%d:%d not within the input area 0:0:%d:%d\n",
> > +               crop->x, crop->y, crop->w, crop->h, link->w, link->h);
> > +        return -1;
> > +    }
> [...]
> > +
> > +    crop->x &= ~((1 << crop->hsub) - 1);
> > +    crop->y &= ~((1 << crop->vsub) - 1);
> > +    crop->w &= ~((1 << crop->hsub) - 1);
> > +    crop->h &= ~((1 << crop->vsub) - 1);
> > +
> > +    av_log(link->dst, AV_LOG_INFO, "x:%d y:%d w:%d h:%d\n", crop->x, crop->y, crop->w, crop->h);
> > +
> > +    if (crop->w == 0 || crop->h == 0) {
> > +        av_log(link->dst, AV_LOG_ERROR, "Size values too small\n");
> > +        return -1;
> > +    }
> 
> please move the check at the end, dont check and then change and then check
> again. or dont round at all and rather complain that the value is not ok

Implemented as normalize+round+check, I slightly prefer to round than
to pretend the user to do that (which implies a deep knowledge of the
various image formats).
 
> [...]
> > +static void draw_slice(AVFilterLink *link, int y, int h)
> > +{
> > +    AVFilterContext *ctx = link->dst;
> > +    CropContext *crop = ctx->priv;
> > +
> > +    int y1 = y;
> > +    int h1 = h;
> 
> why these variables?

Right, removed.

New patch attached.
-- 
FFmpeg = Furious Fostering Mastodontic Portable Enchanting Gem
-------------- next part --------------
A non-text attachment was scrubbed...
Name: add-crop-filter.patch
Type: text/x-diff
Size: 8260 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091016/7486fcd0/attachment.patch>