]> git.sesse.net Git - ffmpeg/commitdiff
avformat/matroskaenc: Don't ignore tags of chapters written late
authorAndreas Rheinhardt <andreas.rheinhardt@gmail.com>
Wed, 29 Apr 2020 08:27:23 +0000 (10:27 +0200)
committerAndreas Rheinhardt <andreas.rheinhardt@gmail.com>
Tue, 19 May 2020 01:34:44 +0000 (03:34 +0200)
The Matroska muxer writes the Chapters early when chapters were already
available when writing the header; in this case any tags pertaining to
these chapters get written, too.

Yet if no chapters had been supplied before writing the header, Chapters
can also be written when writing the trailer if any are supplied. Tags
belonging to these chapters were up until now completely ignored.

This commit changes this: Writing the tags belonging to chapters has
been moved to mkv_write_chapters(). If mkv_write_tags() has not been
called yet (i.e. when chapters are written when writing the header),
the AVIOContext for writing the ordinary Tags element is used, but not
output, as this is left to mkv_write_tags() in order to only write one
Tags element. Yet if mkv_write_tags() has already been called,
mkv_write_chapters() will output a Tags element of its own which only
contains the tags for chapters.

When chapters are available initially, the corresponding tags will now
be the first tags in the Tags element; but the ordering of tags in Tags
is irrelevant anyway.

This commit also makes chapter_id_offset local to mkv_write_chapters()
as it is used only there and not reused at all.

Potentially writing a second Tags element means that the maximum number
of SeekHead entries had to be incremented. All the changes to FATE
result from the ensuing increase in the amount of space reserved for the
SeekHead (21 bytes more).

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
13 files changed:
libavformat/matroskaenc.c
tests/fate/matroska.mak
tests/fate/wavpack.mak
tests/ref/fate/aac-autobsf-adtstoasc
tests/ref/fate/binsub-mksenc
tests/ref/fate/matroska-flac-extradata-update
tests/ref/fate/rgb24-mkv
tests/ref/fate/webm-dash-chapters
tests/ref/lavf-fate/av1.mkv
tests/ref/lavf/mka
tests/ref/lavf/mkv
tests/ref/lavf/mkv_attachment
tests/ref/seek/lavf-mkv

index 859c82f7cf2a3a3e94f15081c30beb374f3b0bd3..fccfee539a40eec3d20452c816f64bcb29e30779 100644 (file)
@@ -55,8 +55,8 @@
 #include "libavcodec/mpeg4audio.h"
 
 /* Level 1 elements we create a SeekHead entry for:
- * Info, Tracks, Chapters, Attachments, Tags and Cues */
-#define MAX_SEEKHEAD_ENTRIES 6
+ * Info, Tracks, Chapters, Attachments, Tags (potentially twice) and Cues */
+#define MAX_SEEKHEAD_ENTRIES 7
 
 #define IS_SEEKABLE(pb, mkv) (((pb)->seekable & AVIO_SEEKABLE_NORMAL) && \
                               !(mkv)->is_live)
@@ -142,8 +142,8 @@ typedef struct MatroskaMuxContext {
     unsigned            nb_attachments;
     int                 have_video;
 
-    uint32_t            chapter_id_offset;
     int                 wrote_chapters;
+    int                 wrote_tags;
 
     int                 reserve_cues_space;
     int                 cluster_size_limit;
@@ -1546,6 +1546,8 @@ static int mkv_write_tags(AVFormatContext *s)
     ebml_master tag, *tagp = IS_SEEKABLE(s->pb, mkv) ? &tag : NULL;
     int i, ret;
 
+    mkv->wrote_tags = 1;
+
     ff_metadata_conv_ctx(s, ff_mkv_metadata_conv, NULL);
 
     if (mkv_check_tag(s->metadata, 0)) {
@@ -1586,21 +1588,6 @@ static int mkv_write_tags(AVFormatContext *s)
         }
     }
 
