diff mbox series

[FFmpeg-devel,02/20] avformat/matroskaenc: Don't waste bytes writing durations

Message ID 20200405155928.9323-3-andreas.rheinhardt@gmail.com
State Accepted
Commit bd7dea3f4f185a32966d7542837030d843c8518d
Headers show
Series Matroska muxer patches | expand

Checks

Context Check Description
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Andreas Rheinhardt April 5, 2020, 3:59 p.m. UTC 
Tags in the Matroska file format can be summarized as follows: There is
a level 1-element called Tags containing one or many Tag elements each
of which in turn contain a Targets element and one or many SimpleTags.
Each SimpleTag roughly corresponds to a single key-value pair similar to
an AVDictionaryEntry. The Targets meanwhile contains information to what
the metadata contained in the SimpleTags contained in the containing Tag
applies (i.e. to the file as a whole or to an individual track).

The Matroska muxer writes such metadata. It puts the metadata of every
stream into a Tag whose Targets makes it point to the corresponding
track. And if the output is seekable, then it also adds another Tag for
each track whose Targets corresponds to the track and where it reserves
space in a SimpleTag to write the duration at the end of the muxing
process into.

Yet there is no reason to write two Tag elements for a track and a few
bytes (typically 24 bytes per track) can be saved by adding the duration
SimpleTag to the other Tag of the same track (if it exists).

FATE has been updated because the output files changed. (Tests that
write to unseekable output (pipes) needn't be updated (no duration tag
has ever been written for them) and the same applies to tests without
further metadata.)

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
This one is new.

 libavformat/matroskaenc.c                     | 48 ++++++++-----------
 tests/ref/fate/matroska-flac-extradata-update |  4 +-
 tests/ref/fate/rgb24-mkv                      |  4 +-
 tests/ref/lavf/mka                            |  4 +-
 tests/ref/lavf/mkv                            |  4 +-
 tests/ref/lavf/mkv_attachment                 |  4 +-
 tests/ref/seek/lavf-mkv                       | 44 ++++++++---------
 7 files changed, 53 insertions(+), 59 deletions(-)
diff mbox series

Patch

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index a377d092df..96d8c012ad 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -1519,14 +1519,14 @@  static int mkv_check_tag_name(const char *name, uint32_t elementid)
 }
 
 static int mkv_write_tag(AVFormatContext *s, AVDictionary *m, uint32_t elementid,
-                         uint64_t uid)
+                         uint64_t uid, ebml_master *tag)
 {
     MatroskaMuxContext *mkv = s->priv_data;
-    ebml_master tag;
+    ebml_master tag2;
     int ret;
     AVDictionaryEntry *t = NULL;
 
-    ret = mkv_write_tag_targets(s, elementid, uid, &tag);
+    ret = mkv_write_tag_targets(s, elementid, uid, tag ? tag : &tag2);
     if (ret < 0)
         return ret;
 
@@ -1538,7 +1538,9 @@  static int mkv_write_tag(AVFormatContext *s, AVDictionary *m, uint32_t elementid
         }
     }
 
-    end_ebml_master(mkv->tags_bc, tag);
+    if (!tag)
+        end_ebml_master(mkv->tags_bc, tag2);
+
     return 0;
 }
 
