[FFmpeg-devel] [PATCH 2/2] lavf/avienc: Add xxpc entries to index
Mats Peterson
matsp888 at yahoo.com
Sat Mar 12 16:42:51 CET 2016
On 03/12/2016 04:37 PM, Mats Peterson wrote:
> On 03/12/2016 04:33 PM, Mats Peterson wrote:
>> On 03/12/2016 04:29 PM, Michael Niedermayer wrote:
>>> On Sat, Mar 12, 2016 at 02:54:45PM +0100, 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?
>>>
>>> patch one has a list of exceptions, this list contains every single
>>> case for which evidence has been provided
>>> that is all evidence provided so far is consistent in that a biSize
>>> of 40 is wrong for non palette global headers.
>>
>> 40 is exactly what it should be, in *all* cases, regardless of what
>> follows the BITMAPINFOHEADER. Read below.
>>
>>>
>>> The spec says:
>>> "A stream format chunk ('strf') must follow the stream header chunk.
>>> The stream format chunk describes the format of the data in the
>>> stream. The data contained in this chunk depends on the stream type.
>>> For video streams, the information is a BITMAPINFO structure,
>>> including palette information if appropriate. For audio streams, the
>>> information is a WAVEFORMATEX structure."
>>>
>>> If that chunk is a BITMAPINFO structure + a palette then formats
>>> without a palette would likely have biSize similar to the chunk
>>> size ...
>>>
>>> its quite possible iam missing some details of course ...
>>>
>>
>> You're missing the specification of the BITMAPINFOHEADER. It states that
>> biSize is the size of the structure, i.e. the BITMAPINFOHEADER itself.
>> No less, no more.
>>
>> https://msdn.microsoft.com/en-us/library/windows/desktop/dd183376%28v=vs.85%29.aspx
>>
>>
>>
>
> Now, the HuffYUV author has invented his own spec-breaking
> BITMAPINFOHEADER, and for asv1/asv2, I can accept a biSize of 48 since
> that's what ASUS writes to their files. But that's it.
>
> Mats
>
> _______________________________________________
It's a piece of cake to get the size of the extra data by just
subtracting the BITMAPINFOHEADER size (40) from the strf chunk size. And
all codecs work just fine with a biSize of 40, even HuffYUV.
Mats
More information about the ffmpeg-devel
mailing list