matroskadec: export cover art correctly

Generally, libavformat exports cover art pictures as video streams with
1 packet and AV_DISPOSITION_ATTACHED_PIC set. Only matroskadec exported
it as attachment with codec_id set to AV_CODEC_ID_MJPEG.

Obviously, this should be consistent, so change the Matroska demuxer to
export a AV_DISPOSITION_ATTACHED_PIC pseudo video stream.

Matroska muxing is probably incorrect too. I know that it can create
broken files with an audio track and just 1 video frame when e.g.
remuxing mp3 with APIC to mkv. But for now this commit does not change
anything about muxing, and also continues to write attachments with
AV_CODEC_ID_MJPEG should the muxer application have special knowledge
that the Matroska is broken in this way.

Fixes trac #4423.

Signed-off-by: Michael Niedermayer <michaelni@gmx.at>

4 files changed, 44 insertions, 12 deletions

diff --git a/libavformat/matroska.c b/libavformat/matroska.c
index bc5007a433..faa662d09f 100644
--- a/libavformat/matroska.c
+++ b/libavformat/matroska.c
@@ -99,12 +99,17 @@ const CodecTags ff_mkv_codec_tags[]={
     {""                 , AV_CODEC_ID_NONE}
 };
 
-const CodecMime ff_mkv_mime_tags[] = {
-    {"text/plain"                 , AV_CODEC_ID_TEXT},
+const CodecMime ff_mkv_image_mime_tags[] = {
     {"image/gif"                  , AV_CODEC_ID_GIF},
     {"image/jpeg"                 , AV_CODEC_ID_MJPEG},
     {"image/png"                  , AV_CODEC_ID_PNG},
     {"image/tiff"                 , AV_CODEC_ID_TIFF},
+
+    {""                           , AV_CODEC_ID_NONE}
+};
+
+const CodecMime ff_mkv_mime_tags[] = {
+    {"text/plain"                 , AV_CODEC_ID_TEXT},
     {"application/x-truetype-font", AV_CODEC_ID_TTF},
     {"application/x-font"         , AV_CODEC_ID_TTF},
     {"application/vnd.ms-opentype", AV_CODEC_ID_OTF},
diff --git a/libavformat/matroska.h b/libavformat/matroska.h
index 391c56c0aa..344b2c32fa 100644
--- a/libavformat/matroska.h
+++ b/libavformat/matroska.h
@@ -280,6 +280,7 @@ typedef struct CodecTags{
 
 extern const CodecTags ff_mkv_codec_tags[];
 extern const CodecMime ff_mkv_mime_tags[];
+extern const CodecMime ff_mkv_image_mime_tags[];
 extern const AVMetadataConv ff_mkv_metadata_conv[];
 extern const char * const ff_matroska_video_stereo_mode[MATROSKA_VIDEO_STEREOMODE_TYPE_NB];
 extern const char * const ff_matroska_video_stereo_plane[MATROSKA_VIDEO_STEREO_PLANE_COUNT];
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index cad207e228..dd0bdceb3d 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -2142,20 +2142,41 @@ static int matroska_read_header(AVFormatContext *s)
             av_dict_set(&st->metadata, "filename", attachments[j].filename, 0);
             av_dict_set(&st->metadata, "mimetype", attachments[j].mime, 0);
             st->codec->codec_id   = AV_CODEC_ID_NONE;
-            st->codec->codec_type = AVMEDIA_TYPE_ATTACHMENT;
-            if (ff_alloc_extradata(st->codec, attachments[j].bin.size))
-                break;
-            memcpy(st->codec->extradata, attachments[j].bin.data,
-                   attachments[j].bin.size);
 
-            for (i = 0; ff_mkv_mime_tags[i].id != AV_CODEC_ID_NONE; i++) {
-                if (!strncmp(ff_mkv_mime_tags[i].str, attachments[j].mime,
-                             strlen(ff_mkv_mime_tags[i].str))) {
-                    st->codec->codec_id = ff_mkv_mime_tags[i].id;
+            for (i = 0; ff_mkv_image_mime_tags[i].id != AV_CODEC_ID_NONE; i++) {
+                if (!strncmp(ff_mkv_image_mime_tags[i].str, attachments[j].mime,
+                             strlen(ff_mkv_image_mime_tags[i].str))) {
+                    st->codec->codec_id = ff_mkv_image_mime_tags[i].id;
+                    break;
+                }
+            }
+
+            if (st->codec->codec_id != AV_CODEC_ID_NONE) {
+                st->disposition      |= AV_DISPOSITION_ATTACHED_PIC;
+                st->codec->codec_type = AVMEDIA_TYPE_VIDEO;
+
+                av_init_packet(&st->attached_pic);
+                if ((res = av_new_packet(&st->attached_pic, attachments[j].bin.size)) < 0)
+                    return res;
+                memcpy(st->attached_pic.data, attachments[j].bin.data, attachments[j].bin.size);
+                st->attached_pic.stream_index = st->index;
+                st->attached_pic.flags       |= AV_PKT_FLAG_KEY;
+            } else {
+                st->codec->codec_type = AVMEDIA_TYPE_ATTACHMENT;
+                if (ff_alloc_extradata(st->codec, attachments[j].bin.size))
                     break;
+                memcpy(st->codec->extradata, attachments[j].bin.data,
+                    attachments[j].bin.size);
+
+                for (i = 0; ff_mkv_mime_tags[i].id != AV_CODEC_ID_NONE; i++) {
+                    if (!strncmp(ff_mkv_mime_tags[i].str, attachments[j].mime,
+                                strlen(ff_mkv_mime_tags[i].str))) {
+                        st->codec->codec_id = ff_mkv_mime_tags[i].id;
+                        break;
+                    }
                 }
+                attachments[j].stream = st;
             }
-            attachments[j].stream = st;
         }
     }
 
diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 6b2e390e90..5493e3c631 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -1270,6 +1270,11 @@ static int mkv_write_attachments(AVFormatContext *s)
                     mimetype = ff_mkv_mime_tags[i].str;
                     break;
                 }
+            for (i = 0; ff_mkv_image_mime_tags[i].id != AV_CODEC_ID_NONE; i++)
+                if (ff_mkv_image_mime_tags[i].id == st->codec->codec_id) {
+                    mimetype = ff_mkv_image_mime_tags[i].str;
+                    break;
+                }
         }
         if (!mimetype) {
             av_log(s, AV_LOG_ERROR, "Attachment stream %d has no mimetype tag and "