@@ -1556,53 +1558,43 @@  static int mkv_check_tag(AVDictionary *m, uint32_t elementid)
 static int mkv_write_tags(AVFormatContext *s)
 {
     MatroskaMuxContext *mkv = s->priv_data;
+    ebml_master tag, *tagp;
     int i, ret;
 
     ff_metadata_conv_ctx(s, ff_mkv_metadata_conv, NULL);
 
     if (mkv_check_tag(s->metadata, 0)) {
-        ret = mkv_write_tag(s, s->metadata, 0, 0);
+        ret = mkv_write_tag(s, s->metadata, 0, 0, NULL);
         if (ret < 0) return ret;
     }
 
+    tagp = (s->pb->seekable & AVIO_SEEKABLE_NORMAL) && !mkv->is_live ? &tag : NULL;
     for (i = 0; i < s->nb_streams; i++) {
         AVStream *st = s->streams[i];
 
         if (st->codecpar->codec_type == AVMEDIA_TYPE_ATTACHMENT)
             continue;
 
-        if (!mkv_check_tag(st->metadata, MATROSKA_ID_TAGTARGETS_TRACKUID))
+        if (!tagp && !mkv_check_tag(st->metadata, MATROSKA_ID_TAGTARGETS_TRACKUID))
             continue;
 
-        ret = mkv_write_tag(s, st->metadata, MATROSKA_ID_TAGTARGETS_TRACKUID, i + 1);
+        ret = mkv_write_tag(s, st->metadata, MATROSKA_ID_TAGTARGETS_TRACKUID,
+                            i + 1, tagp);
         if (ret < 0) return ret;
-    }
-
-    if ((s->pb->seekable & AVIO_SEEKABLE_NORMAL) && !mkv->is_live) {
-        for (i = 0; i < s->nb_streams; i++) {
-            AVIOContext *pb;
-            AVStream *st = s->streams[i];
-            ebml_master tag_target;
-            ebml_master tag;
 
-            if (st->codecpar->codec_type == AVMEDIA_TYPE_ATTACHMENT)
-                continue;
-
-            ret = mkv_write_tag_targets(s, MATROSKA_ID_TAGTARGETS_TRACKUID,
-                                        i + 1, &tag_target);
-            if (ret < 0)
-                return ret;
-            pb = mkv->tags_bc;
+        if (tagp) {
+            AVIOContext *pb = mkv->tags_bc;
+            ebml_master simpletag;
 
-            tag = start_ebml_master(pb, MATROSKA_ID_SIMPLETAG, 0);
+            simpletag = start_ebml_master(pb, MATROSKA_ID_SIMPLETAG, 0);
             put_ebml_string(pb, MATROSKA_ID_TAGNAME, "DURATION");
             mkv->tracks[i].duration_offset = avio_tell(pb);
 
             // Reserve space to write duration as a 20-byte string.
             // 2 (ebml id) + 1 (data size) + 20 (data)
             put_ebml_void(pb, 23);
+            end_ebml_master(pb, simpletag);
             end_ebml_master(pb, tag);
-            end_ebml_master(pb, tag_target);
         }
     }
 
@@ -1614,7 +1606,8 @@  static int mkv_write_tags(AVFormatContext *s)
                 continue;
 
             ret = mkv_write_tag(s, ch->metadata, MATROSKA_ID_TAGTARGETS_CHAPTERUID,
-                                (uint32_t)ch->id + (uint64_t)mkv->chapter_id_offset);
+                                (uint32_t)ch->id + (uint64_t)mkv->chapter_id_offset,
+                                NULL);
             if (ret < 0)
                 return ret;
         }
@@ -1628,7 +1621,8 @@  static int mkv_write_tags(AVFormatContext *s)
             if (!mkv_check_tag(st->metadata, MATROSKA_ID_TAGTARGETS_ATTACHUID))
                 continue;
 
-            ret = mkv_write_tag(s, st->metadata, MATROSKA_ID_TAGTARGETS_ATTACHUID, attachment->fileuid);
+            ret = mkv_write_tag(s, st->metadata, MATROSKA_ID_TAGTARGETS_ATTACHUID,
+                                attachment->fileuid, NULL);
             if (ret < 0)
                 return ret;
         }
diff --git a/tests/ref/fate/matroska-flac-extradata-update b/tests/ref/fate/matroska-flac-extradata-update
index 743486c974..f274e1a75d 100644
--- a/tests/ref/fate/matroska-flac-extradata-update
+++ b/tests/ref/fate/matroska-flac-extradata-update
@@ -1,5 +1,5 @@ 
-2b363aecd6422c3ce9964983f5d0f3ef *tests/data/fate/matroska-flac-extradata-update.matroska
-2049 tests/data/fate/matroska-flac-extradata-update.matroska
+5070c157123b54e218c7e0a45d5606c8 *tests/data/fate/matroska-flac-extradata-update.matroska
+1977 tests/data/fate/matroska-flac-extradata-update.matroska
 #extradata 0:       34, 0x7acb09e7
 #extradata 1:       34, 0x7acb09e7
 #extradata 2:       34, 0x443402dd
