On 09/04/2024 14:36, Nuo Mi wrote: > On Mon, Apr 8, 2024 at 11:15 PM Frank Plowman <p...@frankplowman.com> wrote: > >> On 08/04/2024 15:12, Nuo Mi wrote: >>> On Mon, Apr 8, 2024 at 4:37 PM Frank Plowman <p...@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 <jamr...@gmail.com> >> wrote: >>>>>> >>>>>>> Signed-off-by: James Almer <jamr...@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. >>> >> >> I may be misunderstanding the spec, NoOutputBeforeRecoveryFlag is set >> for pictures which have no leading pictures, no? In any case take, for >> instance, a CRA picture. In most cases, CRA pictures have >> NoOutputBeforeRecoveryFlag=0, therefore are not CLVSS pictures and >> sps_id_used is not reset by the existing logic. Do we not need to reset >> sps_id_used when seeking to a CRA then? >> > After seeking, we'll set s->last_eos to 1. > For a CRA, decode_recovery_flag will set s->no_output_before_recovery_flag > to s->last_eos. > So no_output_before_recovery_flag will be 1, not 0.
I see, my mistake. Yes in this case I do not think sps_id_used must be explicitly reset. -- 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-requ...@ffmpeg.org with subject "unsubscribe".
