[FFmpeg-devel] [PATCH] lavfi: add bbox filter

[Stefano Sabatini]

On date Sunday 2012-03-11 10:02:14 +0100, Nicolas George encoded:
> Le duodi 22 ventôse, an CCXX, Stefano Sabatini a écrit :
> > Also add bbox.h and bbox.c files, based on the remove-logo filter by
> > Robert Edele.
> 
> Do you consider the code rewritten/trivial enough not to require Robert's
> approval for the licence change?

I do, but hoping that Robert will reply to my mail (I'll send some
more private mails soon), especially considering that I already did
many changes to the code myself.

> > 
> > These files are useful for sharing code with the pending removelogo port.
> > ---
> >  doc/filters.texi         |    9 +++
> >  libavfilter/Makefile     |    1 +
> >  libavfilter/allfilters.c |    1 +
> >  libavfilter/bbox.c       |   70 ++++++++++++++++++++++++++
> >  libavfilter/bbox.h       |   42 ++++++++++++++++
> >  libavfilter/vf_bbox.c    |  122 ++++++++++++++++++++++++++++++++++++++++++++++
> >  6 files changed, 245 insertions(+), 0 deletions(-)
> >  create mode 100644 libavfilter/bbox.c
> >  create mode 100644 libavfilter/bbox.h
> >  create mode 100644 libavfilter/vf_bbox.c
> > 
> > diff --git a/doc/filters.texi b/doc/filters.texi
> > index b13d5f5..62783c0 100644
> > --- a/doc/filters.texi
> > +++ b/doc/filters.texi
> > @@ -761,6 +761,15 @@ video, use the command:
> >  ass=sub.ass
> >  @end example
> >  
> > + at section bbox
> > +
> > +Compute the bounding box for each input frame.
> > +
> > +This filter computes the bounding box containing all the pixels with a
> > +non-null value. The bounding box is computed for each plane in the
> > +input image, and the corresponding parameters are print on the filter
> > +log.
> 
> "are printed".

Fixed.
 
> Also, we will need to decide something someday for those filters that print
> useful information, to allow to use them reliably in scripts.

Uhm... extend the ffprobe writers API?

[...]
> > +#include "bbox.h"
> > +
> > +void calculate_bounding_rectangle(BoundingBox *bbox,
> > +                                  const uint8_t *data, int linesize, int w, int h,
> > +                                  int min_val)
> 
> ff_ prefix missing.

Added.

> 
> > +{
> > +    int x, y;
> > +    int start_x;
> > +    int start_y;
> > +    int end_x = w - 1;
> > +    int end_y = h - 1;
> > +    int pixel_found;
> > +
> > +    /* left bound */
> > +    for (pixel_found = 0, start_x = 0; start_x < w; start_x++) {
> > +        for (y = 0; y < h; y++)
> > +            pixel_found |= (data[y * linesize + start_x] > min_val);
> 

> Did you check that the compiler is smart enough to optimize the
> multiplication away? If not, keeping and updating a pointer is easy an
> efficient.

not when the linesize is changing.

> > +        if (pixel_found)
> > +            break;
> > +    }
> 
> A goto to break the two loops at once may be more readable.

Changed (should be also faster).

I also added a return value for signaling when there are no pixels
with val>=min in the image.
 
> > +static void end_frame(AVFilterLink *inlink)
> > +{
> > +    AVFilterContext *ctx = inlink->dst;
> > +    BBoxContext *bbox = ctx->priv;
> > +    AVFilterBufferRef *picref = inlink->cur_buf;
> > +    BoundingBox box;
> > +    int plane;
> > +
> > +    for (plane = 0; picref->data[plane] && plane < 4; plane++) {
> > +        int w, h, vsub = 0, hsub = 0;
> > +        if (plane == 1 || plane == 2) {
> > +            hsub = bbox->hsub;
> > +            vsub = bbox->vsub;
> > +        }
> > +        calculate_bounding_rectangle(&box, picref->data[plane], picref->linesize[plane],
> > +                                     inlink->w>>hsub, inlink->h>>vsub, 16);
> 
> Is it right for chrominance planes? AFAIK, for chrominance, "neutral" values
> are around 128, not 0.

Indeed. Now I could drop support to chroma bbox detection or I could
extend the calculate_bounding_box() function to keep two values
(min_val and max_val), or check if the value is != null_value (I
adopted this variant).

More complex checks can be done by using a combination of lut (I may
make the null_value configurable, but then it would be maybe
overkill).

> > +        w = box.x2 - box.x1 + 1;
> > +        h = box.y2 - box.y1 + 1;
> > +        av_log(ctx, AV_LOG_INFO,
> > +               "n:%d pts:%s pts_time:%s plane:%d x1:%d x2:%d y1:%d y2:%d w:%d h:%d"
> > +               " crop=%d:%d:%d:%d drawbox=%d:%d:%d:%d\n",
> > +               bbox->frame,
> > +               av_ts2str(picref->pts), av_ts2timestr(picref->pts, &inlink->time_base),
> > +               plane, box.x1, box.x2, box.y1, box.y2, w, h,
> > +               w, h, box.x1, box.x2,    /* crop params */
> > +               box.x1, box.y1, w, h);   /* drawbox params */
> > +    }
> > +
> > +    bbox->frame++;
> > +    avfilter_end_frame(inlink->dst->outputs[0]);
> > +}
> > +
> > +AVFilter avfilter_vf_bbox = {
> > +    .name          = "bbox",
> > +    .description   = NULL_IF_CONFIG_SMALL("Compute bounding box for each frame."),
> > +    .priv_size     = sizeof(BBoxContext),
> > +    .query_formats = query_formats,
> > +    .init          = init,
> > +
> > +    .inputs = (const AVFilterPad[]) {
> > +        { .name       = "default",
> > +          .type             = AVMEDIA_TYPE_VIDEO,
> 
> Alignment is strange.

Fixed.

Also fixed output values for chroma planes.
-- 
FFmpeg = Fanciful & Frenzy Minimalistic Pacific Egregious Ghost
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavfi-add-bbox-filter.patch
Type: text/x-diff
Size: 11221 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120311/af3c6c2c/attachment.bin>