Re: [FFmpeg-devel] [PATCH 2/3] swscale/utils: correctly return from sws_init_single_context

[Niklas Haas <ffmpeg@haasn.xyz>]

On Mon, 13 Nov 2023 19:30:08 +0100 Michael Niedermayer <michael@niedermayer.cc> wrote:
> but i dont feel like i fully understand the issue here so maybe iam missing
> the goal of this patchset somewhat

So, to summarize the main problem:

1. sws_init_single_context() previously hard-coded decisions based on
   c->srcRange and c->dstRange. This is fundamentally broken, as
   srcRange/dstRange can change at any time with
   sws_setColorspaceDetails.

2. To fix this, this function was made to not early-return, and instead
   run the rest of the init code just in case range conversion is needed
   later. (With the check for whether or not the special converter can
   be used being moved to the callsite instead of the setup site)

3. This caused problems for non-YUV inputs, because previously these
   would always early return, but now they run the rest of the code,
   which triggers at least one assertion for float32 formats.

4. To fix this, this commit restores the early-return for non-YUV,
   preserving the status quo of existing behavior w.r.t not hitting the
   rest of the init function.

5. Separately, this commit fixes an error in previous condition (2) at
   the callsite, which relied on c->lumConvertRange being unset when no
   range conversion is needed. However, that condition did not match the
   condition used in the setup check before.

> * convert_unscaled should only be set when used
> OR
> * if these are set "always" if not alphablend and convert_unscaled should be
>   two seperate fields. But i have not at all looked at what consequences that
>   would have so maybe that has issues

convert_unscaled cannot be set only when used because we don't yet know
if it will be used or not. There is also no advantage I see to splitting
the fields, as they have basically the same logic attached to them
- being dependent only on whether or not range conversion is needed.

> Also if some range convert should not be used/set for some cases then
> the check should maybe be where the range convert is setup not far away
> from it. I mean a check close to the related code is easier to understand
> 

One alternative that would make this possible would be to re-run whole
context init from sws_setColorspaceDetails, if the srcRange/dstRange
change.
_______________________________________________
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".