[FFmpeg-devel] [PATCH 2/3] lavf/avienc: Write palette after BITMAPINFOHEADER

[Mats Peterson]

On 02/19/2016 05:52 PM, Michael Niedermayer wrote:
> On Fri, Feb 19, 2016 at 04:35:17PM +0100, Mats Peterson wrote:
>>
>> --
>> Mats Peterson
>> http://matsp888.no-ip.org/~mats/
>
>>   avienc.c |   31 +++++++++++++++++++++++++++----
>>   1 file changed, 27 insertions(+), 4 deletions(-)
>> 8d58e7c344ff8650d971483a80211a9eeb540d20  0002-lavf-avienc-Write-palette-after-BITMAPINFOHEADER.patch
>>  From 0294d37cd23f971376290be68dd681513740bb06 Mon Sep 17 00:00:00 2001
>> From: Mats Peterson <matsp888 at yahoo.com>
>> Date: Fri, 19 Feb 2016 16:23:17 +0100
>> Subject: [PATCH 2/3] lavf/avienc: Write palette after BITMAPINFOHEADER
>>
>> ---
>>   libavformat/avienc.c |   31 +++++++++++++++++++++++++++----
>>   1 file changed, 27 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavformat/avienc.c b/libavformat/avienc.c
>> index 4e7bca4..3c357a2 100644
>> --- a/libavformat/avienc.c
>> +++ b/libavformat/avienc.c
>> @@ -79,6 +79,8 @@ typedef struct AVIStream {
>>       AVIIndex indexes;
>>   } AVIStream;
>>
>> +static int hdr_pal_done;
>
> this must be in the context, AVIStream
> non constant statics would break if there are 2 avi muxers
>

Alright.


>
>> +
>>   static int avi_write_packet_internal(AVFormatContext *s, AVPacket *pkt);
>>
>>   static inline AVIIentry *avi_get_ientry(const AVIIndex *idx, int ent_id)
>> @@ -211,6 +213,9 @@ static int avi_write_header(AVFormatContext *s)
>>               return AVERROR(ENOMEM);
>>       }
>>
>> +    /* Palette after BITMAPINFOHEADER in strf chunk */
>> +    hdr_pal_done = 0;
>> +
>>       /* header list */
>>       avi->riff_id = 0;
>>       list1 = avi_start_new_riff(s, pb, "AVI ", "hdrl");
>> @@ -359,6 +364,12 @@ static int avi_write_header(AVFormatContext *s)
>>                       && enc->pix_fmt == AV_PIX_FMT_RGB555LE
>>                       && enc->bits_per_coded_sample == 15)
>>                       enc->bits_per_coded_sample = 16;
>> +                /* Use private data for palette offset */
>> +                if (enc->bits_per_coded_sample <= 8) {
>> +                    enc->priv_data = av_malloc(sizeof(int64_t));
>> +                    if (!enc->priv_data)
>> +                        return AVERROR(ENOMEM);
>> +                }
>>                   ff_put_bmp_header(pb, enc, ff_codec_bmp_tags, 0, 0);
>>                   pix_fmt = avpriv_find_pix_fmt(avpriv_pix_fmt_bps_avi,
>>                                                 enc->bits_per_coded_sample);
>> @@ -673,16 +684,28 @@ static int avi_write_packet(AVFormatContext *s, AVPacket *pkt)
>>           if (ret) {
>>               if (ret == CONTAINS_PAL) {
>>                   int pc_tag, i;
>> +                int pal_size = 1 << enc->bits_per_coded_sample;
>> +                if (!hdr_pal_done) {
>> +                    int64_t cur_offset = avio_tell(pb);
>> +                    int64_t *pal_offset = (int64_t *)enc->priv_data;
>> +                    avio_seek(pb, *pal_offset, SEEK_SET);
>> +                    for (i = 0; i < pal_size; i++) {
>> +                        uint32_t v = AV_RL32(data + size - 4*pal_size + 4*i);
>> +                        avio_wl32(pb, v & 0xffffff);
>> +                    }
>> +                    avio_seek(pb, cur_offset, SEEK_SET);
>> +                    av_free(enc->priv_data);
>> +                    hdr_pal_done++;
>> +                }
>>                   avi_stream2fourcc(tag, stream_index, enc->codec_type);
>>                   tag[2] = 'p'; tag[3] = 'c';
>> -
>>                   pc_tag = ff_start_tag(pb, tag);
>>                   avio_w8(pb, 0);
>>                   avio_w8(pb, 0);
>>                   avio_wl16(pb, 0); // reserved
>> -                for (i = 0; i<256; i++) {
>> +                for (i = 0; i < pal_size; i++) {
>
> this mismatches the stored size above

Mismatches what stored size?


>
>
>> -                    uint32_t v = AV_RL32(data + size - 1024 + 4*i);
>> +                    uint32_t v = AV_RL32(data + size - 4*pal_size + 4*i);
>
>> -                    avio_wb32(pb, v<<8);
>> +                    avio_wl32(pb, v & 0xffffff);
>
> it looked correct when i tested this before the patch
>

Yes, but it wasn't. It stored the palette in RGB order instead of BGR.

Mats