-    if (mkv->mode != MODE_WEBM) {
-        for (i = 0; i < s->nb_chapters; i++) {
-            AVChapter *ch = s->chapters[i];
-
-            if (!mkv_check_tag(ch->metadata, MATROSKA_ID_TAGTARGETS_CHAPTERUID))
-                continue;
-
-            ret = mkv_write_tag(mkv, ch->metadata, &mkv->tags.bc, NULL,
-                                MATROSKA_ID_TAGTARGETS_CHAPTERUID,
-                                (uint32_t)ch->id + (uint64_t)mkv->chapter_id_offset);
-            if (ret < 0)
-                return ret;
-        }
-    }
-
     if (mkv->nb_attachments && mkv->mode != MODE_WEBM) {
         for (i = 0; i < s->nb_streams; i++) {
             const mkv_track *track = &mkv->tracks[i];
@@ -1629,8 +1616,9 @@ static int mkv_write_tags(AVFormatContext *s)
 static int mkv_write_chapters(AVFormatContext *s)
 {
     MatroskaMuxContext *mkv = s->priv_data;
-    AVIOContext *dyn_cp = NULL, *pb = s->pb;
+    AVIOContext *dyn_cp = NULL, *dyn_tags = NULL, **tags, *pb = s->pb;
     ebml_master editionentry;
+    uint64_t chapter_id_offset = 0;
     AVRational scale = {1, 1E9};
     int i, ret;
 
@@ -1639,7 +1627,7 @@ static int mkv_write_chapters(AVFormatContext *s)
 
     for (i = 0; i < s->nb_chapters; i++)
         if (!s->chapters[i]->id) {
-            mkv->chapter_id_offset = 1;
+            chapter_id_offset = 1;
             break;
         }
 
@@ -1648,8 +1636,13 @@ static int mkv_write_chapters(AVFormatContext *s)
         return ret;
 
     editionentry = start_ebml_master(dyn_cp, MATROSKA_ID_EDITIONENTRY, 0);
-    if (mkv->mode != MODE_WEBM)
+    if (mkv->mode != MODE_WEBM) {
         put_ebml_uint(dyn_cp, MATROSKA_ID_EDITIONFLAGDEFAULT, 1);
+        /* If mkv_write_tags() has already been called, then any tags
+         * corresponding to chapters will be put into a new Tags element. */
+        tags = mkv->wrote_tags ? &dyn_tags : &mkv->tags.bc;
+    } else
+        tags = NULL;
 
     for (i = 0; i < s->nb_chapters; i++) {
         ebml_master chapteratom, chapterdisplay;
@@ -1661,13 +1654,13 @@ static int mkv_write_chapters(AVFormatContext *s)
             av_log(s, AV_LOG_ERROR,
                    "Invalid chapter start (%"PRId64") or end (%"PRId64").\n",
                    chapterstart, chapterend);
-            ffio_free_dyn_buf(&dyn_cp);
-            return AVERROR_INVALIDDATA;
+            ret = AVERROR_INVALIDDATA;
+            goto fail;
         }
 
         chapteratom = start_ebml_master(dyn_cp, MATROSKA_ID_CHAPTERATOM, 0);
         put_ebml_uint(dyn_cp, MATROSKA_ID_CHAPTERUID,
-                      (uint32_t)c->id + (uint64_t)mkv->chapter_id_offset);
+                      (uint32_t)c->id + chapter_id_offset);
         put_ebml_uint(dyn_cp, MATROSKA_ID_CHAPTERTIMESTART, chapterstart);
         put_ebml_uint(dyn_cp, MATROSKA_ID_CHAPTERTIMEEND, chapterend);
         if ((t = av_dict_get(c->metadata, "title", NULL, 0))) {
@@ -1677,12 +1670,34 @@ static int mkv_write_chapters(AVFormatContext *s)
             end_ebml_master(dyn_cp, chapterdisplay);
         }
         end_ebml_master(dyn_cp, chapteratom);
+
+        if (tags && mkv_check_tag(c->metadata, MATROSKA_ID_TAGTARGETS_CHAPTERUID)) {
+            ret = mkv_write_tag(mkv, c->metadata, tags, NULL,
+                                MATROSKA_ID_TAGTARGETS_CHAPTERUID,
+                                (uint32_t)c->id + chapter_id_offset);
+            if (ret < 0)
+                goto fail;
+        }
     }
     end_ebml_master(dyn_cp, editionentry);
     mkv->wrote_chapters = 1;
 
-    return end_ebml_master_crc32(pb, &dyn_cp, mkv,
-                                 MATROSKA_ID_CHAPTERS, 0, 0, 1);
+    ret = end_ebml_master_crc32(pb, &dyn_cp, mkv, MATROSKA_ID_CHAPTERS, 0, 0, 1);
+    if (ret < 0)
+        goto fail;
+    if (dyn_tags)
+        return end_ebml_master_crc32(pb, &dyn_tags, mkv,
+                                     MATROSKA_ID_TAGS, 0, 0, 1);
+    return 0;
+
+fail:
+    if (tags) {
+        /* tags == &mkv->tags.bc can only happen if mkv->tags.bc was
+         * initially NULL, so we never free older tags. */
+        ffio_free_dyn_buf(tags);
+    }
+    ffio_free_dyn_buf(&dyn_cp);
+    return ret;
 }
 
 static const char *get_mimetype(const AVStream *st)
