diff mbox series

[FFmpeg-devel] avcodec/libaomdec: decode extradata in aom_init

Message ID 20240308094928.62556-1-lq@chinaffmpeg.org
State New
Headers show
Series [FFmpeg-devel] avcodec/libaomdec: decode extradata in aom_init | expand

Checks

Context Check Description
yinshiyou/configure_loongarch64 warning Failed to apply patch
andriy/configure_x86 warning Failed to apply patch

Commit Message

Liu Steven March 8, 2024, 9:49 a.m. UTC 
before patch:
test failed when using libaomdec.
TEST    enhanced-flv-av1

Comments

James Almer March 8, 2024, 12:56 p.m. UTC | #1 
On 3/8/2024 6:49 AM, Steven Liu wrote:
> before patch:
> test failed when using libaomdec.
> TEST    enhanced-flv-av1
> --- src/tests/ref/fate/enhanced-flv-av1	2024-03-08 11:27:13.577935587 +0800
> +++ tests/data/fate/enhanced-flv-av1	2024-03-08 17:38:25.810152322 +0800
> @@ -3,7 +3,7 @@
>   #media_type 0: video
>   #codec_id 0: av1
>   #dimensions 0: 320x176
> -#sar 0: 0/1
> +#sar 0: 1/1
>   0,          0,          0,       41,     4718, 0xc4d912ec
>   0,         42,         42,       41,     5109, 0xc065dd7d, F=0x0
>   0,         83,         83,       41,      192, 0x160c5335, F=0x0
> 
> after patch:
> test passed
> 
> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> ---
>   libavcodec/libaomdec.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/libavcodec/libaomdec.c b/libavcodec/libaomdec.c
> index 69eec8b089..2df2fa6fd7 100644
> --- a/libavcodec/libaomdec.c
> +++ b/libavcodec/libaomdec.c
> @@ -58,6 +58,18 @@ static av_cold int aom_init(AVCodecContext *avctx,
>           return AVERROR(EINVAL);
>       }
>   
> +    if (aom_codec_decode(&ctx->decoder, avctx->extradata, avctx->extradata_size,
> +                         NULL) != AOM_CODEC_OK) {

Extradata may have the ISOBMFF prefix bytes, so you need to skip them 
unless aom_codec_decode() can handle them, which i assume it doesn't.
And of course, if there's nothing after the four prefix bytes, don't 
call anything.

> +        const char *error  = aom_codec_error(&ctx->decoder);
> +        const char *detail = aom_codec_error_detail(&ctx->decoder);
> +
> +        av_log(avctx, AV_LOG_ERROR, "Failed to decode extradata: %s\n", error);
> +        if (detail)
> +            av_log(avctx, AV_LOG_ERROR, "  Additional information: %s\n", detail);
> +
> +        return AVERROR(EINVAL);
> +    }
> +
>       return 0;
>   }
>
Andreas Rheinhardt March 8, 2024, 1:07 p.m. UTC | #2 
Steven Liu:
> before patch:
> test failed when using libaomdec.
> TEST    enhanced-flv-av1
> --- src/tests/ref/fate/enhanced-flv-av1	2024-03-08 11:27:13.577935587 +0800
> +++ tests/data/fate/enhanced-flv-av1	2024-03-08 17:38:25.810152322 +0800
> @@ -3,7 +3,7 @@
>  #media_type 0: video
>  #codec_id 0: av1
>  #dimensions 0: 320x176
> -#sar 0: 0/1
> +#sar 0: 1/1
>  0,          0,          0,       41,     4718, 0xc4d912ec
>  0,         42,         42,       41,     5109, 0xc065dd7d, F=0x0
>  0,         83,         83,       41,      192, 0x160c5335, F=0x0
> 
> after patch:
> test passed
> 
> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> ---
>  libavcodec/libaomdec.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/libavcodec/libaomdec.c b/libavcodec/libaomdec.c
> index 69eec8b089..2df2fa6fd7 100644
> --- a/libavcodec/libaomdec.c
> +++ b/libavcodec/libaomdec.c
> @@ -58,6 +58,18 @@ static av_cold int aom_init(AVCodecContext *avctx,
>          return AVERROR(EINVAL);
>      }
>  
> +    if (aom_codec_decode(&ctx->decoder, avctx->extradata, avctx->extradata_size,
> +                         NULL) != AOM_CODEC_OK) {
> +        const char *error  = aom_codec_error(&ctx->decoder);
> +        const char *detail = aom_codec_error_detail(&ctx->decoder);
> +
> +        av_log(avctx, AV_LOG_ERROR, "Failed to decode extradata: %s\n", error);
> +        if (detail)
> +            av_log(avctx, AV_LOG_ERROR, "  Additional information: %s\n", detail);
> +
> +        return AVERROR(EINVAL);
> +    }
> +
>      return 0;
>  }
>  