diff --git a/tests/ref/fate/rgb24-mkv b/tests/ref/fate/rgb24-mkv
index 43b4ee2e19..686c45ebfb 100644
--- a/tests/ref/fate/rgb24-mkv
+++ b/tests/ref/fate/rgb24-mkv
@@ -1,5 +1,5 @@ 
-92861b36a8220a3dae10d91cba614892 *tests/data/fate/rgb24-mkv.matroska
-58216 tests/data/fate/rgb24-mkv.matroska
+f9feab2a7ed8f1049df1b70982291505 *tests/data/fate/rgb24-mkv.matroska
+58192 tests/data/fate/rgb24-mkv.matroska
 #tb 0: 1/10
 #media_type 0: video
 #codec_id 0: rawvideo
diff --git a/tests/ref/lavf/mka b/tests/ref/lavf/mka
index e8239442e9..f212097bac 100644
--- a/tests/ref/lavf/mka
+++ b/tests/ref/lavf/mka
@@ -1,3 +1,3 @@ 
-58936d3ad4eb1372bd2e5845963fe17c *tests/data/lavf/lavf.mka
-43563 tests/data/lavf/lavf.mka
+9bd06ae5291de75c19a60c3f39db4c09 *tests/data/lavf/lavf.mka
+43538 tests/data/lavf/lavf.mka
 tests/data/lavf/lavf.mka CRC=0x3a1da17e
diff --git a/tests/ref/lavf/mkv b/tests/ref/lavf/mkv
index 4d73fbf39d..04b554bb56 100644
--- a/tests/ref/lavf/mkv
+++ b/tests/ref/lavf/mkv
@@ -1,3 +1,3 @@ 
-6ec6d6a058a85bf616e0b981bb8e383f *tests/data/lavf/lavf.mkv
-320466 tests/data/lavf/lavf.mkv
+85ef0666b3ad642221793b1c4f985011 *tests/data/lavf/lavf.mkv
+320418 tests/data/lavf/lavf.mkv
 tests/data/lavf/lavf.mkv CRC=0xec6c3c68
diff --git a/tests/ref/lavf/mkv_attachment b/tests/ref/lavf/mkv_attachment
index 2e9711483f..56b1078d4a 100644
--- a/tests/ref/lavf/mkv_attachment
+++ b/tests/ref/lavf/mkv_attachment
@@ -1,3 +1,3 @@ 
-41b4f132ada6e0faedf92e909526a899 *tests/data/lavf/lavf.mkv_attachment
-472617 tests/data/lavf/lavf.mkv_attachment
+c339e42793d2a3cb1b8e99fc7a60de31 *tests/data/lavf/lavf.mkv_attachment
+472569 tests/data/lavf/lavf.mkv_attachment
 tests/data/lavf/lavf.mkv_attachment CRC=0xec6c3c68
diff --git a/tests/ref/seek/lavf-mkv b/tests/ref/seek/lavf-mkv
index 223e098829..a3dce35d86 100644
--- a/tests/ref/seek/lavf-mkv
+++ b/tests/ref/seek/lavf-mkv
@@ -1,48 +1,48 @@ 
-ret: 0         st: 1 flags:1 dts: 0.000000 pts: 0.000000 pos:    714 size:   208
+ret: 0         st: 1 flags:1 dts: 0.000000 pts: 0.000000 pos:    666 size:   208
 ret: 0         st:-1 flags:0  ts:-1.000000
-ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    930 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    882 size: 27837
 ret: 0         st:-1 flags:1  ts: 1.894167