@@ -1878,7 +1893,8 @@ static int mkv_write_header(AVFormatContext *s)
             return ret;
     }
 
-    /* Must come after mkv_write_chapters() because of chapter_id_offset */
+    /* Must come after mkv_write_chapters() to write chapter tags
+     * into the same Tags element as the other tags. */
     ret = mkv_write_tags(s);
     if (ret < 0)
         return ret;
index b69f9792a81a0cf2f73f2400c18811a473a31b6e..17fbf466537dc91b6e53f5c97d426e270eb8d405 100644 (file)
@@ -12,7 +12,7 @@ fate-matroska-prores-header-insertion-bz2: CMD = framecrc -i $(TARGET_SAMPLES)/m
 FATE_MATROSKA-$(call DEMMUX, MATROSKA, MATROSKA) += fate-matroska-remux
 fate-matroska-remux: CMD = md5pipe -i $(TARGET_SAMPLES)/vp9-test-vectors/vp90-2-2pass-akiyo.webm -color_trc 4 -c:v copy -fflags +bitexact -strict -2 -f matroska
 fate-matroska-remux: CMP = oneline
-fate-matroska-remux: REF = acaf96f1832264d7f2845b16dd4ab082
+fate-matroska-remux: REF = 26fabd90326e3de4bb6afe1b216ce232
 
 FATE_MATROSKA-$(call ALLYES, MATROSKA_DEMUXER VORBIS_PARSER) += fate-matroska-xiph-lacing
 fate-matroska-xiph-lacing: CMD = framecrc -i $(TARGET_SAMPLES)/mkv/xiph_lacing.mka -c:a copy
index f98eaf9a0b7484631e09f0c679c3f77614f578c0..96b4d16bff9c8dfbd3258d2af194fba5db46da59 100644 (file)
@@ -91,12 +91,12 @@ fate-wavpack-matroskamode: CMD = md5 -i $(TARGET_SAMPLES)/wavpack/special/matros
 FATE_WAVPACK-$(call DEMMUX, WV, MATROSKA) += fate-wavpack-matroska_mux-mono
 fate-wavpack-matroska_mux-mono: CMD = md5pipe -i $(TARGET_SAMPLES)/wavpack/num_channels/mono_16bit_int.wv -c copy -fflags +bitexact -f matroska
 fate-wavpack-matroska_mux-mono: CMP = oneline
-fate-wavpack-matroska_mux-mono: REF = 5388d452e84958d204de9974bb4e3947
+fate-wavpack-matroska_mux-mono: REF = 0a10b6cd4d64c25ced75f52d1c6929a7
 
 FATE_WAVPACK-$(call DEMMUX, WV, MATROSKA) += fate-wavpack-matroska_mux-61
 fate-wavpack-matroska_mux-61: CMD = md5pipe -i $(TARGET_SAMPLES)/wavpack/num_channels/eva_2.22_6.1_16bit-partial.wv -c copy -fflags +bitexact -f matroska
 fate-wavpack-matroska_mux-61: CMP = oneline
