[FFmpeg-devel,19/23] avformat/matroskaenc: Improve mimetype search

Use the mime_types of the corresponding AVCodecDescriptor instead of own
arrays. The former are more encompassing.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavcodec/codec_desc.c   |  1 +
 libavformat/matroskaenc.c | 14 +++-----------
 2 files changed, 4 insertions(+), 11 deletions(-)

On 11/5/2019 11:49 PM, Andreas Rheinhardt wrote:
> Use the mime_types of the corresponding AVCodecDescriptor instead of own
> arrays. The former are more encompassing.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavcodec/codec_desc.c   |  1 +
>  libavformat/matroskaenc.c | 14 +++-----------
>  2 files changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
> index 0602ecb1b5..837b09e7f4 100644
> --- a/libavcodec/codec_desc.c
> +++ b/libavcodec/codec_desc.c
> @@ -3020,6 +3020,7 @@ static const AVCodecDescriptor codec_descriptors[] = {
>          .name      = "text",
>          .long_name = NULL_IF_CONFIG_SMALL("raw UTF-8 text"),
>          .props     = AV_CODEC_PROP_TEXT_SUB,
> +        .mime_types= MT("text/plain"),
>      },
>      {
>          .id        = AV_CODEC_ID_XSUB,
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 19d9b0bc66..f2c8a66a03 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -1694,17 +1694,9 @@ 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];

This will change the value written for AV_CODEC_ID_BIN_DATA from
"binary" to "application/octet-stream". Is that intended and/or desirable?

>          }
>          if (!mimetype) {
>              av_log(s, AV_LOG_ERROR, "Attachment stream %d has no mimetype tag and "
>

James Almer:
> On 11/5/2019 11:49 PM, Andreas Rheinhardt wrote:
>> Use the mime_types of the corresponding AVCodecDescriptor instead of own
>> arrays. The former are more encompassing.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
>> ---
>>  libavcodec/codec_desc.c   |  1 +
>>  libavformat/matroskaenc.c | 14 +++-----------
>>  2 files changed, 4 insertions(+), 11 deletions(-)
>>
>> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
>> index 0602ecb1b5..837b09e7f4 100644
>> --- a/libavcodec/codec_desc.c
>> +++ b/libavcodec/codec_desc.c
>> @@ -3020,6 +3020,7 @@ static const AVCodecDescriptor codec_descriptors[] = {
>>          .name      = "text",
>>          .long_name = NULL_IF_CONFIG_SMALL("raw UTF-8 text"),
>>          .props     = AV_CODEC_PROP_TEXT_SUB,
>> +        .mime_types= MT("text/plain"),
>>      },
>>      {
>>          .id        = AV_CODEC_ID_XSUB,
>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
>> index 19d9b0bc66..f2c8a66a03 100644
>> --- a/libavformat/matroskaenc.c
>> +++ b/libavformat/matroskaenc.c
>> @@ -1694,17 +1694,9 @@ 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];
> 
> This will change the value written for AV_CODEC_ID_BIN_DATA from
> "binary" to "application/octet-stream". Is that intended and/or desirable?
> 
Both. application/octet-stream is on the list of mimetypes [1] and it
is used as default value for binary data [2], whereas "binary" is
AFAIK not even a valid name for a mimetype (and it is also not on the
list of mimetypes and neither does MKVToolNix offer said mimetype (it
uses application/octet-stream for unrecognized non-textual files)).
I don't know whether there are really files that use this mimetype in
the wild. The commit message for c9212abf indicates that there are,
but even in this case the muxer should not use this for
AV_CODEC_ID_BIN_DATA in the absence of a user-specified mimetype.

- Andreas

[1]: https://www.iana.org/assignments/media-types/media-types.xhtml
[2]: E.g.
https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/MIME_types/Complete_list_of_MIME_types