On Mon, Nov 13, 2023 at 04:32:33PM +0100, Niklas Haas wrote: > From: Niklas Haas <g...@haasn.dev> > > Before cedf589, this function would return early return on RGB and float > formats, as well as when range was equal. While this commit > intentionally removed the early return for same-range YUV conversions, > it missed that RGB and float formats that have an unscaled converter > should always early return, no matter what the source range was set to. > > Fixes: cedf589c09c567b72bf4c1a58db53d94622567e1 > --- > libswscale/utils.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/libswscale/utils.c b/libswscale/utils.c > index ec822ff5d9..7ce86f83ea 100644 > --- a/libswscale/utils.c > +++ b/libswscale/utils.c > @@ -1733,6 +1733,9 @@ static av_cold int sws_init_single_context(SwsContext > *c, SwsFilter *srcFilter, > av_log(c, AV_LOG_INFO, > "unscaled %s -> %s special converter is available\n", > av_get_pix_fmt_name(srcFormat), > av_get_pix_fmt_name(dstFormat)); > + > + if (isAnyRGB(dstFormat) || isFloat(srcFormat) || > isFloat(dstFormat)) > + return 0;
this and the other patch result in difficult to understand code.
if i look back to 6.0 the 2 cases had unconditional "return 0"
the code before basically
if(set of conditions for "alphablend")
convert_unscaled = alphablend
return 0
if(set of conditions for "special converter")
convert_unscaled = set special converter
return 0
now they conditionally return
if(set of conditions)
convert_unscaled = alphablend
if set of more conditions
return 0
if(set of conditions)
convert_unscaled = set special converter
if set of more conditions
return 0
I think this could add some burden to whoever eventually has to
clean this up
i may be missing something but i wonder if not either
* 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
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
but i dont feel like i fully understand the issue here so maybe iam missing
the goal of this patchset somewhat
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato
signature.asc
Description: PGP signature
_______________________________________________ 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".
