diff mbox

[FFmpeg-devel] lavf/matroskaenc: use mkv_check_tag_name consistently

Message ID 20160906221352.32102-1-rodger.combs@gmail.com
State Accepted
Headers show

Commit Message

Rodger Combs Sept. 6, 2016, 10:13 p.m. UTC 
Previously, we used a different list of checks when deciding whether to
write a set of tags at all than we did when deciding whether to write an
individual tag in the set. This resulted in sometimes writing an empty
tag master and seekhead. Now we use mkv_check_tag_name everywhere, so
if a dictionary is entirely composed of tags we skip, we don't write a
tag master at all.

This affected the test file, since "language" was on one list but not
the other, so we were writing an empty tag master there. The test hash
is updated to reflect that change.
---
 libavformat/matroskaenc.c | 10 +++++-----
 tests/fate/matroska.mak   |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

Comments

Michael Niedermayer Sept. 6, 2016, 10:19 p.m. UTC | #1 
On Tue, Sep 06, 2016 at 05:13:52PM -0500, Rodger Combs wrote:
> Previously, we used a different list of checks when deciding whether to
> write a set of tags at all than we did when deciding whether to write an
> individual tag in the set. This resulted in sometimes writing an empty
> tag master and seekhead. Now we use mkv_check_tag_name everywhere, so
> if a dictionary is entirely composed of tags we skip, we don't write a
> tag master at all.
> 
> This affected the test file, since "language" was on one list but not
> the other, so we were writing an empty tag master there. The test hash
> is updated to reflect that change.
> ---
>  libavformat/matroskaenc.c | 10 +++++-----
>  tests/fate/matroska.mak   |  2 +-
>  2 files changed, 6 insertions(+), 6 deletions(-)

excelent commit message

LGTM

thx

[...]
diff mbox

Patch

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 7deccaa..3eeb09b 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -1342,12 +1342,12 @@  static int mkv_write_tag(AVFormatContext *s, AVDictionary *m, unsigned int eleme
     return 0;
 }
 
-static int mkv_check_tag(AVDictionary *m)
+static int mkv_check_tag(AVDictionary *m, unsigned int elementid)
 {
     AVDictionaryEntry *t = NULL;
 
     while ((t = av_dict_get(m, "", t, AV_DICT_IGNORE_SUFFIX)))
-        if (av_strcasecmp(t->key, "title") && av_strcasecmp(t->key, "stereo_mode"))
+        if (mkv_check_tag_name(t->key, elementid))
             return 1;
 
     return 0;
@@ -1361,7 +1361,7 @@  static int mkv_write_tags(AVFormatContext *s)
 
     ff_metadata_conv_ctx(s, ff_mkv_metadata_conv, NULL);
 
-    if (mkv_check_tag(s->metadata)) {
+    if (mkv_check_tag(s->metadata, 0)) {
         ret = mkv_write_tag(s, s->metadata, 0, 0, &tags);
         if (ret < 0) return ret;
     }
@@ -1369,7 +1369,7 @@  static int mkv_write_tags(AVFormatContext *s)
     for (i = 0; i < s->nb_streams; i++) {
         AVStream *st = s->streams[i];
 
-        if (!mkv_check_tag(st->metadata))
+        if (!mkv_check_tag(st->metadata, MATROSKA_ID_TAGTARGETS_TRACKUID))
             continue;
 
         ret = mkv_write_tag(s, st->metadata, MATROSKA_ID_TAGTARGETS_TRACKUID, i + 1, &tags);
@@ -1398,7 +1398,7 @@  static int mkv_write_tags(AVFormatContext *s)
     for (i = 0; i < s->nb_chapters; i++) {
         AVChapter *ch = s->chapters[i];
 
-        if (!mkv_check_tag(ch->metadata))
+        if (!mkv_check_tag(ch->metadata, MATROSKA_ID_TAGTARGETS_CHAPTERUID))
             continue;
 
         ret = mkv_write_tag(s, ch->metadata, MATROSKA_ID_TAGTARGETS_CHAPTERUID, ch->id + mkv->chapter_id_offset, &tags);
diff --git a/tests/fate/matroska.mak b/tests/fate/matroska.mak
index 8cf1734..8e4a1e8 100644
--- a/tests/fate/matroska.mak
+++ b/tests/fate/matroska.mak
@@ -4,6 +4,6 @@ 
 FATE_MATROSKA-$(call DEMMUX, MATROSKA, MATROSKA) += fate-matroska-remux
 fate-matroska-remux: CMD = md5 -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 = 5ebcfaa8e3d534f8a800a58fd2b0aca6
+fate-matroska-remux: REF = f08b20b90f158a4de5a02a52c25596b9
 
 FATE_SAMPLES_AVCONV += $(FATE_MATROSKA-yes)