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

[Mats Peterson]

On 03/12/2016 03:00 PM, Mats Peterson wrote:
> 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
>>>

In what way did snow encoding fail? It works just fine here, Michael.

Mats