error coefficients in p3Luminance function

[JunLai1]

p3Luminance uses following coefficients:

// See SMPTE EG 432-1, Equation 7-8.
static const float kP3R = 0.20949f, kP3G = 0.72160f, kP3B = 0.06891f;

However, the NPM in Equation 7-8 is inferred based on white point (0.314, 0.351), which is P3-DCI, not Display-P3 (0.3127, 0.3290). From the context and comments in gainmapmath.cpp, the P3 used by this library should refer to Display-P3. If so, the following coefficients should be used according to SMPTE EG 432-1, Equation G-7:

static const float kP3R = 0.22897f, kP3G = 0.69174f, kP3B = 0.07929f;

In addition, there is a question here. The coefficients used in p3RgbToYuv function is BT601. The comment says:

// Unfortunately, calculation of luma signal differs from calculation of
// luminance for Display-P3, so we can't reuse p3Luminance here.
static const float kP3YR = 0.299f, kP3YG = 0.587f, kP3YB = 0.114f;
static const float kP3Cb = 1.772f, kP3Cr = 1.402f;

As I understand it, the reason of P3`s calculation of luma signal differs from luminance is that Display-P3 standard does not define the RgbToYuv coefficients explicitly. The same calculation can be used for BT601, BT709, BT2100 as they all have RgbToYuv coefficients explicitly defined. In my view, cofficients calculation in BT* series is according to RP177 protocol by given color primaries, the Display-P3 RgbToYuv coefficients according to RP177 is:

// same with SMPTE EG 432-1, Equation G-7
static const float kP3YR = 0.229f, kP3YG = 0.6917f, kP3YB = 0.0793f;
static const float kP3Cb = 1.8414f, kP3Cr = 1.542f;

May be this coefficients shoud be used in p3RgbToYuv. In my experiments, the two version coefficients result in different jpeg visuals.

If I undersand wrongly, could you give me an explanation why BT601 is used here instead of BT709 or something else? Thx

[JunLai1]

Hello, @DichenZhang1 , Do you have time to review this issue? Thx.

[ram-mohan]

@JunLai1 thank you for pointing this out. Upon going through the DataFormat spec of khronos, it seems you are correct. The reason for using bt601 weights for display p3 was because in some obscure android code a similar approach was followed. Further, we have a work in progress CL that updates the weights, please take a look. https://github.com/ittiam-systems/libultrahdr/tree/DispP3

@JunLai1 some notes on how the current choices were made. https://android.googlesource.com/platform/frameworks/native/+/0db53ee3c9ae908d14c09290a4fb51036df25620%5E%21/

[ram-mohan]

@JunLai1 https://www.kernel.org/doc/html/v4.8/media/uapi/v4l/pixfmt-007.html#colorspace-dci-p3-v4l2-colorspace-dci-p3.
This link has some interesting notes with respect to DCI-P3.
Quoting from there, This colorspace does not specify a Y’CbCr encoding since it is not meant to be encoded to Y’CbCr. So a different YCbCr encoding is picked.
Although the current space is Display-P3 which is a variant of DCI-P3 with different white point, perhaps the same idea applies? And using ITU H273 document to compute Kr and Kb from DCI-P3 primaries is not the way to go?

[ram-mohan]

@JunLai1 The existing implementation for luminance computation is using DCI P3 weights instead of Display P3. This needs to be corrected. The weights to be used in p3Luminance computation are (0.228975, 0.691738, 0.079287). However as pointed out using Kr = 0.228975 and Kb = 0.079287 (derived from equations 39 to 44 of itu h.273) for RGB to YCbCr conversion may not be correct from my understanding.

As indicated in here and also in earlier comments, DCI P3 does not specify a Y’CbCr encoding since it is not meant to be encoded to Y’CbCr. So a different YCbCr encoding is picked. This is true even for Display P3. Not always the matrices are derived from primaries. For instance, Netflix uses P3 D65 primaries with BT.2020-NCL or BT.2020 matrices perhaps an indication of matrices not derived from primaries. Also bt601 matrices are not derived from bt601 525 or bt601 625 line primaries instead they are derived from system m or ntsc 1953 primaries.

If the ycbcr encoding is not specified what will be picked could be dependent on resolution. For sd content bt601, for hd content bt709 and for uhd bt2020 is preferred. Here the underlying representation of sdr intent is jpeg and jpeg ycbcr representation aligns with bt601, p3 could be choosing the same.

I will issue a pull request that fixes the p3Luminance() computation. The p3RgbToYuv and p3YuvToRgb, I feel should remain as-is. Will wait for others opinion.

[JunLai1]

@ram-mohan Thanks for your reply. I am not familiar with ITU H273 document, but I insist that the coefficients should be corrected due to:

1. RP177 specification defines a general mathematical method for calculating luminance coefficients, whose inputs are four primary colors, namely r, g, b and w. BT709 and BT2100 follow this method and define their rgb to yuv coefficients. So on my view, if the definitions of r, g, b and w are known, the luminance coefficients can be determined mathematically.

2. BT601`s use of NTSC coefficients is a historical issue. In theory, the coefficients should be redefined accroding BT601 gamut and white point. But NTSC was published in 1953, when BT601 was published in 1966 (or after), NTSC had widely been used in TV, hence the coefficients was remained forcely.

Showing that Display P3 does not define the luminance coefficients, in which case following mathematics in RP177 may be a reasonable way.