[FFmpeg-devel] [PATCH 2/2] lavf/avienc: Add xxpc entries to index

[Mats Peterson]

On 03/12/2016 02:54 PM, Mats Peterson wrote:
> On 03/12/2016 02:52 PM, Michael Niedermayer wrote:
>> On Sat, Mar 12, 2016 at 02:36:59PM +0100, Mats Peterson wrote:
>>> On 03/12/2016 02:26 PM, Mats Peterson wrote:
>>>> On 03/12/2016 12:53 PM, Michael Niedermayer wrote:
>>>>> On Sat, Mar 12, 2016 at 07:14:16AM +0100, Mats Peterson wrote:
>>>>>> Here's an interesting one. Windows Media Player won't make any
>>>>>> palette
>>>>>> changes without the xxpc chunks beeing indexed.
>>>>>>
>>>>>> Fixing the logic for reading and seeking with xxpc chunks in the
>>>>>> demuxer  is a future task. Now the muxing of video with xxpc chunks
>>>>>> works properly at least.
>>>>>>
>>>>>> Try playing the resulting test.avi file from the command line below
>>>>>> with Windows Media Player, with and without this patch.
>>>>>>
>>>>>> ffmpeg -i TOON.AVI -c:v copy -c:a copy test.avi
>>>>>>
>>>>>> Mats
>>>>>>
>>>>>> --
>>>>>> Mats Peterson
>>>>>> http://matsp888.no-ip.org/~mats/
>>>>>
>>>>>>   libavformat/avi.h            |    6 +++-
>>>>>>   libavformat/avienc.c         |   56
>>>>>> +++++++++++++++++++++++++++++++++----------
>>>>>>   tests/ref/lavf-fate/avi_cram |    4 +--
>>>>>>   3 files changed, 51 insertions(+), 15 deletions(-)
>>>>>> 2cf2565f9e258ee1a2bfcb83e4f30ecb1c13296d
>>>>>> 0002-Add-xxpc-entries-to-index.patch
>>>>>>  From 50f6c1dd38f503e77d53e0e6cdbadfe511282126 Mon Sep 17 00:00:00
>>>>>> 2001
>>>>>> From: Mats Peterson <matsp888 at yahoo.com>
>>>>>> Date: Sat, 12 Mar 2016 07:00:33 +0100
>>>>>> Subject: [PATCH 2/2] lavf/avienc: Add xxpc entries to index
>>>>>>
>>>>>> ---
>>>>>>   libavformat/avi.h            |    6 ++++-
>>>>>>   libavformat/avienc.c         |   56
>>>>>> +++++++++++++++++++++++++++++++++---------
>>>>>>   tests/ref/lavf-fate/avi_cram |    4 +--
>>>>>>   3 files changed, 51 insertions(+), 15 deletions(-)
>>>>>>
>>>>>> diff --git a/libavformat/avi.h b/libavformat/avi.h
>>>>>> index 34da76f..af21f2c 100644
>>>>>> --- a/libavformat/avi.h
>>>>>> +++ b/libavformat/avi.h
>>>>>> @@ -32,7 +32,11 @@
>>>>>>   #define AVI_MASTER_INDEX_SIZE   256
>>>>>>   #define AVI_MAX_STREAM_COUNT    100
>>>>>>
>>>>>> +/* stream header flags */
>>>>>> +#define AVISF_VIDEO_PALCHANGES  0x00010000
>>>>>> +
>>>>>>   /* index flags */
>>>>>> -#define AVIIF_INDEX             0x10
>>>>>> +#define AVIIF_INDEX             0x00000010
>>>>>> +#define AVIIF_NO_TIME           0x00000100
>>>>>>
>>>>>>   #endif /* AVFORMAT_AVI_H */
>>>>>> diff --git a/libavformat/avienc.c b/libavformat/avienc.c
>>>>>> index ad50379..b731bc2 100644
>>>>>> --- a/libavformat/avienc.c
>>>>>> +++ b/libavformat/avienc.c
>>>>>> @@ -44,13 +44,14 @@
>>>>>>    */
>>>>>>
>>>>>>   typedef struct AVIIentry {
>>>>>> -    unsigned int flags, pos, len;
>>>>>> +    char tag[5];
>>>>>
>>>>> the tag should be 4 bytes
>>>>> 5 is ugly, it requires padding and bloats the structure with a zero
>>>>> byte
>>>>>
>>>>
>>>> OK.
>>>>
>>>>>
>>>>>> +    unsigned int flags;
>>>>>> +    unsigned int pos;
>>>>>> +    unsigned int len;
>>>>>>   } AVIIentry;
>>>>>>
>>>>>>   #define AVI_INDEX_CLUSTER_SIZE 16384
>>>>>>
>>>>>> -#define AVISF_VIDEO_PALCHANGES 0x00010000
>>>>>> -
>>>>>>   typedef struct AVIIndex {
>>>>>>       int64_t     indx_start;
>>>>>>       int64_t     audio_strm_offset;
>>>>>
>>>>>> @@ -612,9 +613,13 @@ static int avi_write_idx1(AVFormatContext *s)
>>>>>>               }
>>>>>>               if (!empty) {
>>>>>>                   avist = s->streams[stream_id]->priv_data;
>>>>>> -                avi_stream2fourcc(tag, stream_id,
>>>>>> +                if (*ie->tag)
>>>>>
>>>>> ==18406== Conditional jump or move depends on uninitialised value(s,
>>>>> ==18406==    at 0x598D80: avi_write_idx1 (avienc.c:616)
>>>>> ==18406==    by 0x599D6D: avi_write_trailer (avienc.c:859)
>>>>> ==18406==    by 0x64A234: av_write_trailer (mux.c:1124)
>>>>> ==18406==    by 0x43A729: transcode (ffmpeg.c:4173)
>>>>> ==18406==    by 0x43ACE3: main (ffmpeg.c:4334)
>>>>>
>>>>
>>>> OK.
>>>
>>>
>>> It's not really uninitalised, is it? Since it isn't used by anything
>>> but my own code, it's all zero bytes, right?
>>
>> after this patch snow encoding failed, i run valgrind and saw this
>> so there was something wrong, i dont know for sure if it was this
>>
>>
>
> OK. By the way, are you in the process of applying patch 1 at least?
>
> Mats
>

I really don't like people who don't follow the AVI specs, using a 
biSize other than 40. But we could make an exception for HuffYUV, 
ffvhuff, asv1 and asv2.

Mats