[FFmpeg-devel] [PATCH] avformat/mov: don't abort on duplicate Mastering Display Metadata boxes

[FFmpeg-devel] [PATCH] avformat/mov: don't abort on duplicate Mastering Display Metadata boxes

[James Almer]

The VP9 spec defines a SmDm box for this information, and the ISOBMFF spec defines a
mdvc one. If both are present, just ignore one of them.
This is in line with clli and CoLL boxes.

Fixes ticket #10711.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavformat/mov.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index e7aa8d1833..5463f36770 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -6140,8 +6140,10 @@ static int mov_read_smdm(MOVContext *c, AVIOContext *pb, MOVAtom atom)
         av_log(c->fc, AV_LOG_WARNING, "Unsupported Mastering Display Metadata box version %d\n", version);
         return 0;
     }
-    if (sc->mastering)
-        return AVERROR_INVALIDDATA;
+    if (sc->mastering) {
+        av_log(c->fc, AV_LOG_WARNING, "Ignoring duplicate Mastering Display Metadata\n");
+        return 0;
+    }
 
     avio_skip(pb, 3); /* flags */
 
@@ -6178,11 +6180,16 @@ static int mov_read_mdcv(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 
     sc = c->fc->streams[c->fc->nb_streams - 1]->priv_data;
 
-    if (atom.size < 24 || sc->mastering) {
+    if (atom.size < 24) {
         av_log(c->fc, AV_LOG_ERROR, "Invalid Mastering Display Color Volume box\n");
         return AVERROR_INVALIDDATA;
     }
 
+    if (sc->mastering) {
+        av_log(c->fc, AV_LOG_WARNING, "Ignoring duplicate Mastering Display Color Volume\n");
+        return 0;
+    }
+
     sc->mastering = av_mastering_display_metadata_alloc();
     if (!sc->mastering)
         return AVERROR(ENOMEM);
-- 
2.44.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".

Re: [FFmpeg-devel] [PATCH] avformat/mov: don't abort on duplicate Mastering Display Metadata boxes

[Andreas Rheinhardt]

James Almer:
> The VP9 spec defines a SmDm box for this information, and the ISOBMFF spec defines a
> mdvc one. If both are present, just ignore one of them.
> This is in line with clli and CoLL boxes.
> 
> Fixes ticket #10711.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavformat/mov.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index e7aa8d1833..5463f36770 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -6140,8 +6140,10 @@ static int mov_read_smdm(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>          av_log(c->fc, AV_LOG_WARNING, "Unsupported Mastering Display Metadata box version %d\n", version);
>          return 0;
>      }
> -    if (sc->mastering)
> -        return AVERROR_INVALIDDATA;
> +    if (sc->mastering) {
> +        av_log(c->fc, AV_LOG_WARNING, "Ignoring duplicate Mastering Display Metadata\n");

If this is expected (and maybe even encouraged/required by some spec),
then why is this a warning?

> +        return 0;
> +    }
>  
>      avio_skip(pb, 3); /* flags */
>  
> @@ -6178,11 +6180,16 @@ static int mov_read_mdcv(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>  
>      sc = c->fc->streams[c->fc->nb_streams - 1]->priv_data;
>  
> -    if (atom.size < 24 || sc->mastering) {
> +    if (atom.size < 24) {
>          av_log(c->fc, AV_LOG_ERROR, "Invalid Mastering Display Color Volume box\n");
>          return AVERROR_INVALIDDATA;
>      }
>  
> +    if (sc->mastering) {
> +        av_log(c->fc, AV_LOG_WARNING, "Ignoring duplicate Mastering Display Color Volume\n");
> +        return 0;
> +    }
> +
>      sc->mastering = av_mastering_display_metadata_alloc();
>      if (!sc->mastering)
>          return AVERROR(ENOMEM);

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avformat/mov: don't abort on duplicate Mastering Display Metadata boxes

[James Almer]

On 3/26/2024 9:13 PM, Andreas Rheinhardt wrote:
> James Almer:
>> The VP9 spec defines a SmDm box for this information, and the ISOBMFF spec defines a
>> mdvc one. If both are present, just ignore one of them.
>> This is in line with clli and CoLL boxes.
>>
>> Fixes ticket #10711.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavformat/mov.c | 13 ++++++++++---
>>   1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>> index e7aa8d1833..5463f36770 100644
>> --- a/libavformat/mov.c
>> +++ b/libavformat/mov.c
>> @@ -6140,8 +6140,10 @@ static int mov_read_smdm(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>>           av_log(c->fc, AV_LOG_WARNING, "Unsupported Mastering Display Metadata box version %d\n", version);
>>           return 0;
>>       }
>> -    if (sc->mastering)
>> -        return AVERROR_INVALIDDATA;
>> +    if (sc->mastering) {
>> +        av_log(c->fc, AV_LOG_WARNING, "Ignoring duplicate Mastering Display Metadata\n");
> 
> If this is expected (and maybe even encouraged/required by some spec),
> then why is this a warning?

I don't know if it's expected. I'd expect VP9 streams would not come 
with mdvc and CoLL boxes at all, but apparently Youtube ones now do 
(Guess they are phasing out webm-dash and so now VP9 is served on mp4). 
I just copied the existing behavior from clli and CoLL.

> 
>> +        return 0;
>> +    }
>>   
>>       avio_skip(pb, 3); /* flags */
>>   
>> @@ -6178,11 +6180,16 @@ static int mov_read_mdcv(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>>   
>>       sc = c->fc->streams[c->fc->nb_streams - 1]->priv_data;
>>   
>> -    if (atom.size < 24 || sc->mastering) {
>> +    if (atom.size < 24) {
>>           av_log(c->fc, AV_LOG_ERROR, "Invalid Mastering Display Color Volume box\n");
>>           return AVERROR_INVALIDDATA;
>>       }
>>   
>> +    if (sc->mastering) {
>> +        av_log(c->fc, AV_LOG_WARNING, "Ignoring duplicate Mastering Display Color Volume\n");
>> +        return 0;
>> +    }
>> +
>>       sc->mastering = av_mastering_display_metadata_alloc();
>>       if (!sc->mastering)
>>           return AVERROR(ENOMEM);
> 
> _______________________________________________
> 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".
_______________________________________________
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".