diff mbox

[FFmpeg-devel] avformat/matroskaenc: fix targets for attachment tags

Message ID 20161004000533.5284-1-jamrial@gmail.com
State Superseded
Headers show

Commit Message

James Almer Oct. 4, 2016, 12:05 a.m. UTC
Attachment tags were being written targetting non-existent streams in the
output file.
Also filter "filename" and "mimetype" entries, as they are standard elements
in the Attachment master.

Signed-off-by: James Almer <jamrial@gmail.com>
---
The fileuid is changed to be four bytes long instead of eight, because the
mkv_write_tag*() functions pass that value as unsigned int.
If preferred i can change them to pass uint64_t and keep the current huge
fileuids.

 libavformat/matroskaenc.c | 66 ++++++++++++++++++++++++++++++++++++++++++-----
 tests/ref/lavf/mkv        |  4 +--
 2 files changed, 61 insertions(+), 9 deletions(-)
diff mbox

Patch

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 3eeb09b..4561db7 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -95,6 +95,16 @@  typedef struct mkv_track {
     int64_t         ts_offset;
 } mkv_track;
 
+typedef struct mkv_attachment {
+    int             stream_idx;
+    uint32_t        fileuid;
+} mkv_attachment;
+
+typedef struct mkv_attachments {
+    mkv_attachment  *entries;
+    int             num_entries;
+} mkv_attachments;
+
 #define MODE_MATROSKAv2 0x01
 #define MODE_WEBM       0x02
 
@@ -116,6 +126,7 @@  typedef struct MatroskaMuxContext {
     mkv_seekhead    *main_seekhead;
     mkv_cues        *cues;
     mkv_track       *tracks;
+    mkv_attachments *attachments;
 
     AVPacket        cur_audio_pkt;
 
@@ -323,6 +334,10 @@  static void mkv_free(MatroskaMuxContext *mkv) {
         av_freep(&mkv->cues->entries);
         av_freep(&mkv->cues);
     }
+    if (mkv->attachments) {
+        av_freep(&mkv->attachments->entries);
+        av_freep(&mkv->attachments);
+    }
     av_freep(&mkv->tracks);
     av_freep(&mkv->stream_durations);
     av_freep(&mkv->stream_duration_offsets);
@@ -1316,7 +1331,10 @@  static int mkv_check_tag_name(const char *name, unsigned int elementid)
            av_strcasecmp(name, "encoding_tool") &&
            av_strcasecmp(name, "duration") &&
            (elementid != MATROSKA_ID_TAGTARGETS_TRACKUID ||
-            av_strcasecmp(name, "language"));
+            av_strcasecmp(name, "language")) &&
+           (elementid != MATROSKA_ID_TAGTARGETS_ATTACHUID ||
+            (av_strcasecmp(name, "filename") &&
+             av_strcasecmp(name, "mimetype")));
 }
 
 static int mkv_write_tag(AVFormatContext *s, AVDictionary *m, unsigned int elementid,
@@ -1369,6 +1387,9 @@  static int mkv_write_tags(AVFormatContext *s)
     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))
             continue;
 