-ret: 0         st: 0 flags:1 dts: 0.971000 pts: 0.971000 pos: 292346 size: 27834
+ret: 0         st: 0 flags:1 dts: 0.971000 pts: 0.971000 pos: 292298 size: 27834
 ret: 0         st: 0 flags:0  ts: 0.788000
-ret: 0         st: 0 flags:1 dts: 0.971000 pts: 0.971000 pos: 292346 size: 27834
+ret: 0         st: 0 flags:1 dts: 0.971000 pts: 0.971000 pos: 292298 size: 27834
 ret: 0         st: 0 flags:1  ts:-0.317000
-ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    930 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    882 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: 320187 size:   209
+ret: 0         st: 1 flags:1 dts: 0.993000 pts: 0.993000 pos: 320139 size:   209
 ret: 0         st:-1 flags:0  ts: 0.365002
-ret: 0         st: 0 flags:1 dts: 0.491000 pts: 0.491000 pos: 146898 size: 27925
+ret: 0         st: 0 flags:1 dts: 0.491000 pts: 0.491000 pos: 146850 size: 27925
 ret: 0         st:-1 flags:1  ts:-0.740831
-ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    930 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    882 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: 292346 size: 27834
+ret: 0         st: 0 flags:1 dts: 0.971000 pts: 0.971000 pos: 292298 size: 27834
 ret: 0         st: 1 flags:0  ts:-0.058000
-ret: 0         st: 1 flags:1 dts: 0.000000 pts: 0.000000 pos:    714 size:   208
+ret: 0         st: 1 flags:1 dts: 0.000000 pts: 0.000000 pos:    666 size:   208
 ret: 0         st: 1 flags:1  ts: 2.836000
-ret: 0         st: 1 flags:1 dts: 0.993000 pts: 0.993000 pos: 320187 size:   209
+ret: 0         st: 1 flags:1 dts: 0.993000 pts: 0.993000 pos: 320139 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: 146898 size: 27925
+ret: 0         st: 0 flags:1 dts: 0.491000 pts: 0.491000 pos: 146850 size: 27925
 ret: 0         st: 0 flags:0  ts:-0.482000
-ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    930 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    882 size: 27837
 ret: 0         st: 0 flags:1  ts: 2.413000
-ret: 0         st: 0 flags:1 dts: 0.971000 pts: 0.971000 pos: 292346 size: 27834
+ret: 0         st: 0 flags:1 dts: 0.971000 pts: 0.971000 pos: 292298 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:    714 size:   208
+ret: 0         st: 1 flags:1 dts: 0.000000 pts: 0.000000 pos:    666 size:   208
 ret: 0         st:-1 flags:0  ts:-0.904994
-ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    930 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    882 size: 27837
 ret: 0         st:-1 flags:1  ts: 1.989173
-ret: 0         st: 0 flags:1 dts: 0.971000 pts: 0.971000 pos: 292346 size: 27834
+ret: 0         st: 0 flags:1 dts: 0.971000 pts: 0.971000 pos: 292298 size: 27834
 ret: 0         st: 0 flags:0  ts: 0.883000
-ret: 0         st: 0 flags:1 dts: 0.971000 pts: 0.971000 pos: 292346 size: 27834
+ret: 0         st: 0 flags:1 dts: 0.971000 pts: 0.971000 pos: 292298 size: 27834
 ret: 0         st: 0 flags:1  ts:-0.222000
-ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    930 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    882 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: 320187 size:   209
+ret: 0         st: 1 flags:1 dts: 0.993000 pts: 0.993000 pos: 320139 size:   209
 ret: 0         st:-1 flags:0  ts: 0.460008
-ret: 0         st: 0 flags:1 dts: 0.491000 pts: 0.491000 pos: 146898 size: 27925
+ret: 0         st: 0 flags:1 dts: 0.491000 pts: 0.491000 pos: 146850 size: 27925
 ret: 0         st:-1 flags:1  ts:-0.645825
-ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    930 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.011000 pts: 0.011000 pos:    882 size: 27837