-fate-wavpack-matroska_mux-61: REF = c28c51910fc8f3806b1ada1efc82ccae
+fate-wavpack-matroska_mux-61: REF = fee4bb1d6cf2d54f78eddc4483e7c990
 
 FATE_SAMPLES_AVCONV += $(FATE_WAVPACK-yes)
 fate-wavpack: $(FATE_WAVPACK-yes)
index c7eae5d6c14bb0211ac6da40d6924d0e0cbf3fc5..bfb18aedd24096f1d065b9eabf83747f3fe13fcc 100644 (file)
@@ -1,5 +1,5 @@
-6ffdfc7f11f06f94c22cda3a29bf576b *tests/data/fate/aac-autobsf-adtstoasc.matroska
-6627 tests/data/fate/aac-autobsf-adtstoasc.matroska
+5088977b7f13181d707a4077771e4552 *tests/data/fate/aac-autobsf-adtstoasc.matroska
+6648 tests/data/fate/aac-autobsf-adtstoasc.matroska
 #extradata 0:        2, 0x0030001c
 #tb 0: 1/1000
 #media_type 0: audio
index 48a7a562604821cbe294c01f022dc452b2139587..4740569d9c5ea4509897c66cf0696786771f6273 100644 (file)
@@ -1 +1 @@
-8f355f39124753f89ead698e66f98098
+5706cf33f9ded660eb404275ec136127
index 8b575903e68a20c39e6f3ba7ada835cbb38454d3..fb17bfea0b9e4a90d962b08c9ada23ad1b472b1c 100644 (file)
@@ -1,5 +1,5 @@
-332bf4d9c92d24478d2a218e81223433 *tests/data/fate/matroska-flac-extradata-update.matroska
-2011 tests/data/fate/matroska-flac-extradata-update.matroska
+3c721898cf2cf3e2e6c43ad58952bd2d *tests/data/fate/matroska-flac-extradata-update.matroska
+2032 tests/data/fate/matroska-flac-extradata-update.matroska
 #extradata 0:       34, 0x7acb09e7
 #extradata 1:       34, 0x7acb09e7
 #extradata 2:       34, 0x443402dd
index c9438dffe42674431de9d77190a88d99cf07e731..34d028cbfdda4bbd78f8e90f6f2fbbab339e9e15 100644 (file)
@@ -1,5 +1,5 @@
-84474651e0744e7049e27dd19b8c18a8 *tests/data/fate/rgb24-mkv.matroska
-58192 tests/data/fate/rgb24-mkv.matroska
+fdc02d700dbe99315a9f0d928a9b935e *tests/data/fate/rgb24-mkv.matroska
+58213 tests/data/fate/rgb24-mkv.matroska
 #tb 0: 1/10
 #media_type 0: video
 #codec_id 0: rawvideo
index 62d70cf73d0dfb109babba2da7e257152afa16f0..20ddfc031d66e6301f95a86e282cbc88839d5cf6 100644 (file)
@@ -1,5 +1,5 @@
-eada3550583906e53114d30cb39126f5 *tests/data/fate/webm-dash-chapters.webm
-111199 tests/data/fate/webm-dash-chapters.webm
+e7fde2ecc9683a7a5296dab33b028653 *tests/data/fate/webm-dash-chapters.webm
+111220 tests/data/fate/webm-dash-chapters.webm
 #extradata 0:     3469, 0xc6769ddc
 #tb 0: 1/1000
 #media_type 0: audio
