diff mbox series

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

Message ID 20200909060217.25794-7-rcombs@rcombs.me
State New
Headers show
Series [FFmpeg-devel,01/15] lavc/codec_desc: add MIME type to AV_CODEC_ID_TEXT
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Ridley Combs Sept. 9, 2020, 6:02 a.m. UTC
---
 libavformat/matroskadec.c | 40 ++++++---------------------------------
 1 file changed, 6 insertions(+), 34 deletions(-)

Comments

Andreas Rheinhardt Sept. 9, 2020, 9:49 a.m. UTC | #1
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
diff mbox series

Patch

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;
             }
         }
     }