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 16679412D0 for ; Fri, 20 May 2022 10:28:26 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id B7B2F68B42E; Fri, 20 May 2022 13:28:23 +0300 (EEST) Received: from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net [217.70.183.201]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 200EC68AF3B for ; Fri, 20 May 2022 13:28:17 +0300 (EEST) Received: from localhost (213-47-68-29.cable.dynamic.surfer.at [213.47.68.29]) (Authenticated sender: michael@niedermayer.cc) by mail.gandi.net (Postfix) with ESMTPSA id 416A01BF211 for ; Fri, 20 May 2022 10:28:15 +0000 (UTC) Date: Fri, 20 May 2022 12:28:15 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20220520102815.GT396728@pb2> References: <20220518151817.21270-1-leo.izen@gmail.com> <20220518182338.GR396728@pb2> <20220518182748.GS396728@pb2> MIME-Version: 1.0 In-Reply-To: <20220518182748.GS396728@pb2> Subject: Re: [FFmpeg-devel] [PATCH v4] avutil/csp: create avpriv API for colorspace structs 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="===============7346018460281947090==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============7346018460281947090== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="SSJ6yXlPvEk0CmSI" Content-Disposition: inline --SSJ6yXlPvEk0CmSI Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, May 18, 2022 at 08:27:48PM +0200, Michael Niedermayer wrote: > On Wed, May 18, 2022 at 08:23:38PM +0200, Michael Niedermayer wrote: > > On Wed, May 18, 2022 at 11:18:17AM -0400, Leo Izen wrote: > > > This commit moves some of the functionality from avfilter/colorspace > > > into avutil/csp and exposes it as an avpriv API so it can be used by > > > libavcodec and/or libavformat. > > [...] > > > +#ifndef AVUTIL_CSP_H > > > +#define AVUTIL_CSP_H > > > + > > > +#include "libavutil/pixfmt.h" > > > + > > > +typedef struct AVLumaCoefficients { > > > + double cr, cg, cb; > > > +} AVLumaCoefficients; > > > + > > > +typedef struct AVPrimaryCoefficients { > > > + double xr, yr, xg, yg, xb, yb; > > > +} AVPrimaryCoefficients; > > > + > > > +typedef struct AVWhitepointCoefficients { > > > + double xw, yw; > > > +} AVWhitepointCoefficients; > >=20 > > As said, these should not be floating point. > > Adding a new public API and changing it later is messy, this > > should be changed before its made public >=20 > i now see you replaced some public by avpriv in the latest patch > but still i think this should be changed to fixed point or AVRational > first. Even as API between the libs its messy to change it later > it would require us to keep the double API when its changed until > the next major bump I see some discussion related to this on the IRC log from when i was sleeping. Maybe it would be better to keep this on the mailing list Also iam not sure my concern was clearly worded so ill sort my argument and concerns so its clearer below: 1. exactly representing values if you have a 0.1 you can represent that exactly as AVRational 1/10 but maybe shockingly a double cannot. =20 Try a printf %f of 0.1 and it will do 0.100000 looks good but thats dec= eption try that with more precission %100.99f shows this: 0.100000000000000005551115123125782702118158340454101562500000000000000= 000000000000000000000000000000 =20 so if we want to use exactly the values from the spec, doubles with a= =20 unit/base of 1 do not work. int or even doubles with a base/unit of 30000 might work exactly if AVRational is unpopular. 30000 instead of 10000 is for that one pesky 1= /3 =20 1b. the exact value that 0.1 has in float/double depends on the precission IEEE float 0.100000001490116119384765625000000000000000000000000000000000000000000= 000000000000000000000000000000 IEEE double 0.100000000000000005551115123125782702118158340454101562500000000000000= 000000000000000000000000000000 long double 0.100000000000000000001355252715606880542509316001087427139282226562500= 000000000000000000000000000000 So there will be slight differences if (intermediate) types anywhere ar= ent exactly the same 2. someone said, you need to pick a denominator when doing float -> rational av_d2q() will pick the best denominator for you. 3. avpriv_ vs av_ avpriv is evil, it combines the pain of ABI/API compatibility while the= =20 public cant use it 4. rounding, regressions and inexactness doubles/floats have in the past broken regression tests. They do not always but i suggest we avoid them when theres no clear advantage like higher speed in speed relevant code or much simpler code =20 thx =20 [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The day soldiers stop bringing you their problems is the day you have stopp= ed=20 leading them. They have either lost confidence that you can help or conclud= ed=20 you do not care. Either case is a failure of leadership. - Colin Powell --SSJ6yXlPvEk0CmSI Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCYodtOwAKCRBhHseHBAsP q9T8AJ4juZ8mGxuHqeBOjBJ8tBsYkbNaGwCggfkUC2umzvM6wJrbRRy4Dnn6rIM= =I4QH -----END PGP SIGNATURE----- --SSJ6yXlPvEk0CmSI-- --===============7346018460281947090== 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". --===============7346018460281947090==--