[FFmpeg-devel] [PATCH] lavfi/color: cache and reuse colored picture in context

Stefano Sabatini stefasab at gmail.com
Sun Jul 29 18:32:27 CEST 2012


On date Sunday 2012-07-29 17:27:00 +0200, Nicolas George encoded:
> Le duodi 12 thermidor, an CCXX, Stefano Sabatini a écrit :
> > Avoid to avoid to fill the same picture again and again with the same
> 
> "Avoid to avoid"?

Fixed.
 
> > content.
> > Optimize computation, and provides an example for the use of the
> > AV_PERM_REUSE permission flag.
> > ---
> >  libavfilter/vsrc_color.c |   34 ++++++++++++++++++++++------------
> >  1 files changed, 22 insertions(+), 12 deletions(-)
> > 
> > diff --git a/libavfilter/vsrc_color.c b/libavfilter/vsrc_color.c
> > index ca336ac..9d78a32 100644
> > --- a/libavfilter/vsrc_color.c
> > +++ b/libavfilter/vsrc_color.c
> > @@ -45,6 +45,7 @@ typedef struct {
> >      uint64_t pts;
> >      FFDrawContext draw;
> >      FFDrawColor color;
> > +    AVFilterBufferRef *picref;  ///< cached reference containing the painted picture
> >  } ColorContext;
> >  
> >  #define OFFSET(x) offsetof(ColorContext, x)
> > @@ -92,6 +93,12 @@ end:
> >      return ret;
> >  }
> >  
> > +static av_cold void color_uninit(AVFilterContext *ctx)
> > +{
> > +    ColorContext *color = ctx->priv;
> > +    avfilter_unref_bufferp(&color->picref);
> > +}
> > +
> >  static int query_formats(AVFilterContext *ctx)
> >  {
> >      ff_set_common_formats(ctx, ff_draw_supported_pixel_formats(0));
> > @@ -124,18 +131,24 @@ static int color_config_props(AVFilterLink *inlink)
> >  static int color_request_frame(AVFilterLink *link)
> >  {
> >      ColorContext *color = link->src->priv;
> > -    AVFilterBufferRef *picref = ff_get_video_buffer(link, AV_PERM_WRITE, color->w, color->h);
> >      AVFilterBufferRef *buf_out;
> >      int ret;
> >  
> > -    if (!picref)
> > -        return AVERROR(ENOMEM);
> > -
> > -    picref->video->sample_aspect_ratio = (AVRational) {1, 1};
> > -    picref->pts = color->pts++;
> > -    picref->pos = -1;
> > +    if (!color->picref) {
> > +        color->picref =
> > +            ff_get_video_buffer(link, AV_PERM_WRITE|AV_PERM_REUSE,
> > +                                color->w, color->h);
> > +        if (!color->picref)
> > +            return AVERROR(ENOMEM);
> > +        ff_fill_rectangle(&color->draw, &color->color,
> > +                          color->picref->data, color->picref->linesize,
> > +                          0, 0, color->w, color->h);
> > +        color->picref->video->sample_aspect_ratio = (AVRational) {1, 1};
> > +        color->picref->pos = -1;
> > +    }
> >  
> > -    buf_out = avfilter_ref_buffer(picref, ~0);
> > +    color->picref->pts = color->pts++;
> 
> > +    buf_out = avfilter_ref_buffer(color->picref, ~0);
> 
> If you do not remove WRITE, the next filter may write in the original buffer
> and its result will stay on all frames. I predict that "drawtext=x=t*10:..."
> would leave a sludge.
> 
> PRESERVE may help avoid a few useless copies, but that I am not completely
> sure about.
> 
> (Speaking of which, your opinion in the thread "Understanding lavfi's
> permissions system" would be appreciated.)

I noticed the issue just after sending the patch, and yes, I was
reading that thread when I conceived the patch (sorry that I didn't
yet replied).

In this specific example I see two different ways to fix the problem.

1)
color->picref->perms = WRITE|READ|PRESERVE|REUSE;
buf_out = avfilter_ref_buffer(color->picref, ~0);

Since the new reference will inherit the color->picref flags, it will
be marked WRITE|READ|PRESERVE|REUSE, and a filter willing to write on it
will reject it because of rej_perm = PRESERVE.

2)
color->picref->perms = WRITE|REUSE;
buf_out->perms = READ;

in this case a filter willing to write on buf_out will reject it
because of min_perms = WRITE.

In general a filter which writes on a buffer should always set
rej_perms = PRESERVE and min_perms = WRITE, so the two solutions
should be equivalent.

I feel like there is some redundancy in this mechanism, maybe PRESERVE
could be removed in favor of the WRITE flag, I can't say for myself
which solution should be considered better amongst the two above.
-- 
FFmpeg = Furious Friendly Mournful Philosophical Educated Gorilla


More information about the ffmpeg-devel mailing list