[FFmpeg-devel] [PATCH] lavc/vvc: Check fc->ref contains valid reference

[FFmpeg-devel] [PATCH] lavc/vvc: Check fc->ref contains valid reference

[post]

From: Frank Plowman <post@frankplowman.com>

Depending on where exactly decode_nal_unit failed, it is possible that
fc->ref holds a VVCFrame which has had ff_vvc_unref_frame called on it
and not yet had ref_frame called on it.  In this case, fc->ref most of
the fields of fc->ref are NULL and attempting to call
ff_vvc_report_frame_finished on it will result in a null dereference.

Patch fixes the error described above by checking fc->ref has not only
been allocated, but also references a valid AVFrame before attempting to
call ff_vvc_report_frame_finished on it.

Signed-off-by: Frank Plowman <post@frankplowman.com>
---
 libavcodec/vvc/vvcdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/vvc/vvcdec.c b/libavcodec/vvc/vvcdec.c
index 8163b5ecb6..246ee79299 100644
--- a/libavcodec/vvc/vvcdec.c
+++ b/libavcodec/vvc/vvcdec.c
@@ -820,7 +820,7 @@ static int decode_nal_units(VVCContext *s, VVCFrameContext *fc, AVPacket *avpkt)
     return 0;
 
 fail:
-    if (fc->ref)
+    if (fc->ref && fc->ref->frame->buf[0])
         ff_vvc_report_frame_finished(fc->ref);
     return ret;
 }
-- 
2.43.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] lavc/vvc: Check fc->ref contains valid reference

[Lynne]

Feb 8, 2024, 22:16 by post@frankplowman.com:

> From: Frank Plowman <post@frankplowman.com>
>
> Depending on where exactly decode_nal_unit failed, it is possible that
> fc->ref holds a VVCFrame which has had ff_vvc_unref_frame called on it
> and not yet had ref_frame called on it.  In this case, fc->ref most of
> the fields of fc->ref are NULL and attempting to call
> ff_vvc_report_frame_finished on it will result in a null dereference.
>
> Patch fixes the error described above by checking fc->ref has not only
> been allocated, but also references a valid AVFrame before attempting to
> call ff_vvc_report_frame_finished on it.
>
> Signed-off-by: Frank Plowman <post@frankplowman.com>
> ---
>  libavcodec/vvc/vvcdec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/vvc/vvcdec.c b/libavcodec/vvc/vvcdec.c
> index 8163b5ecb6..246ee79299 100644
> --- a/libavcodec/vvc/vvcdec.c
> +++ b/libavcodec/vvc/vvcdec.c
> @@ -820,7 +820,7 @@ static int decode_nal_units(VVCContext *s, VVCFrameContext *fc, AVPacket *avpkt)
>  return 0;
>  
>  fail:
> -    if (fc->ref)
> +    if (fc->ref && fc->ref->frame->buf[0])
>  ff_vvc_report_frame_finished(fc->ref);
>  return ret;
>  }
>

In general, for other codecs, if a reference does not exist,
we simply allocate it and pretend it existed and was correctly decoded.
This has better resilience against corrupt bitstreams or just bad muxing,
and yields an (maybe corrupt) output, which is better than nothing.

Is this not possible for VVC?
_______________________________________________
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] lavc/vvc: Check fc->ref contains valid reference

[Frank Plowman]

