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 6105B430F2 for ; Fri, 20 May 2022 17:20:35 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 6D72568B462; Fri, 20 May 2022 20:20:33 +0300 (EEST) Received: from haasn.dev (haasn.dev [78.46.187.166]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 9C28C68AEFF for ; Fri, 20 May 2022 20:20:26 +0300 (EEST) Received: from haasn.dev (unknown [10.30.0.2]) by haasn.dev (Postfix) with ESMTP id 3115841F36 for ; Fri, 20 May 2022 19:20:26 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=haasn.xyz; s=mail; t=1653067226; bh=LLPDEUHYO8cmYYE8r0oLaQ6phT62EIntdRJ9sfYtL60=; h=Date:From:To:Subject:In-Reply-To:References:From; b=C1sHEiS6kpSZgAv4o0gxihoGHkjfMUDJPO5lxwcSLrQowWuCUCPG4D9mMD5U/Wrhs gUohzCe0NtdLg4ucxwzmXmLlZjXb7D7hNXUj9P4tS7hwBUgDcVsNK3kuvN48MKCKhS lIH7OHQjRqVIacjxGqu4bYyBm3PKktvnaopMJPPQ= Date: Fri, 20 May 2022 19:20:26 +0200 Message-ID: <20220520192026.GB74590@haasn.xyz> From: Niklas Haas To: FFmpeg development discussions and patches In-Reply-To: <20220520102815.GT396728@pb2> References: <20220518151817.21270-1-leo.izen@gmail.com> <20220518182338.GR396728@pb2> <20220518182748.GS396728@pb2> <20220520102815.GT396728@pb2> MIME-Version: 1.0 Content-Disposition: inline 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: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On Fri, 20 May 2022 12:28:15 +0200 Michael Niedermayer wrote: > 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; > > > > > > 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 > > > > 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. > > Try a printf %f of 0.1 and it will do 0.100000 looks good but thats deception > try that with more precission %100.99f shows this: > 0.100000000000000005551115123125782702118158340454101562500000000000000000000000000000000000000000000 > > so if we want to use exactly the values from the spec, doubles with a > 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 > > 1b. the exact value that 0.1 has in float/double depends on the precission > IEEE float > 0.100000001490116119384765625000000000000000000000000000000000000000000000000000000000000000000000000 > IEEE double > 0.100000000000000005551115123125782702118158340454101562500000000000000000000000000000000000000000000 > long double > 0.100000000000000000001355252715606880542509316001087427139282226562500000000000000000000000000000000 > So there will be slight differences if (intermediate) types anywhere arent > exactly the same It's worth pointing out that these values are already, in many cases, arbitrarily rounded. D65, for example, has many different definitions: - ISO/CIE provide a version rounded to 6 decimal digits but also provides the full tabulated spectral curves (sampled at 10nm intervals) from which you can derive a higher precision version - ITU-R always rounds to 4 digits, but newer ITU-R standards (e.g. BT.2100) reference the aforementioned ISO document for the definition, making it technically ambiguous. - Older EBU documents even round to 3 digits - There exist other sources that round to 5 digits also In my mind, this weakens somewhat the case of making sure these values have exact representations, if the underlying physical constant being represented does not have an exact decimal representation to begin with. > > 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 > 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 > > thx > > > [...] > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > The day soldiers stop bringing you their problems is the day you have stopped > leading them. They have either lost confidence that you can help or concluded > you do not care. Either case is a failure of leadership. - Colin Powell > _______________________________________________ > 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". _______________________________________________ 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".