[FFmpeg-devel] [PATCH v3] lavc/vvc: Stop c->pix_fmt and s->pix_fmt diverging

[Nuo Mi]

On Sat, Jan 18, 2025 at 11:12 PM Nuo Mi <nuomi2021 at gmail.com> wrote:

>
>
> On Sat, Jan 18, 2025 at 4:18 AM Frank Plowman <post at frankplowman.com>
> wrote:
>
>> When the chroma format changes mid-sequence, c->pix_fmt and s->pix_fmt
>> can get out-of-sync.  More specifically,
>> 1. export_frame_params is called.
>>    c->pix_fmt and s->pix_fmt are both set to their new values.
>> 2. set_output_format is called.
>>    c->pix_fmt is set back to its old value.
>>
>> The two getting out of sync results in a memory error.  The check in
>> export_frame_params passes as it only tests the s->pix_fmt, therefore
>> c->pix_fmt keeps the old value and the buffers are not allocated for the
>> new pixel format.
>>
>> Patch fixes this by not setting c->pix_fmt in set_output_format.
>>
>> Signed-off-by: Frank Plowman <post at frankplowman.com>
>> ---
>> @Nuo Mi, Fei Wang:
>> Does this look okay?  I am not really sure why c->pix_fmt was set again
>> in set_output_format, was this accounting for some specific scenario?
>> In terms of bitstreams where the pix_fmt changes, I only have fuzzed
>> bitstreams unfortunately, so while this prevents crashing it is
>> difficult to know whether the output is correct.
>>
> Hi Frank,
> I agree with you; it's better to remove the duplicate pix_fmt in the VVC
> context.
> If we need it for hardware, it should be renamed to hw_fmt or something
> more descriptive.
> Could you share the clip?
>
Hi Frank,
Thank you for the clip.
If c->pix_fmt is not needed, we might also remove ff_set_dimensions, as
setting dimensions without a format is meaningless.

Hi @Frank Plowman <post at frankplowman.com>  and @Wang, Fei W
<fei.w.wang at intel.com>,
please help review
https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=13775


---
>>  libavcodec/vvc/dec.c | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/libavcodec/vvc/dec.c b/libavcodec/vvc/dec.c
>> index 1cb168de7e..53e3eeec6d 100644
>> --- a/libavcodec/vvc/dec.c
>> +++ b/libavcodec/vvc/dec.c
>> @@ -970,7 +970,6 @@ static int set_output_format(const VVCContext *s,
>> const AVFrame *output)
>>          if ((ret = ff_set_dimensions(c, output->width, output->height))
>> < 0)
>>              return ret;
>>      }
>> -    c->pix_fmt = output->format;
>>      return 0;
>>  }
>>
>> --
>> 2.47.0
>>
>>