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

[Frank Plowman]

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 at gmail.com> wrote:
>>
>>> Signed-off-by: James Almer <jamrial at 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.

Thanks,
--
Frank