diff mbox series

[FFmpeg-devel,02/15] lavc/codec_desc: add additional JPEG and BMP MIME types

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

Checks

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

Commit Message

rcombs Sept. 9, 2020, 6:02 a.m. UTC 
---
 libavcodec/codec_desc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Paul B Mahol Sept. 9, 2020, 9:24 a.m. UTC | #1 
On Wed, Sep 09, 2020 at 01:02:04AM -0500, rcombs wrote:
> ---
>  libavcodec/codec_desc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

    lgtm
> 
> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
> index 3dee640360..49a00ad264 100644
> --- a/libavcodec/codec_desc.c
> +++ b/libavcodec/codec_desc.c
> @@ -82,7 +82,7 @@ static const AVCodecDescriptor codec_descriptors[] = {
>          .name      = "mjpeg",
>          .long_name = NULL_IF_CONFIG_SMALL("Motion JPEG"),
>          .props     = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSY,
> -        .mime_types= MT("image/jpeg"),
> +        .mime_types= MT("image/jpeg", "image/jpg"),
>          .profiles  = NULL_IF_CONFIG_SMALL(ff_mjpeg_profiles),
>      },
>      {
> @@ -588,7 +588,7 @@ static const AVCodecDescriptor codec_descriptors[] = {
>          .name      = "bmp",
>          .long_name = NULL_IF_CONFIG_SMALL("BMP (Windows and OS/2 bitmap)"),
>          .props     = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSLESS,
> -        .mime_types= MT("image/x-ms-bmp"),
> +        .mime_types= MT("image/x-ms-bmp", "image/bmp"),
>      },
>      {
>          .id        = AV_CODEC_ID_CSCD,
> -- 
> 2.27.0
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Andreas Rheinhardt Sept. 9, 2020, 9:40 a.m. UTC | #2 
rcombs:
> ---
>  libavcodec/codec_desc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
> index 3dee640360..49a00ad264 100644
> --- a/libavcodec/codec_desc.c
> +++ b/libavcodec/codec_desc.c
> @@ -82,7 +82,7 @@ static const AVCodecDescriptor codec_descriptors[] = {
>          .name      = "mjpeg",
>          .long_name = NULL_IF_CONFIG_SMALL("Motion JPEG"),
>          .props     = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSY,
> -        .mime_types= MT("image/jpeg"),
> +        .mime_types= MT("image/jpeg", "image/jpg"),
>          .profiles  = NULL_IF_CONFIG_SMALL(ff_mjpeg_profiles),
>      },
>      {
> @@ -588,7 +588,7 @@ static const AVCodecDescriptor codec_descriptors[] = {
>          .name      = "bmp",
>          .long_name = NULL_IF_CONFIG_SMALL("BMP (Windows and OS/2 bitmap)"),
>          .props     = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSLESS,
> -        .mime_types= MT("image/x-ms-bmp"),
> +        .mime_types= MT("image/x-ms-bmp", "image/bmp"),
>      },
>      {
>          .id        = AV_CODEC_ID_CSCD,
> 
The mime types should be sorted by "best match first". Is x-ms-bmp
really better than just bmp?

- Andreas
Oneric Sept. 9, 2020, 1:23 p.m. UTC | #3 
On Wed, Sep 09, 2020 at 01:02:04 -0500, rcombs wrote:
> ---
>  libavcodec/codec_desc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
> index 3dee640360..49a00ad264 100644
> --- a/libavcodec/codec_desc.c
> +++ b/libavcodec/codec_desc.c
> @@ -82,7 +82,7 @@ static const AVCodecDescriptor codec_descriptors[] = {
>          .name      = "mjpeg",
>          .long_name = NULL_IF_CONFIG_SMALL("Motion JPEG"),
>          .props     = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSY,
> -        .mime_types= MT("image/jpeg"),
> +        .mime_types= MT("image/jpeg", "image/jpg"),

"image/jpg" doesn't seeem to be an official IANA MIME type ("image/jpeg" is).
That ofc doesn't mean it isn't being used, but after a quick search I can't 
find a reference for "image/jpg" anywhere.
Did you encounter a program using "image/jpg"? If not it may be better to 
stick with "image/jpeg" only.


>          .profiles  = NULL_IF_CONFIG_SMALL(ff_mjpeg_profiles),
>      },
>      {
> @@ -588,7 +588,7 @@ static const AVCodecDescriptor codec_descriptors[] = {
>          .name      = "bmp",
>          .long_name = NULL_IF_CONFIG_SMALL("BMP (Windows and OS/2 bitmap)"),
>          .props     = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSLESS,
> -        .mime_types= MT("image/x-ms-bmp"),
> +        .mime_types= MT("image/x-ms-bmp", "image/bmp"),
>      },
>      {
>          .id        = AV_CODEC_ID_CSCD,
> -- 
> 2.27.0
>
Andreas Rheinhardt Sept. 9, 2020, 1:28 p.m. UTC | #4 
Oneric:
> On Wed, Sep 09, 2020 at 01:02:04 -0500, rcombs wrote:
>> ---
>>  libavcodec/codec_desc.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
>> index 3dee640360..49a00ad264 100644
>> --- a/libavcodec/codec_desc.c
>> +++ b/libavcodec/codec_desc.c
>> @@ -82,7 +82,7 @@ static const AVCodecDescriptor codec_descriptors[] = {
>>          .name      = "mjpeg",
>>          .long_name = NULL_IF_CONFIG_SMALL("Motion JPEG"),
>>          .props     = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSY,
>> -        .mime_types= MT("image/jpeg"),
>> +        .mime_types= MT("image/jpeg", "image/jpg"),
> 
> "image/jpg" doesn't seeem to be an official IANA MIME type ("image/jpeg" is).
> That ofc doesn't mean it isn't being used, but after a quick search I can't 
> find a reference for "image/jpg" anywhere.
> Did you encounter a program using "image/jpg"? If not it may be better to 
> stick with "image/jpeg" only.
> 
The ff_id3v2_mime_tags array (removed in patch 14 of this series)
contains this. It has been added intentionally in
f704eb612b3333a589d83741e07bfbdf1cffb8cb, yet without giving any reason
for it.

- Andreas
Oneric Sept. 9, 2020, 2:23 p.m. UTC | #5 
On Wed, Sep 09, 2020 at 15:28:05 +0200, Andreas Rheinhardt wrote:
> Oneric:
> > On Wed, Sep 09, 2020 at 01:02:04 -0500, rcombs wrote:
> >> -        .mime_types= MT("image/jpeg"),
> >> +        .mime_types= MT("image/jpeg", "image/jpg"),
> > 
> > "image/jpg" doesn't seeem to be an official IANA MIME type ("image/jpeg" is).
> > That ofc doesn't mean it isn't being used, but after a quick search I can't 
> > find a reference for "image/jpg" anywhere.
> > Did you encounter a program using "image/jpg"? If not it may be better to 
> > stick with "image/jpeg" only.
> > 
> The ff_id3v2_mime_tags array (removed in patch 14 of this series)
> contains this. It has been added intentionally in
> f704eb612b3333a589d83741e07bfbdf1cffb8cb, yet without giving any reason
> for it.
> 
> - Andreas

Ohh, I missed it being in ff_id3v2_mime_tags, thanks. In that case 
disregard my previous mail. It's probably a good idea to keep "image/jpg" to 
not break any files, as long as "image/jpeg" stays the preferred option.
rcombs Sept. 9, 2020, 5:49 p.m. UTC | #6 
> On Sep 9, 2020, at 08:23, Oneric <oneric@oneric.de> wrote:
> 
> On Wed, Sep 09, 2020 at 01:02:04 -0500, rcombs wrote:
>> ---
>> libavcodec/codec_desc.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
>> index 3dee640360..49a00ad264 100644
>> --- a/libavcodec/codec_desc.c
>> +++ b/libavcodec/codec_desc.c
>> @@ -82,7 +82,7 @@ static const AVCodecDescriptor codec_descriptors[] = {
>>         .name      = "mjpeg",
>>         .long_name = NULL_IF_CONFIG_SMALL("Motion JPEG"),
>>         .props     = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSY,
>> -        .mime_types= MT("image/jpeg"),
>> +        .mime_types= MT("image/jpeg", "image/jpg"),
> 
> "image/jpg" doesn't seeem to be an official IANA MIME type ("image/jpeg" is).
> That ofc doesn't mean it isn't being used, but after a quick search I can't 
> find a reference for "image/jpg" anywhere.
> Did you encounter a program using "image/jpg"? If not it may be better to 
> stick with "image/jpeg" only.

It was in ff_id3v2_mime_tags, and the commits adding indicate that some files
used it, so I added it here for compatibility with that.

> 
> 
>>         .profiles  = NULL_IF_CONFIG_SMALL(ff_mjpeg_profiles),
>>     },
>>     {
>> @@ -588,7 +588,7 @@ static const AVCodecDescriptor codec_descriptors[] = {
>>         .name      = "bmp",
>>         .long_name = NULL_IF_CONFIG_SMALL("BMP (Windows and OS/2 bitmap)"),
>>         .props     = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSLESS,
>> -        .mime_types= MT("image/x-ms-bmp"),
>> +        .mime_types= MT("image/x-ms-bmp", "image/bmp"),
>>     },
>>     {
>>         .id        = AV_CODEC_ID_CSCD,
>> -- 
>> 2.27.0
>> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe".
rcombs Sept. 9, 2020, 5:53 p.m. UTC | #7 
> On Sep 9, 2020, at 04:40, Andreas Rheinhardt <andreas.rheinhardt@gmail.com> wrote:
> 
> rcombs:
>> ---
>> libavcodec/codec_desc.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
>> index 3dee640360..49a00ad264 100644
>> --- a/libavcodec/codec_desc.c
>> +++ b/libavcodec/codec_desc.c
>> @@ -82,7 +82,7 @@ static const AVCodecDescriptor codec_descriptors[] = {
>>         .name      = "mjpeg",
>>         .long_name = NULL_IF_CONFIG_SMALL("Motion JPEG"),
>>         .props     = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSY,
>> -        .mime_types= MT("image/jpeg"),
>> +        .mime_types= MT("image/jpeg", "image/jpg"),
>>         .profiles  = NULL_IF_CONFIG_SMALL(ff_mjpeg_profiles),
>>     },
>>     {
>> @@ -588,7 +588,7 @@ static const AVCodecDescriptor codec_descriptors[] = {
>>         .name      = "bmp",
>>         .long_name = NULL_IF_CONFIG_SMALL("BMP (Windows and OS/2 bitmap)"),
>>         .props     = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSLESS,
>> -        .mime_types= MT("image/x-ms-bmp"),
>> +        .mime_types= MT("image/x-ms-bmp", "image/bmp"),
>>     },
>>     {
>>         .id        = AV_CODEC_ID_CSCD,
>> 
> The mime types should be sorted by "best match first". Is x-ms-bmp
> really better than just bmp?

For some (pseudo-)codecs it might be better to prefer an older type name
for compatibility (e.g. TTFs should be muxed in MKV as 
application/x-truetype-font rather than font/ttf for compatibility with
players that don't yet handle font/ttf as a font), but yeah in this case image/bmp
is better-supported and should go first; fixed.

> 
> - Andreas
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
index 3dee640360..49a00ad264 100644
--- a/libavcodec/codec_desc.c
+++ b/libavcodec/codec_desc.c
@@ -82,7 +82,7 @@  static const AVCodecDescriptor codec_descriptors[] = {
         .name      = "mjpeg",
         .long_name = NULL_IF_CONFIG_SMALL("Motion JPEG"),
         .props     = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSY,
-        .mime_types= MT("image/jpeg"),
+        .mime_types= MT("image/jpeg", "image/jpg"),
         .profiles  = NULL_IF_CONFIG_SMALL(ff_mjpeg_profiles),
     },
     {
@@ -588,7 +588,7 @@  static const AVCodecDescriptor codec_descriptors[] = {
         .name      = "bmp",
         .long_name = NULL_IF_CONFIG_SMALL("BMP (Windows and OS/2 bitmap)"),
         .props     = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSLESS,
-        .mime_types= MT("image/x-ms-bmp"),
+        .mime_types= MT("image/x-ms-bmp", "image/bmp"),
     },
     {
         .id        = AV_CODEC_ID_CSCD,