[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