[FFmpeg-devel] [PATCH] Add crop filter

[Stefano Sabatini]

On date Monday 2009-10-12 09:28:29 +0200, Michael Niedermayer encoded:
> On Sun, Oct 11, 2009 at 09:40:40PM +0200, Stefano Sabatini wrote:
> > On date Saturday 2009-10-10 01:51:39 +0200, Michael Niedermayer encoded:
> > > On Wed, Oct 07, 2009 at 11:50:19PM +0200, Stefano Sabatini wrote:
> > > > 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:
> [...]
> > +#define SET_OFFSET(x, max)                                              \
> > +    if (x < 0)                                                          \
> > +        x = max + x;                                                    \
> > +    if (x < 0 || x > max-1) {                                           \
> > +        av_log(ctx, AV_LOG_ERROR, "Value for offset %d out of the valid range 0 - %d\n", x, max-1); \
> > +        return -1;                                                      \
> > +    }
> > +
> > +static int norm_size(int x, int max, int max_contained)
> > +{
> > +    if (x == 0)
> > +        x = max;
> > +    if (x < 0)
> > +        x = max + x;
> > +    return av_clip(x, 1, max_contained);
> > +}
> > +
> > +static int config_input(AVFilterLink *link)
> > +{
> > +    AVFilterContext *ctx = link->dst;
> > +    CropContext *crop = ctx->priv;
> > +
> > +    SET_OFFSET(crop->x, link->w);
> > +    SET_OFFSET(crop->y, link->h);
> > +
> > +    crop->w = norm_size(crop->w, link->w, link->w - crop->x);
> > +    crop->h = norm_size(crop->h, link->h, link->h - crop->y);
> 
> mess
> 
> if(   crop->x < 0 || crop->y < 0 || crop->w < 0 || crop->h < 0
>    || (unsigned)crop->x + (unsigned)crop->w > link->w
>    || (unsigned)crop->y + (unsigned)crop->h > link->h)

Bah, this is missing even the original filter facility (size == 0 ->
size = max allowed) and doesn't provide any hints about what the user
is doing wrong in that case (just a laconic: somethin's wrong).

So specify which semantics you want implemented, that's clearly not
what I documented in the texi file.
 
> > +
> > +    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;
> > +    }
> 
> i wonder if we have some code that does that alraedy 

av_get_bits_per_pixel() in libavcodec/pixdesc.c, though it's still
disabled (all hwaccel pixdesc definitions missing).
 
> [...]
> > +static void start_frame(AVFilterLink *link, AVFilterPicRef *picref)
> > +{
> > +    CropContext *crop = link->dst->priv;
> > +    AVFilterPicRef *ref2 = avfilter_ref_pic(picref, ~0);
> > +    int i;
> > +
> > +    ref2->w        = crop->w;
> > +    ref2->h        = crop->h;
> > +
> > +    switch (link->format) {
> 
> > +    case PIX_FMT_MONOWHITE:
> > +    case PIX_FMT_MONOBLACK:
> > +        ref2->data[0] += (crop->y * ref2->linesize[0]) + (crop->x >> 3);
> > +        break;
> > +
> > +    case PIX_FMT_PAL8:
> > +        ref2->data[0] += crop->y * ref2->linesize[0] + crop->x;
> > +        break;
> > +
> > +    default:
> > +        ref2->data[0] += crop->y * ref2->linesize[0];
> > +        ref2->data[0] += crop->x * crop->bpp;
> 
> that looks factorizeable

The alternative I get is less readable though:

    ref2->data[0] += (crop->y * ref2->linesize[0]);
    if (link->format == PIX_FMT_MONOWHITE || PIX_FMT_MONOBLACK)
        ref2->data[0] += (crop->x >> 3);
    else
        ref2->data[0] += crop->x * crop->bpp;

    if (link->format != PIX_FMT_PAL8) {
        for (i = 1; i < 3; i ++) {
            if (ref2->data[i]) {
                ref2->data[i] += (crop->y >> crop->vsub) * ref2->linesize[i];
                ref2->data[i] +=  crop->x >> crop->hsub;
            }
        }
    }

Regards.
-- 
FFmpeg = Faboulous & Forgiving Merciless Problematic Elitarian Gadget
-------------- next part --------------
A non-text attachment was scrubbed...
Name: add-crop-filter.patch
Type: text/x-diff
Size: 8865 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091012/48168970/attachment.patch>