[FFmpeg-devel] [PATCH] lavfi: copy palette in start_frame()

[Reimar Döffinger]

On Tue, Apr 10, 2012 at 04:39:57PM +0200, Stefano Sabatini wrote:
> On date Saturday 2012-04-07 10:30:53 +0200, Clément Bœsch encoded:
> > On Sat, Apr 07, 2012 at 10:03:04AM +0200, Stefano Sabatini wrote:
> > > Fix -vf copy with pal8 format.
> > > ---
> > >  libavfilter/avfilter.c |    4 ++++
> > >  1 files changed, 4 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> > > index 141fb9d..edf19dc 100644
> > > --- a/libavfilter/avfilter.c
> > > +++ b/libavfilter/avfilter.c
> > > @@ -588,6 +588,10 @@ void avfilter_start_frame(AVFilterLink *link, AVFilterBufferRef *picref)
> > >          link->cur_buf = avfilter_get_video_buffer(link, dst->min_perms, link->w, link->h);
> > >          link->src_buf = picref;
> > >          avfilter_copy_buffer_ref_props(link->cur_buf, link->src_buf);
> > > +
> > > +        /* copy palette if required */
> > > +        if (link->format == PIX_FMT_PAL8)
> > > +            memcpy(link->cur_buf->data[1], link->src_buf-> data[1], 256*4);
> > 
> > AVPALETTE_SIZE?
> 
> I'm not sure, what if we have palette with more than 8 bits per
> component?  Then maybe we should call it AV_PAL8_PALETTE_SIZE.

Well, AVPALETTE_SIZE is currently the defined size of data[1] for
paletted and pseudo-paletted formats, thus using that is the
sensible thing anyway IMHO.
Which makes me wonder though, why copy it only for PAL8?
What makes sure that the output data[1] is correctly initialized
for formats like RGB8?
And while we see crazy stuff all the time, more than 8 bit/component
for a paletted format seems a bit too crazy to me...
But there would be the option of adding a PAL16 flag then instead of
the PAL8 in the pixfmt description and you can figure it out from that
whether to use AVPALETTE_SIZE or AVPALETTE16_SIZE