index 3288a4481db3b1266228a6822fdcea98ebb03cf9..bc568a600ed25eee6d125d6227f304ecb480cc24 100644 (file)
@@ -1,3 +1,3 @@
-696074a7600fb5a02a434839d09201f0 *tests/data/lavf-fate/lavf.av1.mkv
-55636 tests/data/lavf-fate/lavf.av1.mkv
+bd676bfc89422755920099ec6ebdebe6 *tests/data/lavf-fate/lavf.av1.mkv
+55657 tests/data/lavf-fate/lavf.av1.mkv
 tests/data/lavf-fate/lavf.av1.mkv CRC=0x7c27cc15
index 303867228c074e007a1ef93d0ac6d4f20a02705a..e66b418b1d54464feab4fd60f9aefe0142962085 100644 (file)
@@ -1,3 +1,3 @@
-a67d0e6113de91ee53ee00b47fa6ff42 *tests/data/lavf/lavf.mka
-43559 tests/data/lavf/lavf.mka
+ea812a6619f4f0d266bec82fcfa54e78 *tests/data/lavf/lavf.mka
+43580 tests/data/lavf/lavf.mka
 tests/data/lavf/lavf.mka CRC=0x3a1da17e
index 9eaf5a4a11ffc91809ab6795943e8699f9487720..0dd4521bc87eed6de3f9d5ee0562a7a0294c2a17 100644 (file)
@@ -1,3 +1,3 @@
-e08b2effe716fb6b72f4a2cd2598b6d4 *tests/data/lavf/lavf.mkv
-320411 tests/data/lavf/lavf.mkv
+c984a76e996f43943d2fe9eb5f2495e4 *tests/data/lavf/lavf.mkv
+320432 tests/data/lavf/lavf.mkv
 tests/data/lavf/lavf.mkv CRC=0xec6c3c68
index d6cd1cdf6490bee8565eb14332128c03d50b339b..e0e1a22d9cfd7e6d238e4b1e10dccb236f2dfd81 100644 (file)
@@ -1,3 +1,3 @@
-aa2419820c590d0512ce199b7b59b9c9 *tests/data/lavf/lavf.mkv_attachment
-472566 tests/data/lavf/lavf.mkv_attachment
+8182124c770de9579e6d0d6759d01655 *tests/data/lavf/lavf.mkv_attachment
+472587 tests/data/lavf/lavf.mkv_attachment
 tests/data/lavf/lavf.mkv_attachment CRC=0xec6c3c68
index 3299861d9cc917f32a5bfa103773adfaa63c0753..8981f1d7b73ce5f5e7822e6f0dd15ce94919f1fd 100644 (file)
@@ -1,48 +1,48 @@
-ret: 0         st: 1 flags:1 dts: 0.000000 pts: 0.000000 pos:    659 size:   208
+ret: 0         st: 1 flags:1 dts: 0.000000 pts: 0.000000 pos:    680 size:   208
 ret: 0         st:-1 flags:0  ts:-1.000000
-ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    875 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    896 size: 27837
 ret: 0         st:-1 flags:1  ts: 1.894167
-ret: 0         st: 0 flags:1 dts: 0.971000 pts: 0.971000 pos: 292291 size: 27834
+ret: 0         st: 0 flags:1 dts: 0.971000 pts: 0.971000 pos: 292312 size: 27834
 ret: 0         st: 0 flags:0  ts: 0.788000
-ret: 0         st: 0 flags:1 dts: 0.971000 pts: 0.971000 pos: 292291 size: 27834
+ret: 0         st: 0 flags:1 dts: 0.971000 pts: 0.971000 pos: 292312 size: 27834
 ret: 0         st: 0 flags:1  ts:-0.317000
-ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    875 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    896 size: 27837
 ret:-1         st: 1 flags:0  ts: 2.577000
 ret: 0         st: 1 flags:1  ts: 1.471000
-ret: 0         st: 1 flags:1 dts: 0.993000 pts: 0.993000 pos: 320132 size:   209
+ret: 0         st: 1 flags:1 dts: 0.993000 pts: 0.993000 pos: 320153 size:   209
 ret: 0         st:-1 flags:0  ts: 0.365002
