[FFmpeg-devel] [PATCH] Add crop filter

[Stefano Sabatini]

On date Friday 2009-10-16 12:49:33 +0200, Michael Niedermayer encoded:
> On Fri, Oct 16, 2009 at 12:34:07AM +0200, Stefano Sabatini wrote:
> > 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).
> 
> fine but you know that crop silently does not do what the user asks for then

I know but a check+warning for that also seems overkill, better ideas
are welcome...

> [...]
> > +static int config_input(AVFilterLink *link)
> > +{
> > +    AVFilterContext *ctx = link->dst;
> > +    CropContext *crop = ctx->priv;
> > +    int round_hsub, round_vsub;
> > +
> > +    switch (link->format) {
> > +    case PIX_FMT_RGB32:
> > +    case PIX_FMT_BGR32:
> > +        crop->bpp = 32;
> > +        break;
> > +    case PIX_FMT_RGB24:
> > +    case PIX_FMT_BGR24:
> > +        crop->bpp = 24;
> > +        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 = 16;
> > +        break;
> > +    case PIX_FMT_MONOWHITE:
> > +    case PIX_FMT_MONOBLACK:
> > +        crop->bpp  = 1;
> 
> inconsistemt spaces before =
> 
> 
> > +        break;
> > +    default:
> > +        crop->bpp = 8;
> > +    }
> > +
> > +    avcodec_get_chroma_sub_sample(link->format, &crop->hsub, &crop->vsub);
> > +
> > +    if (crop->w == 0)
> > +        crop->w = link->w - crop->x;
> > +    if (crop->h == 0)
> > +        crop->h = link->h - crop->y;
> > +
> 
> > +    if (link->format == PIX_FMT_MONOWHITE || link->format == PIX_FMT_MONOBLACK) {
> > +        round_hsub = 3;
> > +        round_vsub = 0;
> > +    } else {
> 
> I think you should drop mono support, its not in SOC and i would prefer if
> this does not delay this 99.9% acceptable patch for months
> (its broken anyway as rounding is not correct, crop of mono should work
>  with any pixel value even not *8 but i know we will fight about how to
>  implement this so this would add a big delay ...)

Yes, this can be added later.

> > +        round_hsub = crop->hsub;
> > +        round_vsub = crop->vsub;
> > +    }
> > +
> > +    crop->x &= ~((1 << round_hsub) - 1);
> > +    crop->y &= ~((1 << round_vsub) - 1);
> 
> > +    crop->w &= ~((1 << round_hsub) - 1);
> > +    crop->h &= ~((1 << round_vsub) - 1);
> 
> i dont think these are needed

How so?

Regards.
-- 
FFmpeg = Faboulous Fundamental Muttering Proud Enlightening Game
-------------- next part --------------
A non-text attachment was scrubbed...
Name: add-crop-filter.patch
Type: text/x-diff
Size: 9883 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091019/185b46eb/attachment.patch>