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

[Carl Eugen Hoyos]

Reimar Döffinger <Reimar.Doeffinger <at> gmx.de> writes:

> On Sun, Sep 28, 2014 at 09:50:43AM +0200, Reimar Döffinger wrote:
> > On Sat, Sep 27, 2014 at 10:51:25AM +0200, Carl Eugen Hoyos wrote:
> > > Hi!
> > > 
> > > Attached patch is one of two patches necessary to fix ticket #1304.
> > > The MSDN documentation indicates that the palette is not part of the 
> > > bitmapinfoheader but the bitmapinfoheader is part of the bitmapinfo 
> > > structure that also contains the palette.
> > > 
> > > An alternative is to add the extradata_size except if it is a palette 
> > > but imo, attached is more correct.

I still believe that my original patch is correct but I 
will send one that only changes the bisize for palette 
so that the MPlayer demuxer will not break.

> > 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.
So when I wrote above "do not add extradata size" I of 
course meant the size of the palette.
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.
MPlayer's avi demuxer is the only one that only reads 
the extradata if it is part of the size of the 
bitmapinfoheader.

> > > 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.

The MPlayer demuxer is the only one that needs 
the extradata size added to the bitmapinfoheader 
size, this should be fixed there.

> 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?

Carl Eugen