On 08/02/2024 21:50, Lynne wrote:
> Feb 8, 2024, 22:16 by post@frankplowman.com:
> 
>> From: Frank Plowman <post@frankplowman.com>
>>
>> Depending on where exactly decode_nal_unit failed, it is possible that
>> fc->ref holds a VVCFrame which has had ff_vvc_unref_frame called on it
>> and not yet had ref_frame called on it.  In this case, fc->ref most of
>> the fields of fc->ref are NULL and attempting to call
>> ff_vvc_report_frame_finished on it will result in a null dereference.
>>
>> Patch fixes the error described above by checking fc->ref has not only
>> been allocated, but also references a valid AVFrame before attempting to
>> call ff_vvc_report_frame_finished on it.
>>
>> Signed-off-by: Frank Plowman <post@frankplowman.com>
>> ---
>>  libavcodec/vvc/vvcdec.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/vvc/vvcdec.c b/libavcodec/vvc/vvcdec.c
>> index 8163b5ecb6..246ee79299 100644
>> --- a/libavcodec/vvc/vvcdec.c
>> +++ b/libavcodec/vvc/vvcdec.c
>> @@ -820,7 +820,7 @@ static int decode_nal_units(VVCContext *s, VVCFrameContext *fc, AVPacket *avpkt)
>>  return 0;
>>  
>>  fail:
>> -    if (fc->ref)
>> +    if (fc->ref && fc->ref->frame->buf[0])
>>  ff_vvc_report_frame_finished(fc->ref);
>>  return ret;
>>  }
>>
> 
> In general, for other codecs, if a reference does not exist,
> we simply allocate it and pretend it existed and was correctly decoded.
> This has better resilience against corrupt bitstreams or just bad muxing,
> and yields an (maybe corrupt) output, which is better than nothing.
> 
> Is this not possible for VVC?

I think the naming is confusing here.  fc->ref is a pointer to the
VVCFrame currently being decoded.

-- 
Frank
_______________________________________________
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] lavc/vvc: Check fc->ref contains valid reference

[Nuo Mi]

On Fri, Feb 9, 2024 at 6:46 AM Frank Plowman <post@frankplowman.com> wrote:

>
>
> On 08/02/2024 21:50, Lynne wrote:
> > Feb 8, 2024, 22:16 by post@frankplowman.com:
> >
> >> From: Frank Plowman <post@frankplowman.com>
> >>
> >> Depending on where exactly decode_nal_unit failed, it is possible that
> >> fc->ref holds a VVCFrame which has had ff_vvc_unref_frame called on it
> >> and not yet had ref_frame called on it.  In this case, fc->ref most of
> >> the fields of fc->ref are NULL and attempting to call
> >> ff_vvc_report_frame_finished on it will result in a null dereference.
> >>
> >> Patch fixes the error described above by checking fc->ref has not only
> >> been allocated, but also references a valid AVFrame before attempting to
> >> call ff_vvc_report_frame_finished on it.
> >>
> >> Signed-off-by: Frank Plowman <post@frankplowman.com>
> >> ---
> >>  libavcodec/vvc/vvcdec.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/libavcodec/vvc/vvcdec.c b/libavcodec/vvc/vvcdec.c
> >> index 8163b5ecb6..246ee79299 100644
> >> --- a/libavcodec/vvc/vvcdec.c
> >> +++ b/libavcodec/vvc/vvcdec.c
> >> @@ -820,7 +820,7 @@ static int decode_nal_units(VVCContext *s,
> VVCFrameContext *fc, AVPacket *avpkt)
> >>  return 0;
> >>
> >>  fail:
> >> -    if (fc->ref)
> >> +    if (fc->ref && fc->ref->frame->buf[0])
> >>  ff_vvc_report_frame_finished(fc->ref);
> >>  return ret;
> >>  }
> >>
> >
> > In general, for other codecs, if a reference does not exist,
> > we simply allocate it and pretend it existed and was correctly decoded.
> > This has better resilience against corrupt bitstreams or just bad muxing,
> > and yields an (maybe corrupt) output, which is better than nothing.
> >
> > Is this not possible for VVC?
>
> I think the naming is confusing here.  fc->ref is a pointer to the
> VVCFrame currently being decoded.
>
HI Lynee,
the name is from hevc. I do not like it either, but so many functions
copied from hevc will use it. ...

Hi Frank,
Please try https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=10775
thank you

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

Re: [FFmpeg-devel] [PATCH] lavc/vvc: Check fc->ref contains valid reference

[Anton Khirnov]

Quoting Nuo Mi (2024-02-13 04:03:11)
> HI Lynee,
> the name is from hevc. I do not like it either, but so many functions
> copied from hevc will use it. ...

The naming in hevcdec is really schizophrenic, I wish somebody did
something about it.
Certainly do not feel compelled to use it as a model.

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