[FFmpeg-devel] [PATCH 1/2] avformat/matroskaenc: Improve mimetype search

Andreas Rheinhardt andreas.rheinhardt at gmail.com
Thu Apr 16 05:18:03 EEST 2020


Use the mime_types of the corresponding AVCodecDescriptor instead of
tables specific to Matroska. The former are generally more encompassing:
They contain every item of the current lists except "text/plain" for
AV_CODEC_ID_TEXT and "binary" for AV_CODEC_ID_BIN_DATA.

The former has been preserved by special-casing it while the latter is
a hack added in c9212abf so that the demuxer (which uses the same tables)
sets the appropriate CodecID for broken files ("binary" is not a correct
mime type at all); using it for the muxer was a mistake. The correct
mime type for AV_CODEC_ID_BIN_DATA is "application/octet-stream" and
this is what one gets from the AVCodecDescriptor.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
---
I also have a patch like this for the Matroska demuxer; it would make
the mime_tables in matroska.c completely superfluous. But there are two
things that need to be fixed before this can happen:

1. The Matroska muxer must be able to write video streams with
attached pic disposition as attachment. (This is currently broken and if
more attachments will be exported as attached picture video streams (as
will happen because the AVCodecDescriptor list contains entries (e.g.
WebP) that the current list does not contain), then more simple
Matroska->Matroska remuxing scenarios will be affected by this.)

2. The Matroska demuxer does not set the dimensions of attached pictures
(because they are not available at the container level). So if no decoder
is available, these values will never be set. But init_muxer() in mux.c
checks for valid dimensions (unless the format is of AVFMT_NODIMENSIONS
type which Matroska is not (strangely not even the null or the hash
muxers have this set)) and this breaks e.g. remuxing Matroska->Matroska.
Given that e.g. the flac muxer needs the dimensions for attached
pictures, one cannot simply omit the check for attached picture streams.
The only solution I can think of is adding another flag which says that
the format does not need dimensions for attached pictures.

Of course, 1. needs to be fixed before the proposed solution for 2.
makes any sense (at least for Matroska, I don't know if there are other
formats that are affected by this).

 libavformat/matroskaenc.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index d3256d8f5d..e1ddf366d4 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -1667,17 +1667,11 @@ static int mkv_write_attachments(AVFormatContext *s)
         if (t = av_dict_get(st->metadata, "mimetype", NULL, 0))
             mimetype = t->value;
         else if (st->codecpar->codec_id != AV_CODEC_ID_NONE ) {
-            int i;
-            for (i = 0; ff_mkv_mime_tags[i].id != AV_CODEC_ID_NONE; i++)
-                if (ff_mkv_mime_tags[i].id == st->codecpar->codec_id) {
-                    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->codecpar->codec_id) {
-                    mimetype = ff_mkv_image_mime_tags[i].str;
-                    break;
-                }
+            const AVCodecDescriptor *desc = avcodec_descriptor_get(st->codecpar->codec_id);
+            if (desc && desc->mime_types) {
+                mimetype = desc->mime_types[0];
+            } else if (st->codecpar->codec_id = AV_CODEC_ID_TEXT)
+                mimetype = "text/plain";
         }
         if (!mimetype) {
             av_log(s, AV_LOG_ERROR, "Attachment stream %d has no mimetype tag and "
-- 
2.20.1



More information about the ffmpeg-devel mailing list