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

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(-)

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?

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.

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

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.