@@ -1378,9 +1399,13 @@  static int mkv_write_tags(AVFormatContext *s)
 
     if (!mkv->is_live) {
         for (i = 0; i < s->nb_streams; i++) {
+            AVStream *st = s->streams[i];
             ebml_master tag_target;
             ebml_master tag;
 
+            if (st->codecpar->codec_type == AVMEDIA_TYPE_ATTACHMENT)
+                continue;
+
             mkv_write_tag_targets(s, MATROSKA_ID_TAGTARGETS_TRACKUID, i + 1, &tags, &tag_target);
 
             tag = start_ebml_master(s->pb, MATROSKA_ID_SIMPLETAG, 0);
@@ -1405,6 +1430,20 @@  static int mkv_write_tags(AVFormatContext *s)
         if (ret < 0) return ret;
     }
 
+    if (mkv->have_attachments) {
+        for (i = 0; i < mkv->attachments->num_entries; i++) {
+            mkv_attachment *attachment = &mkv->attachments->entries[i];
+            AVStream *st = s->streams[attachment->stream_idx];
+
+            if (!mkv_check_tag(st->metadata, MATROSKA_ID_TAGTARGETS_ATTACHUID))
+                continue;
+
+            ret = mkv_write_tag(s, st->metadata, MATROSKA_ID_TAGTARGETS_ATTACHUID, attachment->fileuid, &tags);
+            if (ret < 0)
+                return ret;
+        }
+    }
+
     if (tags.pos)
         end_ebml_master(s->pb, tags);
     return 0;
@@ -1421,6 +1460,10 @@  static int mkv_write_attachments(AVFormatContext *s)
     if (!mkv->have_attachments)
         return 0;
 
+    mkv->attachments = av_mallocz(sizeof(*mkv->attachments));
+    if (!mkv->attachments)
+        return ret;
+
     av_lfg_init(&c, av_get_random_seed());
 
     ret = mkv_add_seekhead_entry(mkv->main_seekhead, MATROSKA_ID_ATTACHMENTS, avio_tell(pb));
@@ -1431,13 +1474,19 @@  static int mkv_write_attachments(AVFormatContext *s)
     for (i = 0; i < s->nb_streams; i++) {
         AVStream *st = s->streams[i];
         ebml_master attached_file;
+        mkv_attachment *attachment = mkv->attachments->entries;
         AVDictionaryEntry *t;
         const char *mimetype = NULL;
-        uint64_t fileuid;
+        uint32_t fileuid;
 
         if (st->codecpar->codec_type != AVMEDIA_TYPE_ATTACHMENT)
             continue;
 
+        attachment = av_realloc_array(attachment, mkv->attachments->num_entries + 1, sizeof(mkv_attachment));
+        if (!attachment)
+            return AVERROR(ENOMEM);
+        mkv->attachments->entries = attachment;
+
         attached_file = start_ebml_master(pb, MATROSKA_ID_ATTACHEDFILE, 0);
 
         if (t = av_dict_get(st->metadata, "title", NULL, 0))
@@ -1477,17 +1526,20 @@  static int mkv_write_attachments(AVFormatContext *s)
             av_sha_update(sha, st->codecpar->extradata, st->codecpar->extradata_size);
             av_sha_final(sha, digest);
             av_free(sha);
-            fileuid = AV_RL64(digest);
+            fileuid = AV_RL32(digest);
         } else {
             fileuid = av_lfg_get(&c);
         }
-        av_log(s, AV_LOG_VERBOSE, "Using %.16"PRIx64" for attachment %d\n",
-               fileuid, i);
+        av_log(s, AV_LOG_VERBOSE, "Using %.8"PRIx32" for attachment %d\n",
+               fileuid, mkv->attachments->num_entries);
 
         put_ebml_string(pb, MATROSKA_ID_FILEMIMETYPE, mimetype);
         put_ebml_binary(pb, MATROSKA_ID_FILEDATA, st->codecpar->extradata, st->codecpar->extradata_size);
         put_ebml_uint(pb, MATROSKA_ID_FILEUID, fileuid);
         end_ebml_master(pb, attached_file);
+
+        mkv->attachments->entries[mkv->attachments->num_entries].stream_idx = i;
+        mkv->attachments->entries[mkv->attachments->num_entries++].fileuid  = fileuid;
     }
     end_ebml_master(pb, attachments);
 
@@ -1659,11 +1711,11 @@  static int mkv_write_header(AVFormatContext *s)
         if (ret < 0)
             goto fail;
 
-        ret = mkv_write_tags(s);
+        ret = mkv_write_attachments(s);
         if (ret < 0)
             goto fail;
 
-        ret = mkv_write_attachments(s);
+        ret = mkv_write_tags(s);
         if (ret < 0)
             goto fail;
     }
diff --git a/tests/ref/lavf/mkv b/tests/ref/lavf/mkv
index 5b0d386..1fa591a 100644
--- a/tests/ref/lavf/mkv
+++ b/tests/ref/lavf/mkv
@@ -1,5 +1,5 @@ 
-5b982c8dfbadc71f51b206cbd10b9a71 *./tests/data/lavf/lavf.mkv
-472875 ./tests/data/lavf/lavf.mkv
+6ba4c26f1c59cbe9be0f23e965b5df97 *./tests/data/lavf/lavf.mkv
+472701 ./tests/data/lavf/lavf.mkv
 ./tests/data/lavf/lavf.mkv CRC=0xec6c3c68
 b4a295bae8e6cf536563cb840386f3a4 *./tests/data/lavf/lavf.mkv
 320551 ./tests/data/lavf/lavf.mkv