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

[Mats Peterson]

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