Re: [FFmpeg-devel] [PATCH] avcodec/vvc/ps: reset sps_id_used on PS uninit

[Nuo Mi <nuomi2021@gmail.com>]

On Mon, Apr 8, 2024 at 4:37 PM Frank Plowman <post@frankplowman.com> wrote:

> On 07/04/2024 15:48, James Almer wrote:
> > On 4/7/2024 10:38 AM, Nuo Mi wrote:
> >> On Sun, Apr 7, 2024 at 11:05 AM James Almer <jamrial@gmail.com> wrote:
> >>
> >>> Signed-off-by: James Almer <jamrial@gmail.com>
> >>> ---
> >>>   libavcodec/vvc/ps.c | 1 +
> >>>   1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/libavcodec/vvc/ps.c b/libavcodec/vvc/ps.c
> >>> index 3c71c34bae..83ee75fb62 100644
> >>> --- a/libavcodec/vvc/ps.c
> >>> +++ b/libavcodec/vvc/ps.c
> >>> @@ -912,6 +912,7 @@ void ff_vvc_ps_uninit(VVCParamSets *ps)
> >>>           ff_refstruct_unref(&ps->sps_list[i]);
> >>>       for (int i = 0; i < FF_ARRAY_ELEMS(ps->pps_list); i++)
> >>>           ff_refstruct_unref(&ps->pps_list[i]);
> >>> +    ps->sps_id_used = 0;
> >>>
> >> Hi James,
> >> thank you for the patch.
> >>
> >> Is this really necessary?
> >> vvc_ps_uninit will be called by vvc_decode_free,
> >> We are not supposed to use any member of VVCParamSets after
> >> vvc_decode_free.
> >
> > My bad, i thought it was also called on every flush() call.
> >
> > Something like the following:
> >
> >> diff --git a/libavcodec/vvc/dec.c b/libavcodec/vvc/dec.c
> >> index eb447604fe..463536512e 100644
> >> --- a/libavcodec/vvc/dec.c
> >> +++ b/libavcodec/vvc/dec.c
> >> @@ -963,6 +963,8 @@ static av_cold void
> >> vvc_decode_flush(AVCodecContext *avctx)
> >>          ff_vvc_flush_dpb(last);
> >>      }
> >>
> >> +    s->ps->sps_id_used = 0;
> >> +
> >>      s->eos = 1;
> >>  }
> >
> > Should be done on FFCodec.flush() (like when seeking) as the previous
> > state is no longer valid, right?
>
> Yes I agree, I think this is needed.  Cases where the random access
> point has no leading pictures should be covered by the existing logic as
> these always fall at the start of a CVS, but I think this is needed to
> cover the case in which there are leading pictures.
>
This patch isn't necessary.
Leading pictures won't carry SPS.
IDR, CRA, and GDR will carry SPS, but they will also start a new CVS, which
will covered by the current logic.


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