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 <matsp...@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
> + 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)
> + ffio_wfourcc(pb, ie->tag);
> + else {
> + avi_stream2fourcc(tag, stream_id,
> s->streams[stream_id]->codec->codec_type);
> - ffio_wfourcc(pb, tag);
> + ffio_wfourcc(pb, tag);
> + }
> avio_wl32(pb, ie->flags);
> avio_wl32(pb, ie->pos);
> avio_wl32(pb, ie->len);
> @@ -673,6 +678,7 @@ static int avi_write_packet(AVFormatContext *s, AVPacket
> *pkt)
> return avi_write_packet_internal(s, pkt); /* Passthrough */
>
> if (enc->codec_type == AVMEDIA_TYPE_VIDEO) {
> + AVIContext *avi = s->priv_data;
> AVIStream *avist = s->streams[stream_index]->priv_data;
> AVIOContext *pb = s->pb;
> AVPacket *opkt = pkt;
> @@ -709,6 +715,39 @@ static int avi_write_packet(AVFormatContext *s, AVPacket
> *pkt)
> unsigned char tag[5];
> avi_stream2fourcc(tag, stream_index, enc->codec_type);
> tag[2] = 'p'; tag[3] = 'c';
> + if (s->pb->seekable) {
> + AVIIndex *idx;
> + int cl, id;
> + if (avist->strh_flags_offset) {
> + int64_t cur_offset = avio_tell(pb);
> + avio_seek(pb, avist->strh_flags_offset,
> SEEK_SET);
> + avio_wl32(pb, AVISF_VIDEO_PALCHANGES);
> + avio_seek(pb, cur_offset, SEEK_SET);
> + avist->strh_flags_offset = 0;
> + }
> + idx = &avist->indexes;
> + cl = idx->entry / AVI_INDEX_CLUSTER_SIZE;
> + id = idx->entry % AVI_INDEX_CLUSTER_SIZE;
> + if (idx->ents_allocated <= idx->entry) {
> + idx->cluster = av_realloc_f(idx->cluster,
> sizeof(void*), cl+1);
> + if (!idx->cluster) {
> + idx->ents_allocated = 0;
> + idx->entry = 0;
> + return AVERROR(ENOMEM);
> + }
> + idx->cluster[cl] =
> + av_malloc(AVI_INDEX_CLUSTER_SIZE *
> sizeof(AVIIentry));
> + if (!idx->cluster[cl])
> + return AVERROR(ENOMEM);
> + idx->ents_allocated += AVI_INDEX_CLUSTER_SIZE;
> + }
this is identical to code from avi_write_packet_internal()
code should not be duplicated, its a recipe for bugs, duplicated code
will often not be fixed on both sides when one has a bug
> + strcpy(idx->cluster[cl][id].tag, tag);
fourccs are not really strings, strcpy is generally a bad idea
security wise
> + idx->cluster[cl][id].flags = AVIIF_NO_TIME;
> + idx->cluster[cl][id].pos = avio_tell(pb) -
> avi->movi_list;
> + idx->cluster[cl][id].len = pal_size * 4 + 4;
> + avist->max_size = FFMAX(avist->max_size, pal_size *
> 4 + 4);
> + idx->entry++;
this too is identical to avi_write_packet_internal()
> + }
> pc_tag = ff_start_tag(pb, tag);
> avio_w8(pb, 0);
> avio_w8(pb, pal_size & 0xFF);
> @@ -719,13 +758,6 @@ static int avi_write_packet(AVFormatContext *s, AVPacket
> *pkt)
> }
> ff_end_tag(pb, pc_tag);
> memcpy(avist->old_palette, avist->palette, pal_size * 4);
> - if (pb->seekable && avist->strh_flags_offset) {
> - int64_t cur_offset = avio_tell(pb);
> - avio_seek(pb, avist->strh_flags_offset, SEEK_SET);
> - avio_wl32(pb, AVISF_VIDEO_PALCHANGES);
> - avio_seek(pb, cur_offset, SEEK_SET);
> - avist->strh_flags_offset = 0;
> - }
> }
> }
> }
> diff --git a/tests/ref/lavf-fate/avi_cram b/tests/ref/lavf-fate/avi_cram
> index 7b4e69c..82882fb 100644
> --- a/tests/ref/lavf-fate/avi_cram
> +++ b/tests/ref/lavf-fate/avi_cram
> @@ -1,3 +1,3 @@
> -ba77c5c8bd2b0d1e0478d143346cc3b3 *./tests/data/lavf-fate/lavf.avi
> -928228 ./tests/data/lavf-fate/lavf.avi
> +6fc88702c23b895c305c5e1f51a0904e *./tests/data/lavf-fate/lavf.avi
> +928260 ./tests/data/lavf-fate/lavf.avi
> ./tests/data/lavf-fate/lavf.avi CRC=0xa4770de2
> --
> 1.7.10.4
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
It is what and why we do it that matters, not just one of them.
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