-ret: 0         st: 0 flags:1 dts: 0.491000 pts: 0.491000 pos: 146843 size: 27925
+ret: 0         st: 0 flags:1 dts: 0.491000 pts: 0.491000 pos: 146864 size: 27925
 ret: 0         st:-1 flags:1  ts:-0.740831
-ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    875 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    896 size: 27837
 ret:-1         st: 0 flags:0  ts: 2.153000
 ret: 0         st: 0 flags:1  ts: 1.048000
-ret: 0         st: 0 flags:1 dts: 0.971000 pts: 0.971000 pos: 292291 size: 27834
+ret: 0         st: 0 flags:1 dts: 0.971000 pts: 0.971000 pos: 292312 size: 27834
 ret: 0         st: 1 flags:0  ts:-0.058000
-ret: 0         st: 1 flags:1 dts: 0.000000 pts: 0.000000 pos:    659 size:   208
+ret: 0         st: 1 flags:1 dts: 0.000000 pts: 0.000000 pos:    680 size:   208
 ret: 0         st: 1 flags:1  ts: 2.836000
-ret: 0         st: 1 flags:1 dts: 0.993000 pts: 0.993000 pos: 320132 size:   209
+ret: 0         st: 1 flags:1 dts: 0.993000 pts: 0.993000 pos: 320153 size:   209
 ret:-1         st:-1 flags:0  ts: 1.730004
 ret: 0         st:-1 flags:1  ts: 0.624171
-ret: 0         st: 0 flags:1 dts: 0.491000 pts: 0.491000 pos: 146843 size: 27925
+ret: 0         st: 0 flags:1 dts: 0.491000 pts: 0.491000 pos: 146864 size: 27925
 ret: 0         st: 0 flags:0  ts:-0.482000
-ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    875 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    896 size: 27837
 ret: 0         st: 0 flags:1  ts: 2.413000
-ret: 0         st: 0 flags:1 dts: 0.971000 pts: 0.971000 pos: 292291 size: 27834
+ret: 0         st: 0 flags:1 dts: 0.971000 pts: 0.971000 pos: 292312 size: 27834
 ret:-1         st: 1 flags:0  ts: 1.307000
 ret: 0         st: 1 flags:1  ts: 0.201000
-ret: 0         st: 1 flags:1 dts: 0.000000 pts: 0.000000 pos:    659 size:   208
+ret: 0         st: 1 flags:1 dts: 0.000000 pts: 0.000000 pos:    680 size:   208
 ret: 0         st:-1 flags:0  ts:-0.904994
-ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    875 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    896 size: 27837
 ret: 0         st:-1 flags:1  ts: 1.989173
-ret: 0         st: 0 flags:1 dts: 0.971000 pts: 0.971000 pos: 292291 size: 27834
+ret: 0         st: 0 flags:1 dts: 0.971000 pts: 0.971000 pos: 292312 size: 27834
 ret: 0         st: 0 flags:0  ts: 0.883000
-ret: 0         st: 0 flags:1 dts: 0.971000 pts: 0.971000 pos: 292291 size: 27834
+ret: 0         st: 0 flags:1 dts: 0.971000 pts: 0.971000 pos: 292312 size: 27834
 ret: 0         st: 0 flags:1  ts:-0.222000
-ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    875 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    896 size: 27837
 ret:-1         st: 1 flags:0  ts: 2.672000
 ret: 0         st: 1 flags:1  ts: 1.566000
-ret: 0         st: 1 flags:1 dts: 0.993000 pts: 0.993000 pos: 320132 size:   209
+ret: 0         st: 1 flags:1 dts: 0.993000 pts: 0.993000 pos: 320153 size:   209
 ret: 0         st:-1 flags:0  ts: 0.460008
-ret: 0         st: 0 flags:1 dts: 0.491000 pts: 0.491000 pos: 146843 size: 27925
+ret: 0         st: 0 flags:1 dts: 0.491000 pts: 0.491000 pos: 146864 size: 27925
 ret: 0         st:-1 flags:1  ts:-0.645825
-ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    875 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    896 size: 27837