From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id D2FE9475E6 for ; Mon, 13 Nov 2023 18:30:21 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 0C9696803EE; Mon, 13 Nov 2023 20:30:18 +0200 (EET) Received: from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net [217.70.183.195]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id C57686803EE for ; Mon, 13 Nov 2023 20:30:10 +0200 (EET) Received: by mail.gandi.net (Postfix) with ESMTPSA id 5EE6560003 for ; Mon, 13 Nov 2023 18:30:09 +0000 (UTC) Date: Mon, 13 Nov 2023 19:30:08 +0100 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20231113183008.GK3543730@pb2> References: <20231113153234.8812-1-ffmpeg@haasn.xyz> <20231113153234.8812-2-ffmpeg@haasn.xyz> MIME-Version: 1.0 In-Reply-To: <20231113153234.8812-2-ffmpeg@haasn.xyz> X-GND-Sasl: michael@niedermayer.cc Subject: Re: [FFmpeg-devel] [PATCH 2/3] swscale/utils: correctly return from sws_init_single_context X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: multipart/mixed; boundary="===============6769722686562135308==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============6769722686562135308== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="oKZ2/WGKdwSRHmj9" Content-Disposition: inline --oKZ2/WGKdwSRHmj9 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Nov 13, 2023 at 04:32:33PM +0100, Niklas Haas wrote: > From: Niklas Haas >=20 > 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. >=20 > Fixes: cedf589c09c567b72bf4c1a58db53d94622567e1 > --- > libswscale/utils.c | 3 +++ > 1 file changed, 3 insertions(+) >=20 > 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(SwsConte= xt *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_na= me(dstFormat)); > + > + if (isAnyRGB(dstFormat) || isFloat(srcFormat) || isFloat(dst= Format)) > + 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 =3D alphablend return 0 if(set of conditions for "special converter") convert_unscaled =3D set special converter return 0 now they conditionally return if(set of conditions) convert_unscaled =3D alphablend if set of more conditions return 0 if(set of conditions) convert_unscaled =3D 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 th= at 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 =66rom 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 [...] --=20 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 --oKZ2/WGKdwSRHmj9 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCZVJrJQAKCRBhHseHBAsP q7UVAKCM4RyrB33O2tL4+6GHVGWbhuyz9gCfdkeJPLhDLgv4uQ0YQ99uNNhcD4U= =Sa2U -----END PGP SIGNATURE----- --oKZ2/WGKdwSRHmj9-- --===============6769722686562135308== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ 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". --===============6769722686562135308==--