This patch is very problematic: If you change the ref file to match
libaomdec, it will be broken for those that use other decoders than
libaomdec. Actually, there should be no reason to use libaomdec for
probing since 194414f62d987ad831654c45fff6ee27898b832e. Maybe we should
add the AVOID_PROBING cap to the other AV1 decoders in case the native
one is available?

- Andreas
James Almer March 8, 2024, 1:13 p.m. UTC | #3 
On 3/8/2024 10:07 AM, Andreas Rheinhardt wrote:
> Steven Liu:
>> before patch:
>> test failed when using libaomdec.
>> TEST    enhanced-flv-av1
>> --- src/tests/ref/fate/enhanced-flv-av1	2024-03-08 11:27:13.577935587 +0800
>> +++ tests/data/fate/enhanced-flv-av1	2024-03-08 17:38:25.810152322 +0800
>> @@ -3,7 +3,7 @@
>>   #media_type 0: video
>>   #codec_id 0: av1
>>   #dimensions 0: 320x176
>> -#sar 0: 0/1
>> +#sar 0: 1/1
>>   0,          0,          0,       41,     4718, 0xc4d912ec
>>   0,         42,         42,       41,     5109, 0xc065dd7d, F=0x0
>>   0,         83,         83,       41,      192, 0x160c5335, F=0x0
>>
>> after patch:
>> test passed
>>
>> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
>> ---
>>   libavcodec/libaomdec.c | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/libavcodec/libaomdec.c b/libavcodec/libaomdec.c
>> index 69eec8b089..2df2fa6fd7 100644
>> --- a/libavcodec/libaomdec.c
>> +++ b/libavcodec/libaomdec.c
>> @@ -58,6 +58,18 @@ static av_cold int aom_init(AVCodecContext *avctx,
>>           return AVERROR(EINVAL);
>>       }
>>   
>> +    if (aom_codec_decode(&ctx->decoder, avctx->extradata, avctx->extradata_size,
>> +                         NULL) != AOM_CODEC_OK) {
>> +        const char *error  = aom_codec_error(&ctx->decoder);
>> +        const char *detail = aom_codec_error_detail(&ctx->decoder);
>> +
>> +        av_log(avctx, AV_LOG_ERROR, "Failed to decode extradata: %s\n", error);
>> +        if (detail)
>> +            av_log(avctx, AV_LOG_ERROR, "  Additional information: %s\n", detail);
>> +
>> +        return AVERROR(EINVAL);
>> +    }
>> +
>>       return 0;
>>   }
>>   
> 
> This patch is very problematic: If you change the ref file to match
> libaomdec, it will be broken for those that use other decoders than
> libaomdec. Actually, there should be no reason to use libaomdec for
> probing since 194414f62d987ad831654c45fff6ee27898b832e. Maybe we should
> add the AVOID_PROBING cap to the other AV1 decoders in case the native
> one is available?

