[FFmpeg-devel] [PATCH] avcodec/cbs_apv: don't return an error when reading empty buffers

[FFmpeg-devel] [PATCH] avcodec/cbs_apv: don't return an error when reading empty buffers

[James Almer]

The output will be a fragment with zero units, which is a lot more user friendly
than making them think something went wrong, as it already happens with cbs_av1.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/cbs_apv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/cbs_apv.c b/libavcodec/cbs_apv.c
index 2c4a1daaf0..66ea4cebab 100644
--- a/libavcodec/cbs_apv.c
+++ b/libavcodec/cbs_apv.c
@@ -175,7 +175,7 @@ static int cbs_apv_split_fragment(CodedBitstreamContext *ctx,
 
     if (frag->data_size < 4) {
         // Too small to be a valid fragment.
-        return AVERROR_INVALIDDATA;
+        return 0;
     }
 
     // Don't include parsing here in trace output.
-- 
2.49.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] avcodec/cbs_apv: don't return an error when reading empty buffers

[Mark Thompson]

On 29/04/2025 20:57, James Almer wrote:
> The output will be a fragment with zero units, which is a lot more user friendly
> than making them think something went wrong, as it already happens with cbs_av1.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/cbs_apv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/cbs_apv.c b/libavcodec/cbs_apv.c
> index 2c4a1daaf0..66ea4cebab 100644
> --- a/libavcodec/cbs_apv.c
> +++ b/libavcodec/cbs_apv.c
> @@ -175,7 +175,7 @@ static int cbs_apv_split_fragment(CodedBitstreamContext *ctx,
>  
>      if (frag->data_size < 4) {
>          // Too small to be a valid fragment.
> -        return AVERROR_INVALIDDATA;
> +        return 0;
>      }
>  
>      // Don't include parsing here in trace output.

Seems fair for a zero-size buffer, but I don't like this for a three-byte fragment containing { 0xab, 0xcd, 0xef } - that feels like it should return an error.

Maybe it would make more sense to have a separate check for zero.  What do you think?

Thanks,

- Mark

_______________________________________________
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] avcodec/cbs_apv: don't return an error when reading empty buffers

[James Almer]

[-- Attachment #1.1.1: Type: text/plain, Size: 1509 bytes --]

On 4/30/2025 6:17 PM, Mark Thompson wrote:
> On 29/04/2025 20:57, James Almer wrote:
>> The output will be a fragment with zero units, which is a lot more user friendly
>> than making them think something went wrong, as it already happens with cbs_av1.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavcodec/cbs_apv.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/cbs_apv.c b/libavcodec/cbs_apv.c
>> index 2c4a1daaf0..66ea4cebab 100644
>> --- a/libavcodec/cbs_apv.c
>> +++ b/libavcodec/cbs_apv.c
>> @@ -175,7 +175,7 @@ static int cbs_apv_split_fragment(CodedBitstreamContext *ctx,
>>   
>>       if (frag->data_size < 4) {
>>           // Too small to be a valid fragment.
>> -        return AVERROR_INVALIDDATA;
>> +        return 0;
>>       }
>>   
>>       // Don't include parsing here in trace output.
> 
> Seems fair for a zero-size buffer, but I don't like this for a three-byte fragment containing { 0xab, 0xcd, 0xef } - that feels like it should return an error.
> 
> Maybe it would make more sense to have a separate check for zero.  What do you think?

Sure, will send an updated patch in a bit.

> 
> Thanks,
> 
> - Mark
> 
> _______________________________________________
> 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".


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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] [PATCH v2] avcodec/cbs_apv: don't return an error when reading empty buffers

[James Almer]

The output will be a fragment with zero units, which is a lot more user friendly
than making them think something went wrong, as it already happens with cbs_av1.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/cbs_apv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/cbs_apv.c b/libavcodec/cbs_apv.c
index 2c4a1daaf0..e7dd04825c 100644
--- a/libavcodec/cbs_apv.c
+++ b/libavcodec/cbs_apv.c
@@ -168,8 +168,8 @@ static int cbs_apv_split_fragment(CodedBitstreamContext *ctx,
     uint32_t signature;
     int err, trace;
 
-    if (header) {
-        // Ignore extradata fragments.
+    if (header || !frag->data_size) {
+        // Ignore empty or extradata fragments.
         return 0;
     }
 
-- 
2.49.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 v2] avcodec/cbs_apv: don't return an error when reading empty buffers

[Mark Thompson]

On 30/04/2025 22:22, James Almer wrote:
> The output will be a fragment with zero units, which is a lot more user friendly
> than making them think something went wrong, as it already happens with cbs_av1.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/cbs_apv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/cbs_apv.c b/libavcodec/cbs_apv.c
> index 2c4a1daaf0..e7dd04825c 100644
> --- a/libavcodec/cbs_apv.c
> +++ b/libavcodec/cbs_apv.c
> @@ -168,8 +168,8 @@ static int cbs_apv_split_fragment(CodedBitstreamContext *ctx,
>      uint32_t signature;
>      int err, trace;
>  
> -    if (header) {
> -        // Ignore extradata fragments.
> +    if (header || !frag->data_size) {
> +        // Ignore empty or extradata fragments.
>          return 0;
>      }
>  

LGTM.

Thanks,

- Mark

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