[vlc-devel] [PATCH] Revert "avi: Tidy the palette handling bit."

[Frode Tennebø]

On 15 October 2013 18:17, Rafaël Carré <funman at videolan.org> wrote:
> Le 15/10/2013 18:13, Jean-Baptiste Kempf a écrit :
>> On 15 Oct, Rafaël Carré wrote :
>>> Le 15/10/2013 17:27, Denis Charmet a écrit :
>>>> Hi,
>>>>
>>>> Le mardi 15 octobre 2013 à 05:18:06, Rafaël Carré a écrit :
>>>>> +                    __MAX( p_vids->p_bih->biSize - sizeof( VLC_BITMAPINFOHEADER ),
>>>>> +                           p_vids->i_chunk_size - sizeof(VLC_BITMAPINFOHEADER) );
>>>> not __MIN?
>>>
>>> See 7f99f183c055dbb657a896adb568687b6264df44 from the same author.

If memory serves me correctly, the reason for the original patch was
to make sure that biSize should not be used to pick the palette and
was only meant as a stop-gap measure for the real patch.  biSize is
the size of the BITMAPINFORHEADER which is already loaded into p_bih.
The palette data is directly after the BITMAPINFORHEADER (p_bih[1])
and occupies the rest of the chunk ( p_vids->i_chunk_size -
sizeof(VLC_BITMAPINFOHEADER)).

>>> https://trac.videolan.org/vlc/ticket/5952 has been reopened.

Which I don't understand as the videos plays nicely with 2.0.8 on
Ubuntu Linux.  However, I believe the palette issue is still not fixed
for Windows, but I have not been able to compile for Windows and I'm
not sure how to proceed on that platform, but I think that is a
separate issue.

The file in ticket 9546 also plays fine in 2.0.8 on Ubuntu Linux (no
sound though, and apparently I have a too slow laptop nowadays...).

>>> Should we revert the whole of his 2 commits ?

Reverting the first patch will also bring back the all-black palette bug.

I'm uncertain how reverting the second patch changes anything except
creating fmt.p_extra also for non-palletized formats.  However, the
comment just between those chunks which has now been reverted suggest
that it should not make any difference:

 /* Extract palette from extradata if bpp <= 8
  * (assumes that extradata contains only palette but appears
  *  to be true for all palettized codecs we support) */

However (https://en.wikipedia.org/wiki/BMP_file_format) suggest otherwise.

But again, it is strange this works on Linux...

>> If you wish.
>> Mailing him would be nice
>
> Done, let's see what he thinks of this

Thanks for the heads up.  I'm happy to assist in any way I can.

  -Frode