Or add -c:v av1 as input argument for the final decode in stream_remux() 
from fate-run.sh
Steven Liu March 8, 2024, 1:31 p.m. UTC | #4 
James Almer <jamrial@gmail.com> 于2024年3月8日周五 21:13写道：
>
> On 3/8/2024 10:07 AM, Andreas Rheinhardt wrote:
> > Steven Liu:
> >> before patch:
> >> test failed when using libaomdec.
> >> TEST    enhanced-flv-av1
> >> --- src/tests/ref/fate/enhanced-flv-av1      2024-03-08 11:27:13.577935587 +0800
> >> +++ tests/data/fate/enhanced-flv-av1 2024-03-08 17:38:25.810152322 +0800
> >> @@ -3,7 +3,7 @@
> >>   #media_type 0: video
> >>   #codec_id 0: av1
> >>   #dimensions 0: 320x176
> >> -#sar 0: 0/1
> >> +#sar 0: 1/1
> >>   0,          0,          0,       41,     4718, 0xc4d912ec
> >>   0,         42,         42,       41,     5109, 0xc065dd7d, F=0x0
> >>   0,         83,         83,       41,      192, 0x160c5335, F=0x0
> >>
> >> after patch:
> >> test passed
> >>
> >> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> >> ---
> >>   libavcodec/libaomdec.c | 12 ++++++++++++
> >>   1 file changed, 12 insertions(+)
> >>
> >> diff --git a/libavcodec/libaomdec.c b/libavcodec/libaomdec.c
> >> index 69eec8b089..2df2fa6fd7 100644
> >> --- a/libavcodec/libaomdec.c
> >> +++ b/libavcodec/libaomdec.c
> >> @@ -58,6 +58,18 @@ static av_cold int aom_init(AVCodecContext *avctx,
> >>           return AVERROR(EINVAL);
> >>       }
> >>
> >> +    if (aom_codec_decode(&ctx->decoder, avctx->extradata, avctx->extradata_size,
> >> +                         NULL) != AOM_CODEC_OK) {
> >> +        const char *error  = aom_codec_error(&ctx->decoder);
> >> +        const char *detail = aom_codec_error_detail(&ctx->decoder);
> >> +
> >> +        av_log(avctx, AV_LOG_ERROR, "Failed to decode extradata: %s\n", error);
> >> +        if (detail)
> >> +            av_log(avctx, AV_LOG_ERROR, "  Additional information: %s\n", detail);
> >> +
> >> +        return AVERROR(EINVAL);
> >> +    }
> >> +
> >>       return 0;
> >>   }
> >>
> >
> > This patch is very problematic: If you change the ref file to match
> > libaomdec, it will be broken for those that use other decoders than
> > libaomdec. Actually, there should be no reason to use libaomdec for
> > probing since 194414f62d987ad831654c45fff6ee27898b832e. Maybe we should
> > add the AVOID_PROBING cap to the other AV1 decoders in case the native
> > one is available?
>
> Or add -c:v av1 as input argument for the final decode in stream_remux()
> from fate-run.sh
It's using -c:v av1 right now, or should I move libavformat/av1.c to
libavcodec and modify for support set the params?


Thanks
Steven
diff mbox series

Patch

--- src/tests/ref/fate/enhanced-flv-av1	2024-03-08 11:27:13.577935587 +0800
+++ tests/data/fate/enhanced-flv-av1	2024-03-08 17:38:25.810152322 +0800
@@ -3,7 +3,7 @@ 
 #media_type 0: video
 #codec_id 0: av1
 #dimensions 0: 320x176
-#sar 0: 0/1
+#sar 0: 1/1
 0,          0,          0,       41,     4718, 0xc4d912ec
 0,         42,         42,       41,     5109, 0xc065dd7d, F=0x0
 0,         83,         83,       41,      192, 0x160c5335, F=0x0

after patch:
test passed

Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
---
 libavcodec/libaomdec.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/libavcodec/libaomdec.c b/libavcodec/libaomdec.c
index 69eec8b089..2df2fa6fd7 100644
--- a/libavcodec/libaomdec.c
+++ b/libavcodec/libaomdec.c
@@ -58,6 +58,18 @@  static av_cold int aom_init(AVCodecContext *avctx,
         return AVERROR(EINVAL);
     }
 
+    if (aom_codec_decode(&ctx->decoder, avctx->extradata, avctx->extradata_size,
+                         NULL) != AOM_CODEC_OK) {
+        const char *error  = aom_codec_error(&ctx->decoder);
+        const char *detail = aom_codec_error_detail(&ctx->decoder);
+
+        av_log(avctx, AV_LOG_ERROR, "Failed to decode extradata: %s\n", error);
+        if (detail)
+            av_log(avctx, AV_LOG_ERROR, "  Additional information: %s\n", detail);
+
+        return AVERROR(EINVAL);
+    }
+
     return 0;
 }