[FFmpeg-devel] [PATCH] avcodec/vvcdec: frame_context_setup, set fc->ref to NULL

[FFmpeg-devel] [PATCH] avcodec/vvcdec: frame_context_setup, set fc->ref to NULL

[Nuo Mi]

fc->ref points to an old VVCFrame, which cannot be used after frame_context_setup.
This prevents crashes in decode_nal_units-->ff_vvc_report_frame_finished.

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

diff --git a/libavcodec/vvc/vvcdec.c b/libavcodec/vvc/vvcdec.c
index 8163b5ecb6..e88e746de4 100644
--- a/libavcodec/vvc/vvcdec.c
+++ b/libavcodec/vvc/vvcdec.c
@@ -594,6 +594,8 @@ static int frame_context_setup(VVCFrameContext *fc, VVCContext *s)
 {
     int ret;
 
+    fc->ref = NULL;
+
     // copy refs from the last frame
     if (s->nb_frames && s->nb_fcs > 1) {
         VVCFrameContext *prev = get_frame_context(s, fc, -1);
-- 
2.25.1

_______________________________________________
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/vvcdec: frame_context_setup, set fc->ref to NULL

[Frank Plowman]

On 13/02/2024 02:30, Nuo Mi wrote:
> fc->ref points to an old VVCFrame, which cannot be used after frame_context_setup.
> This prevents crashes in decode_nal_units-->ff_vvc_report_frame_finished.
> 
> Signed-off-by: Frank Plowman <post@frankplowman.com>
> ---
>  libavcodec/vvc/vvcdec.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/libavcodec/vvc/vvcdec.c b/libavcodec/vvc/vvcdec.c
> index 8163b5ecb6..e88e746de4 100644
> --- a/libavcodec/vvc/vvcdec.c
> +++ b/libavcodec/vvc/vvcdec.c
> @@ -594,6 +594,8 @@ static int frame_context_setup(VVCFrameContext *fc, VVCContext *s)
>  {
>      int ret;
>  
> +    fc->ref = NULL;
> +
>      // copy refs from the last frame
>      if (s->nb_frames && s->nb_fcs > 1) {
>          VVCFrameContext *prev = get_frame_context(s, fc, -1);

LGTM.  Fixes the crash on all the fuzz data I have which produce it.
FATE runners are failing at the time of writing, but I manually ran this
against the VVC tests as well as the suite from the FFVVC GitHub and all
tests passed.

Btw, I don't think you should add Signed-off-by tags for other people.
Their exact meaning varies by project and I am not sure of their meaning
in FFmpeg (if there is one), but generally they indicate that person
claims some sort of responsibility for the patch in the case of e.g. a
license violation.  That being said, I am happy to sign this off.

-- 
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] avcodec/vvcdec: frame_context_setup, set fc->ref to NULL

[Nuo Mi]

On Tue, Feb 13, 2024 at 6:39 PM Frank Plowman <post@frankplowman.com> wrote:

> On 13/02/2024 02:30, Nuo Mi wrote:
> > fc->ref points to an old VVCFrame, which cannot be used after
> frame_context_setup.
> > This prevents crashes in decode_nal_units-->ff_vvc_report_frame_finished.
> >
> > Signed-off-by: Frank Plowman <post@frankplowman.com>
> > ---
> >  libavcodec/vvc/vvcdec.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/libavcodec/vvc/vvcdec.c b/libavcodec/vvc/vvcdec.c
> > index 8163b5ecb6..e88e746de4 100644
> > --- a/libavcodec/vvc/vvcdec.c
> > +++ b/libavcodec/vvc/vvcdec.c
> > @@ -594,6 +594,8 @@ static int frame_context_setup(VVCFrameContext *fc,
> VVCContext *s)
> >  {
> >      int ret;
> >
> > +    fc->ref = NULL;
> > +
> >      // copy refs from the last frame
> >      if (s->nb_frames && s->nb_fcs > 1) {
> >          VVCFrameContext *prev = get_frame_context(s, fc, -1);
>
> LGTM.  Fixes the crash on all the fuzz data I have which produce it.
> FATE runners are failing at the time of writing, but I manually ran this
> against the VVC tests as well as the suite from the FFVVC GitHub and all
> tests passed.
>
> Btw, I don't think you should add Signed-off-by tags for other people.
> Their exact meaning varies by project and I am not sure of their meaning
> in FFmpeg (if there is one), but generally they indicate that person
> claims some sort of responsibility for the patch in the case of e.g. a
> license violation.  That being said, I am happy to sign this off.
>
Hi Frank,
Thank you for the review.
Sorry for the misuse. I will pay attention to it next time.
Patch applied.

>
> --
> 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] avcodec/vvcdec: frame_context_setup, set fc->ref to NULL

[Anton Khirnov]

Quoting Frank Plowman (2024-02-13 11:38:50)
> Btw, I don't think you should add Signed-off-by tags for other people.
> Their exact meaning varies by project and I am not sure of their meaning
> in FFmpeg (if there is one),

There is no officially accepted one.

E.g. I add my signoff to patches I push where I'm not the author.

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