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 <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


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.



+                    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


In my book, any code is duplicated code, depending on how you look at it. Do you have a better suggestion? Shared static function, perhaps?


+                        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




_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel



--
Mats Peterson
http://matsp888.no-ip.org/~mats/
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to