[FFmpeg-devel] [PATCH 07/15] lavf/matroskadec: use avcodec_descriptor_get_by_mime_type

[Andreas Rheinhardt]

rcombs:
> ---
>  libavformat/matroskadec.c | 40 ++++++---------------------------------
>  1 file changed, 6 insertions(+), 34 deletions(-)
> 
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index b1ef344aa7..71debe692a 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -749,25 +749,6 @@ static EbmlSyntax matroska_cluster_enter[] = {
>  };
>  #undef CHILD_OF
>  
> -static const CodecMime 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}
> -};
> -
> -static const CodecMime 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},
> -    {"binary"                     , AV_CODEC_ID_BIN_DATA},
> -
> -    {""                           , AV_CODEC_ID_NONE}
> -};
> -
>  static const char *const matroska_doctypes[] = { "matroska", "webm" };
>  
>  static int matroska_read_close(AVFormatContext *s);
> @@ -2908,6 +2889,7 @@ static int matroska_read_header(AVFormatContext *s)
>                attachments[j].bin.data && attachments[j].bin.size > 0)) {
>              av_log(matroska->ctx, AV_LOG_ERROR, "incomplete attachment\n");
>          } else {
> +            const AVCodecDescriptor *desc = avcodec_descriptor_get_by_mime_type(attachments[j].mime, NULL);
>              AVStream *st = avformat_new_stream(s, NULL);
>              if (!st)
>                  break;
> @@ -2917,17 +2899,12 @@ static int matroska_read_header(AVFormatContext *s)
>                  av_dict_set(&st->metadata, "title", attachments[j].description, 0);
>              st->codecpar->codec_id   = AV_CODEC_ID_NONE;
>  
> -            for (i = 0; mkv_image_mime_tags[i].id != AV_CODEC_ID_NONE; i++) {
> -                if (!strncmp(mkv_image_mime_tags[i].str, attachments[j].mime,
> -                             strlen(mkv_image_mime_tags[i].str))) {
> -                    st->codecpar->codec_id = mkv_image_mime_tags[i].id;
> -                    break;
> -                }
> -            }
> +            if (desc)
> +                st->codecpar->codec_id = desc->id;
>  
>              attachments[j].stream = st;
>  
> -            if (st->codecpar->codec_id != AV_CODEC_ID_NONE) {
> +            if (desc && desc->type == AVMEDIA_TYPE_VIDEO) {
>                  AVPacket *pkt = &st->attached_pic;
>  
>                  st->disposition         |= AV_DISPOSITION_ATTACHED_PIC;
> @@ -2947,13 +2924,8 @@ static int matroska_read_header(AVFormatContext *s)
>                  memcpy(st->codecpar->extradata, attachments[j].bin.data,
>                         attachments[j].bin.size);
>  
> -                for (i = 0; mkv_mime_tags[i].id != AV_CODEC_ID_NONE; i++) {
> -                    if (!strncmp(mkv_mime_tags[i].str, attachments[j].mime,
> -                                strlen(mkv_mime_tags[i].str))) {
> -                        st->codecpar->codec_id = mkv_mime_tags[i].id;
> -                        break;
> -                    }
> -                }
> +                if (!strcmp(attachments[j].mime, "binary"))
> +                    st->codecpar->codec_id = AV_CODEC_ID_BIN_DATA;
>              }
>          }
>      }
> 
The reason I have not already done this when I made the Matroska muxer
use the codec descriptor list in [1] (I tried) is that your change
breaks remuxing scenarios when no decoder is available for a format. To
quote myself:
"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."

If I am not mistaken, I found this with an svg pic.

- Andreas

[1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-April/260726.html