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

[Andreas Rheinhardt]

Andreas Rheinhardt:
> 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 "
> 
Will apply tomorrow if there are no objections.

- Andreas