| Message ID | 20220518151817.21270-1-leo.izen@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v4] avutil/csp: create avpriv API for colorspace structs | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
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 thx [...]
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 thx [...]
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 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 [...]
Hi Michael, On Fri, May 20, 2022 at 6:28 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > 1. exactly representing values > This isn't actually what I meant when I made the argument. If the spec says "0.137", I'd expect to be able to git grep the source code for "0.137" and find where it's defined. This is lost with AVRational, where it becomes { 137, 1000 }. This may sound silly, but I find this helpful. One way to address this is to add the exact value in a comment, like "(AVRational) { 137, 1000 }, // 0.137". This isn't pretty but retains grep-discoverability. Ronald
On 5/20/2022 7:28 AM, 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 > > 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 Not making it public allows us to not commit to a fixed design *now*. Unless there's a clear need for this to be part of the public API, i don't think it's a good idea to do so. This change is being made because this API is afaict needed in lavc, and not by some project using lavu. Removing/changing an avpriv symbol or struct is a matter or waiting for the nearest major bump. No need for an arbitrary 2 year wait period, compat wrappers, or anything crazy. > > 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 > > > [...] > > > _______________________________________________ > 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".
On 5/20/22 06:28, Michael Niedermayer wrote: > > 2. someone said, you need to pick a denominator when doing float -> rational > av_d2q() will pick the best denominator for you. > av_d2q() requires you to pass a maximum denominator, which essentially determines precision of the conversion. You still have to make a (somewhat) arbitrary choice in that matter. - Leo Izen (thebombzen)
Hi Ronald On Fri, May 20, 2022 at 07:26:56AM -0400, Ronald S. Bultje wrote: > Hi Michael, > > On Fri, May 20, 2022 at 6:28 AM Michael Niedermayer <michael@niedermayer.cc> > wrote: > > > 1. exactly representing values > > > > This isn't actually what I meant when I made the argument. If the spec says > "0.137", I'd expect to be able to git grep the source code for "0.137" and > find where it's defined. This is lost with AVRational, where it becomes { > 137, 1000 }. This may sound silly, but I find this helpful. > > One way to address this is to add the exact value in a comment, like > "(AVRational) { 137, 1000 }, // 0.137". This isn't pretty but retains > grep-discoverability. We use fixed point numbers in multiple places without loosing grep-discoverability for example libavcodec/mpegaudiotab.h: FIX(0.54119610014619701222), libavcodec/mpegaudiotab.h: FIX(1.3065629648763763537), libavcodec/mpegaudiotab.h: FIX(0.50979557910415917998), libavcodec/mpegaudiotab.h: FIX(2.5629154477415054814), libavcodec/mpegaudiotab.h: FIX(0.89997622313641556513), libavcodec/mpegaudiotab.h: FIX(0.60134488693504528634), libavcodec/mpegaudiotab.h: FIX(0.5024192861881556782), libavcodec/jfdctint_template.c:#define FIX_0_765366865 FIX(0.765366865) libavcodec/jfdctint_template.c:#define FIX_0_899976223 FIX(0.899976223) libavcodec/jfdctint_template.c:#define FIX_1_175875602 FIX(1.175875602) libavcodec/jfdctint_template.c:#define FIX_1_501321110 FIX(1.501321110) libavcodec/jfdctint_template.c:#define FIX_1_847759065 FIX(1.847759065) libavcodec/jfdctint_template.c:#define FIX_1_961570560 FIX(1.961570560) libavcodec/dcadata.c: SCALE(0.01112466771), SCALE(0.43016362190), SCALE(0.53190881014), SCALE(0.02678431384), libavcodec/dcadata.c: SCALE(0.01170534454), SCALE(0.43572667241), SCALE(0.52686679363), SCALE(0.02568206564), libavcodec/dcadata.c: SCALE(0.01230939943), SCALE(0.44127810001), SCALE(0.52177828550), SCALE(0.02461459488), libavcodec/dcadata.c: SCALE(0.01293735672), SCALE(0.44681602716), SCALE(0.51664537191), SCALE(0.02358125709), libavcodec/dcadata.c: SCALE(0.01358995494), SCALE(0.45233830810), SCALE(0.51147013903), SCALE(0.02258131653), libavcodec/dcadata.c: SCALE(0.01426773332), SCALE(0.45784294605), SCALE(0.50625455379), SCALE(0.02161412500), tests/utils.c: lum[0] = (FIX(0.29900) * r + FIX(0.58700) * g + tests/utils.c: FIX(0.11400) * b + ONE_HALF) >> SCALEBITS; tests/utils.c: lum[1] = (FIX(0.29900) * r + FIX(0.58700) * g + tests/utils.c: FIX(0.11400) * b + ONE_HALF) >> SCALEBITS; tests/utils.c: lum[0] = (FIX(0.29900) * r + FIX(0.58700) * g + tests/utils.c: FIX(0.11400) * b + ONE_HALF) >> SCALEBITS; tests/utils.c: lum[1] = (FIX(0.29900) * r + FIX(0.58700) * g + tests/utils.c: FIX(0.11400) * b + ONE_HALF) >> SCALEBITS; such a macro could produce a AVRational too easily (if wanted) the advantage over a comment is that a comment can be forgotten the argument for a macro cannot be forgotten and still succeed build thx [...]
On Fri, May 20, 2022 at 08:53:35AM -0300, James Almer wrote: > > > On 5/20/2022 7:28 AM, 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 > > > > 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 > > Not making it public allows us to not commit to a fixed design *now*. > Unless there's a clear need for this to be part of the public API, i don't > think it's a good idea to do so. This change is being made because this API > is afaict needed in lavc, and not by some project using lavu. > > Removing/changing an avpriv symbol or struct is a matter or waiting for the > nearest major bump. No need for an arbitrary 2 year wait period, compat > wrappers, or anything crazy. yes but is that really easier ? sheduling changes to happen during a bump vs adding a warper+deprecating at any time with a automatic #if removing code thx [...]
Hi, On Fri, May 20, 2022 at 9:11 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > On Fri, May 20, 2022 at 07:26:56AM -0400, Ronald S. Bultje wrote: > > On Fri, May 20, 2022 at 6:28 AM Michael Niedermayer < > michael@niedermayer.cc> > > > 1. exactly representing values > > > > This isn't actually what I meant when I made the argument. If the spec > says > > "0.137", I'd expect to be able to git grep the source code for "0.137" > and > > find where it's defined. This is lost with AVRational, where it becomes { > > 137, 1000 }. This may sound silly, but I find this helpful. > > > > One way to address this is to add the exact value in a comment, like > > "(AVRational) { 137, 1000 }, // 0.137". This isn't pretty but retains > > grep-discoverability. > > We use fixed point numbers in multiple places without > loosing grep-discoverability > for example > [..] > the advantage over a comment is that a comment can be forgotten > the argument for a macro cannot be forgotten and still succeed build > That sounds reasonable, I'm OK with this approach. Ronald
Quoting James Almer (2022-05-20 13:53:35) > > > > 3. avpriv_ vs av_ > > avpriv is evil, it combines the pain of ABI/API compatibility while the > > public cant use it > > Not making it public allows us to not commit to a fixed design *now*. > Unless there's a clear need for this to be part of the public API, i > don't think it's a good idea to do so. This change is being made because > this API is afaict needed in lavc, and not by some project using lavu. The commit message claims it will be used in lavc and lavf. Seems to me like a strong sign that it really should be public. As far as I'm concerned, avpriv should not exist at all.
On 5/20/22 11:50, Anton Khirnov wrote: > > The commit message claims it will be used in lavc and lavf. Seems to me > like a strong sign that it really should be public. > > As far as I'm concerned, avpriv should not exist at all. > The intention of this patch is a precursor to using it in avcodec. Specifically I intend to call this API so I can tell the libjxl encoder what primaries/etc. to use when it converts the pixel data to XYB internally. I don't know if it will be used in avformat, but it could be if needed. - Leo Izen (thebombzen)
On Fri, 20 May 2022 08:53:35 -0300 James Almer <jamrial@gmail.com> wrote: > > > On 5/20/2022 7:28 AM, 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 > > > > 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 > > Not making it public allows us to not commit to a fixed design *now*. > Unless there's a clear need for this to be part of the public API, i > don't think it's a good idea to do so. This change is being made because > this API is afaict needed in lavc, and not by some project using lavu. I see it being useful outside FFmpeg. > > Removing/changing an avpriv symbol or struct is a matter or waiting for > the nearest major bump. No need for an arbitrary 2 year wait period, > compat wrappers, or anything crazy. > > > > > 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 > > > > > > [...] > > > > > > _______________________________________________ > > 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".
On Fri, 20 May 2022 12:28:15 +0200 Michael Niedermayer <michael@niedermayer.cc> 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".
diff --git a/libavfilter/colorspace.c b/libavfilter/colorspace.c index 8d7b882375..d6de313a5c 100644 --- a/libavfilter/colorspace.c +++ b/libavfilter/colorspace.c @@ -65,8 +65,8 @@ void ff_matrix_mul_3x3(double dst[3][3], /* * see e.g. http://www.brucelindbloom.com/index.html?Eqn_RGB_XYZ_Matrix.html */ -void ff_fill_rgb2xyz_table(const struct PrimaryCoefficients *coeffs, - const struct WhitepointCoefficients *wp, +void ff_fill_rgb2xyz_table(const AVPrimaryCoefficients *coeffs, + const AVWhitepointCoefficients *wp, double rgb2xyz[3][3]) { double i[3][3], sr, sg, sb, zw; @@ -107,95 +107,7 @@ static const double gbr_matrix[3][3] = { 0.5, -0.5, 0 }, }; -/* - * All constants explained in e.g. https://linuxtv.org/downloads/v4l-dvb-apis/ch02s06.html - * The older ones (bt470bg/m) are also explained in their respective ITU docs - * (e.g. https://www.itu.int/dms_pubrec/itu-r/rec/bt/R-REC-BT.470-5-199802-S!!PDF-E.pdf) - * whereas the newer ones can typically be copied directly from wikipedia :) - */ -static const struct LumaCoefficients luma_coefficients[AVCOL_SPC_NB] = { - [AVCOL_SPC_FCC] = { 0.30, 0.59, 0.11 }, - [AVCOL_SPC_BT470BG] = { 0.299, 0.587, 0.114 }, - [AVCOL_SPC_SMPTE170M] = { 0.299, 0.587, 0.114 }, - [AVCOL_SPC_BT709] = { 0.2126, 0.7152, 0.0722 }, - [AVCOL_SPC_SMPTE240M] = { 0.212, 0.701, 0.087 }, - [AVCOL_SPC_YCOCG] = { 0.25, 0.5, 0.25 }, - [AVCOL_SPC_RGB] = { 1, 1, 1 }, - [AVCOL_SPC_BT2020_NCL] = { 0.2627, 0.6780, 0.0593 }, - [AVCOL_SPC_BT2020_CL] = { 0.2627, 0.6780, 0.0593 }, -}; - -const struct LumaCoefficients *ff_get_luma_coefficients(enum AVColorSpace csp) -{ - const struct LumaCoefficients *coeffs; - - if (csp >= AVCOL_SPC_NB) - return NULL; - coeffs = &luma_coefficients[csp]; - if (!coeffs->cr) - return NULL; - - return coeffs; -} - -#define WP_D65 { 0.3127, 0.3290 } -#define WP_C { 0.3100, 0.3160 } -#define WP_DCI { 0.3140, 0.3510 } -#define WP_E { 1/3.0f, 1/3.0f } - -static const struct ColorPrimaries color_primaries[AVCOL_PRI_NB] = { - [AVCOL_PRI_BT709] = { WP_D65, { 0.640, 0.330, 0.300, 0.600, 0.150, 0.060 } }, - [AVCOL_PRI_BT470M] = { WP_C, { 0.670, 0.330, 0.210, 0.710, 0.140, 0.080 } }, - [AVCOL_PRI_BT470BG] = { WP_D65, { 0.640, 0.330, 0.290, 0.600, 0.150, 0.060 } }, - [AVCOL_PRI_SMPTE170M] = { WP_D65, { 0.630, 0.340, 0.310, 0.595, 0.155, 0.070 } }, - [AVCOL_PRI_SMPTE240M] = { WP_D65, { 0.630, 0.340, 0.310, 0.595, 0.155, 0.070 } }, - [AVCOL_PRI_SMPTE428] = { WP_E, { 0.735, 0.265, 0.274, 0.718, 0.167, 0.009 } }, - [AVCOL_PRI_SMPTE431] = { WP_DCI, { 0.680, 0.320, 0.265, 0.690, 0.150, 0.060 } }, - [AVCOL_PRI_SMPTE432] = { WP_D65, { 0.680, 0.320, 0.265, 0.690, 0.150, 0.060 } }, - [AVCOL_PRI_FILM] = { WP_C, { 0.681, 0.319, 0.243, 0.692, 0.145, 0.049 } }, - [AVCOL_PRI_BT2020] = { WP_D65, { 0.708, 0.292, 0.170, 0.797, 0.131, 0.046 } }, - [AVCOL_PRI_JEDEC_P22] = { WP_D65, { 0.630, 0.340, 0.295, 0.605, 0.155, 0.077 } }, -}; - -const struct ColorPrimaries *ff_get_color_primaries(enum AVColorPrimaries prm) -{ - const struct ColorPrimaries *p; - - if (prm >= AVCOL_PRI_NB) - return NULL; - p = &color_primaries[prm]; - if (!p->prim.xr) - return NULL; - - return p; -} - -enum AVColorPrimaries ff_detect_color_primaries(const struct ColorPrimaries *prm) -{ - double delta; - - for (enum AVColorPrimaries p = 0; p < AVCOL_PRI_NB; p++) { - const struct ColorPrimaries *ref = &color_primaries[p]; - if (!ref->prim.xr) - continue; - - delta = fabs(prm->prim.xr - ref->prim.xr) + - fabs(prm->prim.yr - ref->prim.yr) + - fabs(prm->prim.yg - ref->prim.yg) + - fabs(prm->prim.yg - ref->prim.yg) + - fabs(prm->prim.yb - ref->prim.yb) + - fabs(prm->prim.yb - ref->prim.yb) + - fabs(prm->wp.xw - ref->wp.xw) + - fabs(prm->wp.yw - ref->wp.yw); - - if (delta < 0.001) - return p; - } - - return AVCOL_PRI_UNSPECIFIED; -} - -void ff_fill_rgb2yuv_table(const struct LumaCoefficients *coeffs, +void ff_fill_rgb2yuv_table(const AVLumaCoefficients *coeffs, double rgb2yuv[3][3]) { double bscale, rscale; diff --git a/libavfilter/colorspace.h b/libavfilter/colorspace.h index 6959133a49..879518d242 100644 --- a/libavfilter/colorspace.h +++ b/libavfilter/colorspace.h @@ -20,43 +20,20 @@ #ifndef AVFILTER_COLORSPACE_H #define AVFILTER_COLORSPACE_H +#include "libavutil/csp.h" #include "libavutil/frame.h" #include "libavutil/pixfmt.h" #define REFERENCE_WHITE 100.0f -struct LumaCoefficients { - double cr, cg, cb; -}; - -struct PrimaryCoefficients { - double xr, yr, xg, yg, xb, yb; -}; - -struct WhitepointCoefficients { - double xw, yw; -}; - -struct ColorPrimaries { - struct WhitepointCoefficients wp; - struct PrimaryCoefficients prim; -}; - void ff_matrix_invert_3x3(const double in[3][3], double out[3][3]); void ff_matrix_mul_3x3(double dst[3][3], const double src1[3][3], const double src2[3][3]); -void ff_fill_rgb2xyz_table(const struct PrimaryCoefficients *coeffs, - const struct WhitepointCoefficients *wp, +void ff_fill_rgb2xyz_table(const AVPrimaryCoefficients *coeffs, + const AVWhitepointCoefficients *wp, double rgb2xyz[3][3]); - -/* Returns AVCOL_PRI_UNSPECIFIED if no clear match can be identified */ -enum AVColorPrimaries ff_detect_color_primaries(const struct ColorPrimaries *prm); - -const struct ColorPrimaries *ff_get_color_primaries(enum AVColorPrimaries prm); -const struct LumaCoefficients *ff_get_luma_coefficients(enum AVColorSpace csp); -void ff_fill_rgb2yuv_table(const struct LumaCoefficients *coeffs, +void ff_fill_rgb2yuv_table(const AVLumaCoefficients *coeffs, double rgb2yuv[3][3]); - double ff_determine_signal_peak(AVFrame *in); void ff_update_hdr_metadata(AVFrame *in, double peak); diff --git a/libavfilter/fflcms2.c b/libavfilter/fflcms2.c index efc7cb5189..11b1524772 100644 --- a/libavfilter/fflcms2.c +++ b/libavfilter/fflcms2.c @@ -18,6 +18,7 @@ */ #include "libavutil/color_utils.h" +#include "libavutil/csp.h" #include "fflcms2.h" @@ -148,10 +149,10 @@ int ff_icc_profile_generate(FFIccContext *s, cmsHPROFILE *out_profile) { cmsToneCurve *tonecurve; - const struct ColorPrimaries *prim; + const AVColorPrimariesDesc *prim; int ret; - if (!(prim = ff_get_color_primaries(color_prim))) + if (!(prim = avpriv_get_color_primaries(color_prim))) return AVERROR_INVALIDDATA; if ((ret = get_curve(s, color_trc, &tonecurve)) < 0) return ret; @@ -202,7 +203,7 @@ static av_always_inline void XYZ_xy(cmsCIEXYZ XYZ, double *x, double *y) } int ff_icc_profile_read_primaries(FFIccContext *s, cmsHPROFILE profile, - struct ColorPrimaries *out_primaries) + AVColorPrimariesDesc *out_primaries) { static const uint8_t testprimaries[4][3] = { { 0xFF, 0, 0 }, /* red */ @@ -211,8 +212,8 @@ int ff_icc_profile_read_primaries(FFIccContext *s, cmsHPROFILE profile, { 0xFF, 0xFF, 0xFF }, /* white */ }; - struct WhitepointCoefficients *wp = &out_primaries->wp; - struct PrimaryCoefficients *prim = &out_primaries->prim; + AVWhitepointCoefficients *wp = &out_primaries->wp; + AVPrimaryCoefficients *prim = &out_primaries->prim; cmsFloat64Number prev_adapt; cmsHPROFILE xyz; cmsHTRANSFORM tf; diff --git a/libavfilter/fflcms2.h b/libavfilter/fflcms2.h index ad6c8c47cf..0d238c679f 100644 --- a/libavfilter/fflcms2.h +++ b/libavfilter/fflcms2.h @@ -25,9 +25,9 @@ #ifndef AVFILTER_FFLCMS2_H #define AVFILTER_FFLCMS2_H +#include "libavutil/csp.h" #include "libavutil/frame.h" #include "libavutil/pixfmt.h" -#include "colorspace.h" #include <lcms2.h> @@ -72,7 +72,7 @@ int ff_icc_profile_attach(FFIccContext *s, cmsHPROFILE profile, AVFrame *frame); * Returns 0 on success, or a negative error code. */ int ff_icc_profile_read_primaries(FFIccContext *s, cmsHPROFILE profile, - struct ColorPrimaries *out_primaries); + AVColorPrimariesDesc *out_primaries); /** * Attempt detecting the transfer characteristic that best approximates the diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c index 3c8b3b20eb..2138cfa1e4 100644 --- a/libavfilter/vf_colorspace.c +++ b/libavfilter/vf_colorspace.c @@ -24,6 +24,7 @@ */ #include "libavutil/avassert.h" +#include "libavutil/csp.h" #include "libavutil/mem_internal.h" #include "libavutil/opt.h" #include "libavutil/pixdesc.h" @@ -126,7 +127,7 @@ typedef struct ColorSpaceContext { unsigned rgb_sz; int *dither_scratch[3][2], *dither_scratch_base[3][2]; - const struct ColorPrimaries *in_primaries, *out_primaries; + const AVColorPrimariesDesc *in_primaries, *out_primaries; int lrgb2lrgb_passthrough; DECLARE_ALIGNED(16, int16_t, lrgb2lrgb_coeffs)[3][3][8]; @@ -134,7 +135,7 @@ typedef struct ColorSpaceContext { int rgb2rgb_passthrough; int16_t *lin_lut, *delin_lut; - const struct LumaCoefficients *in_lumacoef, *out_lumacoef; + const AVLumaCoefficients *in_lumacoef, *out_lumacoef; int yuv2yuv_passthrough, yuv2yuv_fastmode; DECLARE_ALIGNED(16, int16_t, yuv2rgb_coeffs)[3][3][8]; DECLARE_ALIGNED(16, int16_t, rgb2yuv_coeffs)[3][3][8]; @@ -233,8 +234,8 @@ static int fill_gamma_table(ColorSpaceContext *s) * This function uses the Bradford mechanism. */ static void fill_whitepoint_conv_table(double out[3][3], enum WhitepointAdaptation wp_adapt, - const struct WhitepointCoefficients *wp_src, - const struct WhitepointCoefficients *wp_dst) + const AVWhitepointCoefficients *wp_src, + const AVWhitepointCoefficients *wp_dst) { static const double ma_tbl[NB_WP_ADAPT_NON_IDENTITY][3][3] = { [WP_ADAPT_BRADFORD] = { @@ -438,7 +439,7 @@ static int create_filtergraph(AVFilterContext *ctx, s->in_prm = default_prm[FFMIN(s->user_iall, CS_NB)]; if (s->user_iprm != AVCOL_PRI_UNSPECIFIED) s->in_prm = s->user_iprm; - s->in_primaries = ff_get_color_primaries(s->in_prm); + s->in_primaries = avpriv_get_color_primaries(s->in_prm); if (!s->in_primaries) { av_log(ctx, AV_LOG_ERROR, "Unsupported input primaries %d (%s)\n", @@ -446,7 +447,7 @@ static int create_filtergraph(AVFilterContext *ctx, return AVERROR(EINVAL); } s->out_prm = out->color_primaries; - s->out_primaries = ff_get_color_primaries(s->out_prm); + s->out_primaries = avpriv_get_color_primaries(s->out_prm); if (!s->out_primaries) { if (s->out_prm == AVCOL_PRI_UNSPECIFIED) { if (s->user_all == CS_UNSPECIFIED) { @@ -466,7 +467,7 @@ static int create_filtergraph(AVFilterContext *ctx, sizeof(*s->in_primaries)); if (!s->lrgb2lrgb_passthrough) { double rgb2xyz[3][3], xyz2rgb[3][3], rgb2rgb[3][3]; - const struct WhitepointCoefficients *wp_out, *wp_in; + const AVWhitepointCoefficients *wp_out, *wp_in; wp_out = &s->out_primaries->wp; wp_in = &s->in_primaries->wp; @@ -551,7 +552,7 @@ static int create_filtergraph(AVFilterContext *ctx, s->in_rng = in->color_range; if (s->user_irng != AVCOL_RANGE_UNSPECIFIED) s->in_rng = s->user_irng; - s->in_lumacoef = ff_get_luma_coefficients(s->in_csp); + s->in_lumacoef = avpriv_get_luma_coefficients(s->in_csp); if (!s->in_lumacoef) { av_log(ctx, AV_LOG_ERROR, "Unsupported input colorspace %d (%s)\n", @@ -564,7 +565,7 @@ static int create_filtergraph(AVFilterContext *ctx, if (!s->out_lumacoef) { s->out_csp = out->colorspace; s->out_rng = out->color_range; - s->out_lumacoef = ff_get_luma_coefficients(s->out_csp); + s->out_lumacoef = avpriv_get_luma_coefficients(s->out_csp); if (!s->out_lumacoef) { if (s->out_csp == AVCOL_SPC_UNSPECIFIED) { if (s->user_all == CS_UNSPECIFIED) { diff --git a/libavfilter/vf_iccdetect.c b/libavfilter/vf_iccdetect.c index fb7871f035..297de8496a 100644 --- a/libavfilter/vf_iccdetect.c +++ b/libavfilter/vf_iccdetect.c @@ -24,6 +24,7 @@ #include <lcms2.h> +#include "libavutil/csp.h" #include "libavutil/opt.h" #include "libavutil/pixdesc.h" @@ -69,7 +70,7 @@ static int iccdetect_filter_frame(AVFilterLink *inlink, AVFrame *frame) AVFilterContext *avctx = inlink->dst; IccDetectContext *s = avctx->priv; const AVFrameSideData *sd; - struct ColorPrimaries coeffs; + AVColorPrimariesDesc coeffs; cmsHPROFILE profile; int ret; @@ -98,7 +99,7 @@ static int iccdetect_filter_frame(AVFilterLink *inlink, AVFrame *frame) if (ret < 0) return ret; - s->profile_prim = ff_detect_color_primaries(&coeffs); + s->profile_prim = avpriv_detect_color_primaries(&coeffs); done: if (s->profile_prim != AVCOL_PRI_UNSPECIFIED) { diff --git a/libavfilter/vf_tonemap.c b/libavfilter/vf_tonemap.c index 1285dbaa4d..0cb0fa2bc0 100644 --- a/libavfilter/vf_tonemap.c +++ b/libavfilter/vf_tonemap.c @@ -27,6 +27,7 @@ #include <stdio.h> #include <string.h> +#include "libavutil/csp.h" #include "libavutil/imgutils.h" #include "libavutil/internal.h" #include "libavutil/intreadwrite.h" @@ -50,16 +51,6 @@ enum TonemapAlgorithm { TONEMAP_MAX, }; -static const struct LumaCoefficients luma_coefficients[AVCOL_SPC_NB] = { - [AVCOL_SPC_FCC] = { 0.30, 0.59, 0.11 }, - [AVCOL_SPC_BT470BG] = { 0.299, 0.587, 0.114 }, - [AVCOL_SPC_SMPTE170M] = { 0.299, 0.587, 0.114 }, - [AVCOL_SPC_BT709] = { 0.2126, 0.7152, 0.0722 }, - [AVCOL_SPC_SMPTE240M] = { 0.212, 0.701, 0.087 }, - [AVCOL_SPC_BT2020_NCL] = { 0.2627, 0.6780, 0.0593 }, - [AVCOL_SPC_BT2020_CL] = { 0.2627, 0.6780, 0.0593 }, -}; - typedef struct TonemapContext { const AVClass *class; @@ -68,7 +59,7 @@ typedef struct TonemapContext { double desat; double peak; - const struct LumaCoefficients *coeffs; + const AVLumaCoefficients *coeffs; } TonemapContext; static av_cold int init(AVFilterContext *ctx) @@ -249,7 +240,7 @@ static int filter_frame(AVFilterLink *link, AVFrame *in) } /* load original color space even if pixel format is RGB to compute overbrights */ - s->coeffs = &luma_coefficients[in->colorspace]; + s->coeffs = avpriv_get_luma_coefficients(in->colorspace); if (s->desat > 0 && (in->colorspace == AVCOL_SPC_UNSPECIFIED || !s->coeffs)) { if (in->colorspace == AVCOL_SPC_UNSPECIFIED) av_log(s, AV_LOG_WARNING, "Missing color space information, "); diff --git a/libavutil/Makefile b/libavutil/Makefile index 234de62a4b..74d21a8103 100644 --- a/libavutil/Makefile +++ b/libavutil/Makefile @@ -20,6 +20,7 @@ HEADERS = adler32.h \ common.h \ cpu.h \ crc.h \ + csp.h \ des.h \ detection_bbox.h \ dict.h \ @@ -113,6 +114,7 @@ OBJS = adler32.o \ color_utils.o \ cpu.o \ crc.o \ + csp.o \ des.o \ detection_bbox.o \ dict.o \ diff --git a/libavutil/csp.c b/libavutil/csp.c new file mode 100644 index 0000000000..9af371b9d2 --- /dev/null +++ b/libavutil/csp.c @@ -0,0 +1,112 @@ +/* + * Copyright (c) 2016 Ronald S. Bultje <rsbultje@gmail.com> + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include <math.h> +#include <stddef.h> + +#include "csp.h" +#include "pixfmt.h" + +/* + * All constants explained in e.g. https://linuxtv.org/downloads/v4l-dvb-apis/ch02s06.html + * The older ones (bt470bg/m) are also explained in their respective ITU docs + * (e.g. https://www.itu.int/dms_pubrec/itu-r/rec/bt/R-REC-BT.470-5-199802-S!!PDF-E.pdf) + * whereas the newer ones can typically be copied directly from wikipedia :) + */ +static const struct AVLumaCoefficients luma_coefficients[AVCOL_SPC_NB] = { + [AVCOL_SPC_FCC] = { 0.30, 0.59, 0.11 }, + [AVCOL_SPC_BT470BG] = { 0.299, 0.587, 0.114 }, + [AVCOL_SPC_SMPTE170M] = { 0.299, 0.587, 0.114 }, + [AVCOL_SPC_BT709] = { 0.2126, 0.7152, 0.0722 }, + [AVCOL_SPC_SMPTE240M] = { 0.212, 0.701, 0.087 }, + [AVCOL_SPC_YCOCG] = { 0.25, 0.5, 0.25 }, + [AVCOL_SPC_RGB] = { 1, 1, 1 }, + [AVCOL_SPC_BT2020_NCL] = { 0.2627, 0.6780, 0.0593 }, + [AVCOL_SPC_BT2020_CL] = { 0.2627, 0.6780, 0.0593 }, +}; + +const struct AVLumaCoefficients *avpriv_get_luma_coefficients(enum AVColorSpace csp) +{ + const AVLumaCoefficients *coeffs; + + if (csp >= AVCOL_SPC_NB) + return NULL; + coeffs = &luma_coefficients[csp]; + if (!coeffs->cr) + return NULL; + + return coeffs; +} + +#define WP_D65 { 0.3127, 0.3290 } +#define WP_C { 0.3100, 0.3160 } +#define WP_DCI { 0.3140, 0.3510 } +#define WP_E { 1/3.0f, 1/3.0f } + +static const AVColorPrimariesDesc color_primaries[AVCOL_PRI_NB] = { + [AVCOL_PRI_BT709] = { WP_D65, { 0.640, 0.330, 0.300, 0.600, 0.150, 0.060 } }, + [AVCOL_PRI_BT470M] = { WP_C, { 0.670, 0.330, 0.210, 0.710, 0.140, 0.080 } }, + [AVCOL_PRI_BT470BG] = { WP_D65, { 0.640, 0.330, 0.290, 0.600, 0.150, 0.060 } }, + [AVCOL_PRI_SMPTE170M] = { WP_D65, { 0.630, 0.340, 0.310, 0.595, 0.155, 0.070 } }, + [AVCOL_PRI_SMPTE240M] = { WP_D65, { 0.630, 0.340, 0.310, 0.595, 0.155, 0.070 } }, + [AVCOL_PRI_SMPTE428] = { WP_E, { 0.735, 0.265, 0.274, 0.718, 0.167, 0.009 } }, + [AVCOL_PRI_SMPTE431] = { WP_DCI, { 0.680, 0.320, 0.265, 0.690, 0.150, 0.060 } }, + [AVCOL_PRI_SMPTE432] = { WP_D65, { 0.680, 0.320, 0.265, 0.690, 0.150, 0.060 } }, + [AVCOL_PRI_FILM] = { WP_C, { 0.681, 0.319, 0.243, 0.692, 0.145, 0.049 } }, + [AVCOL_PRI_BT2020] = { WP_D65, { 0.708, 0.292, 0.170, 0.797, 0.131, 0.046 } }, + [AVCOL_PRI_JEDEC_P22] = { WP_D65, { 0.630, 0.340, 0.295, 0.605, 0.155, 0.077 } }, +}; + +const AVColorPrimariesDesc *avpriv_get_color_primaries(enum AVColorPrimaries prm) +{ + const AVColorPrimariesDesc *p; + + if (prm >= AVCOL_PRI_NB) + return NULL; + p = &color_primaries[prm]; + if (!p->prim.xr) + return NULL; + + return p; +} + +enum AVColorPrimaries avpriv_detect_color_primaries(const AVColorPrimariesDesc *prm) +{ + double delta; + + for (enum AVColorPrimaries p = 0; p < AVCOL_PRI_NB; p++) { + const AVColorPrimariesDesc *ref = &color_primaries[p]; + if (!ref->prim.xr) + continue; + + delta = fabs(prm->prim.xr - ref->prim.xr) + + fabs(prm->prim.yr - ref->prim.yr) + + fabs(prm->prim.yg - ref->prim.yg) + + fabs(prm->prim.yg - ref->prim.yg) + + fabs(prm->prim.yb - ref->prim.yb) + + fabs(prm->prim.yb - ref->prim.yb) + + fabs(prm->wp.xw - ref->wp.xw) + + fabs(prm->wp.yw - ref->wp.yw); + + if (delta < 0.001) + return p; + } + + return AVCOL_PRI_UNSPECIFIED; +} diff --git a/libavutil/csp.h b/libavutil/csp.h new file mode 100644 index 0000000000..27e721fc66 --- /dev/null +++ b/libavutil/csp.h @@ -0,0 +1,48 @@ +/* + * Copyright (c) 2016 Ronald S. Bultje <rsbultje@gmail.com> + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#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; + +typedef struct AVColorPrimariesDesc { + AVWhitepointCoefficients wp; + AVPrimaryCoefficients prim; +} AVColorPrimariesDesc; + +/* Returns AVCOL_PRI_UNSPECIFIED if no clear match can be identified */ +enum AVColorPrimaries avpriv_detect_color_primaries(const AVColorPrimariesDesc *prm); + +const AVColorPrimariesDesc *avpriv_get_color_primaries(enum AVColorPrimaries prm); +const AVLumaCoefficients *avpriv_get_luma_coefficients(enum AVColorSpace csp); + +#endif /* AVUTIL_CSP_H */ diff --git a/libavutil/version.h b/libavutil/version.h index 6735c20090..dd7d20a9fa 100644 --- a/libavutil/version.h +++ b/libavutil/version.h @@ -79,8 +79,8 @@ */ #define LIBAVUTIL_VERSION_MAJOR 57 -#define LIBAVUTIL_VERSION_MINOR 24 -#define LIBAVUTIL_VERSION_MICRO 101 +#define LIBAVUTIL_VERSION_MINOR 25 +#define LIBAVUTIL_VERSION_MICRO 100 #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \ LIBAVUTIL_VERSION_MINOR, \