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

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

[Nuo Mi]

On Sat, Jan 18, 2025 at 4:18 AM Frank Plowman <post@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@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?

> ---
>  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
>
>
_______________________________________________
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 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@gmail.com> wrote:

>
>
> On Sat, Jan 18, 2025 at 4:18 AM Frank Plowman <post@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@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@frankplowman.com>  and @Wang, Fei W
<fei.w.wang@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
>>
>>
_______________________________________________
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".