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

[Stefano Sabatini]

On date Tuesday 2012-04-10 20:39:35 +0200, Reimar Döffinger encoded:
> 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?

Question is, is it really used for so-called "pseudo-palette" formats?
Also I'm not sure it was a good idea to define that flag in pixdesc,
supposing that the pseudo palette is a libswscale internal thing.

/**
 * The pixel format is "pseudo-paletted". This means that Libav treats it as
 * paletted internally, but the palette is generated by the decoder and is not
 * stored in the file.
 */
#define PIX_FMT_PSEUDOPAL 64

Given that libavutil doesn't know nothing about "decoders" it just
looks weird to me.

> 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

Moved to pixfmt, maybe we could also rename it AV_PALETTE_SIZE while
at it.
-- 
FFmpeg = Forgiving & Frightening Merciless Political Ecumenical Governor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavu-pixfmt-move-AVPALETTE_SIZE-and-_COUNT-to-pixfmt.patch
Type: text/x-diff
Size: 1157 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120420/6fa8816a/attachment.bin>