[FFmpeg-devel] [PATCH 2/2] lavf/avienc: Add xxpc entries to index
Michael Niedermayer
michael at niedermayer.cc
Sat Mar 12 16:29:32 CET 2016
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.
It sure might be that the evidence is biased by bad luck as its only
few codecs we looked at in detail but still
also to justify this a bit more through the spec
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 ...
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Frequently ignored answer#1 FFmpeg bugs should be sent to our bugtracker. User
questions about the command line tools should be sent to the ffmpeg-user ML.
And questions about how to use libav* should be sent to the libav-user ML.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160312/75b05020/attachment.sig>
More information about the ffmpeg-devel
mailing list