[FFmpeg-devel] [PATCH]Do not add extradata size to bitmapinfoheader size

Reimar Döffinger Reimar.Doeffinger at gmx.de
Sun Sep 28 10:30:14 CEST 2014

On Sun, Sep 28, 2014 at 08:06:40AM +0000, Carl Eugen Hoyos wrote:
> > > biSize must be calculated so that it points to the 
> > > palette, so if you have both extradata and palette 
> > > you absolutely need to add the extradata size:
> This is wrong afaict.
> Or do we misunderstand each other?
> Currently, the size of the palette is added, I sincerely 
> hope no other extradata is used for pal8 rawvideo.

Not for pal8 rawvideo, but that function isn't only used for that?

> So when I wrote above "do not add extradata size" I of 
> course meant the size of the palette.

But you do not add any extradata, not just not the palette
(if both would exist).
So for a codec that has both extradata and palette the size
must include the other extradata, otherwise the palette
cannot be found properly.

> But I did test with different versions of WMP that all 
> extradata (WMV2 extradata) size does not have to be part 
> of the bitmapinfoheader size: WMP reads the extradata no 
> matter the size of the bitmapinfoheader.

That might be a matter of lack of error checking/handling,
in that it just dumps the whole chunk to the decoder, and
the decoder doesn't actually validate the data it gets but
just assumes it is in the right format.
If so, there's a good chance that this might change in the future.

> MPlayer's avi demuxer is the only one that only reads 
> the extradata if it is part of the size of the 
> bitmapinfoheader.

Does it? It thought it falls back to the size of the chunk?

> > > > An application should use the information stored 
> > > > in the biSize member to locate the color table 
> > > > in a BITMAPINFO structure, as follows.
> This is exactly what is currently broken in FFmpeg.

Yes, but after your change it is broken the other way.
If the palette does not start directly after the
bitmapinfoheader it still will not conform to that.
Not to mention the incorrect biClrUsed.

> > Also, we currently do not set biClrUsed, so I can't 
> > see how anything that uses palette could work at all.
> > Otherwise it would have been correct to set 
> > biSize = extradata_size - 4*biClrUsed
> Setting it to zero as we do seems correct or do I 
> miss something?

Only if the only thing we append is the palette, in
which case calling it "extradata" is rather idiotic.

More information about the ffmpeg-devel mailing list