mpv icon indicating copy to clipboard operation
mpv copied to clipboard

BT.1886 slightly brighter and more banding with ICC profile vs. pre-generated 3D LUT

Open aufkrawall opened this issue 3 years ago • 37 comments

mpv 0.34.0-378-g8ef744d1b7 recent libplacebo git-master

mpv config:

vo=gpu-next
gpu-api=vulkan

dither-depth=8

#icc-profile=profile.icm
#vf=format:gamma=bt.1886

image-lut=bt1886.cube

keep-open=yes

With mpv's own generated 3D LUT, especially dark shades are slightly brighter and show a bit more banding vs. BT.1886 3D LUT generated by DisplayCal.

mpv ICC: icc

DisplayCal 3D LUT: 3dlut

Test image: orange

Log: output.txt

My display's ICC profile and 3D LUT generated by DisplayCal (with latest stable version on Windows): icc3dlut.zip

aufkrawall avatar Jul 23 '22 22:07 aufkrawall

The amount of banding in your screenshots seems roughly equal to me. But I do see that the mpv result is just a tiny bit brighter.

Can you test on a black clipping sample? (e.g. one with labelled steps from 0 to 16)

haasn avatar Jul 23 '22 23:07 haasn

I was testing that, then I noticed this: With ICC correction applied, mpv elevates black. This also happens with vf=format:gamma=gamma2.4.

Black with ICC profile (same as the one linked above) correction in mpv: icc2 4

BT.1886 3D LUT by DisplayCal: 3dlut

"Source" (1 color black): black

I think that ICC profile should be fine and shouldn't make mpv assume elevated black level.

aufkrawall avatar Jul 24 '22 00:07 aufkrawall

Can also confirm the issue on Windows with --gpu-api=d3d11 and --vo=gpu. Can also confirm that mpv elevates black with an ICC profile in 3x curves format without included VCGT data, though then black is elevated a bit less. Also happens without any dithering.

aufkrawall avatar Jul 25 '22 00:07 aufkrawall

This issue occurs (even more distinctively?) also with a sample video that represents normal video (i.e. rec709 bt1886 content): 2022-07-30 00-27-21.zip

Just press s (or capture screen) one time with and one time without ICC profile correction enabled in mpv. With it enabled, clearly black gets elevated when comparing the screenshots. Is also the case with --vo=gpu and --icc-force-contrast=inf.

Could you please take a look before next stable build is declared @haasn ? It looks to me like mpv ICC correction undesirably raises blacks and thus reduces contrast with an ICC profile. From what I can tell, it looks to be generic and shouldn't be limited to only my ICC profile, and apparently happens with any gamma setting/target.

aufkrawall avatar Jul 29 '22 22:07 aufkrawall

It's on my todo list for sure, I just can't make any guarantees about when I'll have the energy to go through it

haasn avatar Jul 29 '22 23:07 haasn

Thanks, that's reassuring to know. I really wonder why I didn't notice this earlier. Perhaps because 100% black isn't a too common encounter in the wild? Anyway, don't let me nag you. I'll gladly wait. :)

aufkrawall avatar Jul 29 '22 23:07 aufkrawall

I really wonder why I didn't notice this earlier.

On my ICC profile the black raises from (0,0,0) to (1,1,1) after passing through the LUT. On your ICC profile, it gets raised to (5,5,3).

Incidentally, if I bump the LUT size up to 256x256x256 then it ends up as (0,0,0) on my profile and (3,5,1) on yours.

haasn avatar Aug 04 '22 11:08 haasn

Logs from my ICC profile:

detected ICC black point: XYZ 0.000824 0.000854 0.0007050
detected ICC white point: XYZ 0.000000 89.975449 0.0000000
detected ICC black point: XYZ 0.000824 0.000854 0.000705
detected ICC contrast: 203.000000 : 0.173462 = 1170.285767 : 1
detected ICC gamma: 2.153214
transforming gray(0) -> 0 0 0
transforming gray(1) -> 0 0 0
transforming gray(2) -> 0 0 0
transforming gray(3) -> 0 0 0
transforming gray(4) -> 0 0 0
transforming gray(5) -> 0 0 0
transforming gray(6) -> 0 0 0
transforming gray(7) -> 0 0 0
transforming gray(8) -> 0 0 0
transforming gray(9) -> 10 11 9
transforming gray(10) -> 136 137 119
transforming gray(11) -> 409 409 348
transforming gray(12) -> 768 767 649
transforming gray(13) -> 1144 1143 972

Logs from yours:

detected ICC white point: XYZ 0.000000 94.100555 0.0000000
detected ICC black point: XYZ 0.000883 0.000916 0.000755
detected ICC contrast: 203.000000 : 0.185852 = 1092.266724 : 1
detected ICC gamma: 2.355127
transforming gray(0) -> 0 0 0
transforming gray(1) -> 0 0 0
transforming gray(2) -> 0 0 0
transforming gray(3) -> 0 0 0
transforming gray(4) -> 0 0 0
transforming gray(5) -> 0 0 0
transforming gray(6) -> 0 0 0
transforming gray(7) -> 0 0 0
transforming gray(8) -> 0 0 0
transforming gray(9) -> 0 0 0
transforming gray(10) -> 0 0 0
transforming gray(11) -> 0 0 0
transforming gray(12) -> 253 49 24
transforming gray(13) -> 694 1175 289
transforming gray(14) -> 1716 1507 1186
transforming gray(15) -> 2480 2478 2328
transforming gray(16) -> 3083 3056 2943

Tone mapping is based on the detected min luma value, i.e. 0.185852. This then gets normalized to 0.0009155270 and passed through the detected ICC gamma function of 1/2.355127, giving us 0.0512753 being indexed from the 3DLUT, corresponding to an index of roughly 13.07.

We can see that, somehow, this is too high. It's possible that the issue lies in a faulty black point detection logic.

haasn avatar Aug 04 '22 12:08 haasn

Indeed, if I manually override the black point by a value corresponding more closely to an index of 12 (which is where the true black point approximately lies, according to this 3DLUT), the black boost disappears.

So in conclusion, it appears that cmsDetectDestinationBlackPoint is not correctly estimating the black point of the ICC profile, and we probably want to write custom logic to do this instead (as we already do for the gamma detection).

haasn avatar Aug 04 '22 12:08 haasn

Thanks for looking into this!

Is my layman guess correct that 100% black shouldn't be elevated at all, but only near-blacks with BT.1886? It seems to be what the BT.1886 3D LUT generated by DisplayCal does. I suppose this generally should also apply to results of HDR -> SDR conversion?

aufkrawall avatar Aug 04 '22 12:08 aufkrawall

Is my layman guess correct that 100% black shouldn't be elevated at all

Yeah, fundamentally all tone curves are designed to map black to black after all. Although it becomes a question of what you mean by "elevated" - elevated relative to what?

Your display device has a black point of around 0.000916 nits, so the source (at 0.0000 nits for an "infinite contrast" SDR source) will have to be elevated to fit.

haasn avatar Aug 05 '22 10:08 haasn

Well, since this is the point of BT.1886 (and I think also SDR -> HDR tonemapping of mpv works similarly?), this makes sense to me if black still looks as black as the display can do. :) On the other hand, when using something like --vf=format:gamma=gamma2.4, I'd expect the result to be clipped below the display's black point (e.g. when content has weirdly bad contrast with BT.1886).

aufkrawall avatar Aug 05 '22 17:08 aufkrawall

Hmm. I tried various other approaches but I'm always hitting the bottleneck that LittleCMS does not seem to support black point compensation with relative colorimetric intent at all. I might end up needing to split up the input encoding per-channel, the way we used to do it in mpv iirc.

haasn avatar Aug 07 '22 10:08 haasn

The author of Little-CMS thinks your ICC profile is wrong, which is why you're getting such bad results.

I'm not sure what to do with this information. I'm still waiting on his feedback before deciding how to change our pipeline. But in the meantime, you might wanna use --icc-intent=0 to work around it.

haasn avatar Aug 08 '22 11:08 haasn

Sorry for interjecting but how does one get that log output for ICC information/generation of cube?

OrangeColaJuice avatar Aug 08 '22 11:08 OrangeColaJuice

Sorry for interjecting but how does one get that log output for ICC information/generation of cube?

I'm just using some hacky debug code

diff --git a/src/shaders/icc.c b/src/shaders/icc.c
index d1a3bdb2..2b6685f1 100644
--- a/src/shaders/icc.c
+++ b/src/shaders/icc.c
@@ -29,6 +29,7 @@ struct icc_priv {
     cmsContext cms;
     cmsHPROFILE profile;
     cmsHPROFILE approx; // approximation profile
+    float a, b, scale; // approxmation tone curve parameters and scaling
     cmsCIEXYZ black;
     float gamma_stddev;
 };
@@ -313,10 +314,10 @@ pl_icc_object pl_icc_open(pl_log log, const struct pl_icc_profile *profile,
         params->intent = cmsGetHeaderRenderingIntent(p->profile);
 
     struct pl_raw_primaries *out_prim = &icc->csp.hdr.prim;
-    if (!detect_csp(icc, out_prim, &icc->gamma))
-        goto error;
     if (!detect_contrast(icc, &icc->csp.hdr, params->max_luma))
         goto error;
+    if (!detect_csp(icc, out_prim, &icc->gamma))
+        goto error;
     infer_clut_size(log, icc);
 
     const struct pl_raw_primaries *best = NULL;
@@ -342,8 +343,19 @@ pl_icc_object pl_icc_open(pl_log log, const struct pl_icc_profile *profile,
         goto error;
     }
 
-    // Create approximation profile
-    cmsToneCurve *curve = cmsBuildGamma(p->cms, icc->gamma);
+    // Create approximation profile. Use a tone-curve based on a BT.1886-style
+    // pure power curve, with an approximation gamma matched to the ICC
+    // profile. We stretch the luminance range *before* the input to the gamma
+    // function, to avoid numerical issues near the black point. (This removes
+    // the need for a separate linear section)
+    //
+    // Y = scale * (aX + b)^y, where Y = PCS luma and X = encoded value ([0-1])
+    p->scale = pl_hdr_rescale(PL_HDR_NITS, PL_HDR_NORM, icc->csp.hdr.max_luma);
+    p->b = powf(icc->csp.hdr.min_luma / icc->csp.hdr.max_luma, 1.0f / icc->gamma);
+    p->a = (1 - p->b);
+    fprintf(stderr, "a: %f, b: %f, y: %f\n", p->a, p->b, icc->gamma); // XXX
+    cmsToneCurve *curve = cmsBuildParametricToneCurve(p->cms, 2,
+            (double[3]) { icc->gamma, p->a, p->b });
     if (!curve)
         goto error;
 
@@ -360,6 +372,8 @@ pl_icc_object pl_icc_open(pl_log log, const struct pl_icc_profile *profile,
     if (!p->approx)
         goto error;
 
+    //cmsSaveProfileToFile(p->approx, "/mem/input.icc");
+
     return icc;
 
 error:
@@ -379,6 +393,7 @@ static void fill_lut(void *datap, const struct sh_lut_params *params, bool decod
     cmsHTRANSFORM tf = cmsCreateTransformTHR(p->cms, srcp, TYPE_RGB_16,
                                              dstp, TYPE_RGBA_16,
                                              icc->params.intent,
+                                             cmsFLAGS_BLACKPOINTCOMPENSATION |
                                              cmsFLAGS_NOCACHE | cmsFLAGS_NOOPTIMIZE);
     if (!tf)
         return;
@@ -386,6 +401,10 @@ static void fill_lut(void *datap, const struct sh_lut_params *params, bool decod
     clock_t after_transform = clock();
     pl_log_cpu_time(log, start, after_transform, "creating ICC transform");
 
+    // XXX
+        float M[3] = {0}, S[3] = {0};
+        int k = 1;
+
     int s_r = params->width, s_g = params->height, s_b = params->depth;
     uint16_t *tmp = pl_alloc(NULL, s_r * 3 * sizeof(tmp[0]));
     for (int b = 0; b < s_b; b++) {
@@ -400,9 +419,36 @@ static void fill_lut(void *datap, const struct sh_lut_params *params, bool decod
             size_t offset = (b * s_g + g) * s_r * 4;
             uint16_t *data = ((uint16_t *) datap) + offset;
             cmsDoTransform(tf, tmp, data, s_r);
+
+            // XXX
+            if (b == g && s_b == s_g && s_r == s_b) {
+                int r = b;
+                fprintf(stderr, "lut[%d] = %d %d %d\n",
+                        r,
+                        (int) data[r*4+0],
+                        (int) data[r*4+1],
+                        (int) data[r*4+2]);
+
+                for (int j = 0; j < 3; j++) {
+                    float y = logf(data[r*4+j] / 65535.0) / logf(r / (s_r - 1.0));
+                    float tmpM = M[j];
+                    if (!isfinite(y))
+                        continue;
+                    M[j] += (y - tmpM) / k;
+                    S[j] += (y - tmpM) * (y - M[j]);
+                }
+                k++;
+            }
+            // XXX
         }
     }
 
+    // XXX
+    {
+        for (int j = 0; j < 3; j++)
+            fprintf(stderr, "lut est gamma %d: %f, stddev: %f\n", j, M[j], sqrt(S[j] / (k - 1)));
+    }
+
     pl_log_cpu_time(log, after_transform, clock(), "generating ICC 3DLUT");
     cmsDeleteTransform(tf);
     pl_free(tmp);
@@ -442,16 +488,20 @@ void pl_icc_decode(pl_shader sh, pl_icc_object icc, pl_shader_obj *lut_obj,
         .priv       = (void *) icc,
     ));
 
+    // Y = scale * (aX + b)^y
+    struct icc_priv *p = PL_PRIV(icc);
     sh_describe(sh, "ICC 3DLUT");
     GLSL("// pl_icc_decode                      \n"
          "{                                     \n"
          "color.rgb = %s(color.rgb).rgb;        \n"
-         "color.rgb = max(color.rgb, 0.0);      \n"
+         "color.rgb = %s * color.rgb + vec3(%s);\n"
          "color.rgb = pow(color.rgb, vec3(%s)); \n"
-         "color.rgb = %s * color.rgb;           \n"  // expand HDR levels
+         "color.rgb = %s * color.rgb;           \n"
          "}                                     \n",
-         lut, SH_FLOAT(icc->gamma),
-         SH_FLOAT(pl_hdr_rescale(PL_HDR_NITS, PL_HDR_NORM, icc->csp.hdr.max_luma)));
+         lut,
+         SH_FLOAT(p->a), SH_FLOAT(p->b),
+         SH_FLOAT(icc->gamma),
+         SH_FLOAT(p->scale));
 
     if (out_csp) {
         *out_csp = (struct pl_color_space) {
@@ -482,16 +532,21 @@ void pl_icc_encode(pl_shader sh, pl_icc_object icc, pl_shader_obj *lut_obj)
         .priv       = (void *) icc,
     ));
 
+    // X = 1/a * (Y/scale)^(1/y) - b/a
+    struct icc_priv *p = PL_PRIV(icc);
     sh_describe(sh, "ICC 3DLUT");
     GLSL("// pl_icc_encode                      \n"
          "{                                     \n"
-         "color.rgb = 1.0/%s * color.rgb;       \n"
          "color.rgb = max(color.rgb, 0.0);      \n"
+         "color.rgb = 1.0/%s * color.rgb;       \n"
          "color.rgb = pow(color.rgb, vec3(%s)); \n"
+         "color.rgb = 1.0/%s * color.rgb - %s;  \n"
          "color.rgb = %s(color.rgb).rgb;        \n"
          "}                                     \n",
-         SH_FLOAT(pl_hdr_rescale(PL_HDR_NITS, PL_HDR_NORM, icc->csp.hdr.max_luma)),
-         SH_FLOAT(1.0f / icc->gamma), lut);
+         SH_FLOAT(p->scale),
+         SH_FLOAT(1.0f / icc->gamma),
+         SH_FLOAT(p->a), SH_FLOAT(p->b / p->a),
+         lut);
 }
 
 #else // !PL_HAVE_LCMS

(This is intermixed with a change to the profile generation, but you can extract out the debug parts)

haasn avatar Aug 08 '22 11:08 haasn

Thanks again for your efforts, haasn!

This sounds like bad news to me, as my profiles are created via the GUI of DisplayCal with common i1 DisplayPro with recommended spectral correction for most wide gamut monitors (LCD PFS Phosphor WLED) without doing anything special. So probably lots of profiles out there would trigger this (to a different extent?). A bug in DisplayCal?

I can confirm that --icc-intent=0 works. I've btw. checked color values of pixels in the screenshots, they are 100% black with --icc-intent=0.

Edit: Hm, pre-generated DisplayCal 3D LUT is still a bit darker with real content than the ICC profile with --icc-intent=0: 3D LUT: Screenshot_20220808_153949

ICC: Screenshot_20220808_154209

aufkrawall avatar Aug 08 '22 13:08 aufkrawall

@aufkrawall something I noticed in my own testing is that enabling --debanding raises the black point in a strange and noticeable way - an encoded value of 16 (i.e. reference black) becomes a value slightly above black, while encoded values of 15 (or below) become true black.

I'll try investigating this further, but it's possible that you should disable debanding when testing for the time being.

In any case, I've pushed a branch here: https://code.videolan.org/videolan/libplacebo/-/merge_requests/272

You can give it a try and see what you think. This won't solve your issue out of the box (I suspect your profile is broken, cf. the linked Little-CMS discussion), but will make --icc-intent=0 work correctly.

haasn avatar Aug 10 '22 17:08 haasn

I've tested with this MR and both deband and dithering disabled, dark scenes are still a bit brighter with BT.1886 via mpv/libplacebo ICC correction than with 3D LUT generated by DisplayCal (same as above).

aufkrawall avatar Aug 10 '22 20:08 aufkrawall

@aufkrawall based on your OP settings, I believe you are applying the 3DLUT incorrectly.

A correct application would be --lut=bt1886nocal.cube --lut-type=conversion. This instructs mpv to use this LUT (instead of the ICC profile) for source->target gamut conversion. When I do this, I get identical results when using --icc-intent=0.

haasn avatar Aug 11 '22 11:08 haasn

I comment out icc-profile when applying the 3D LUT. lut-type=conversion doesn't seem to make a difference with at least regard to bare eyes impression. But I created the 3D LUT with DisplayCal's default intent, which is absolute colorimetric with white point scaling. So I tried with a 3D LUT generated with perceptual intent, but then for whatever weird reasons it seems BT.1886 was behaving like if there was no black point information available. I wonder if that all comes down to the same issue the little-cms dev thinks to be an issue of the ICC profile (and in return of DisplayCal?).

aufkrawall avatar Aug 11 '22 14:08 aufkrawall

In your OP settings you are using --image-lut instead of --lut, too. --lut-type only affects --lut, not --image-lut (that would be --image-lut-type)

Edit: Although, if the rest of the color pipeline is a no-op, it probably makes no difference in either case.

haasn avatar Aug 11 '22 14:08 haasn

The last two screenshots are from an actual video file. :) I think basically any very dark scene will do the trick, but I can also provide a few seconds split-off featuring that exact scene as a sample.

If I had to make a vague guess, I'd say it looks to me like that mpv's ICC BT.1886 perceptual intent treats the display's black point as slightly higher (thus boosts gamma more) than DisplayCal's BT.1886 3D LUT with absolute colorimetric intent.

aufkrawall avatar Aug 11 '22 14:08 aufkrawall

libplacebo-git-4.208.0.76.g1d3ff4d-1 mpv 813164cc07124aabfbc4aa3b8f9fe33fe222c77c

vo=gpu-next
gpu-api=vulkan

icc-profile="G27q-20 #1 2022-05-25 22-12 D6500 2.4 M-S XYZLUT+MTX.icm"
icc-intent=0

#lut=bt1886nocal.cube
#lut-type=conversion

keep-open=yes
screenshot-format=png

icc

vo=gpu-next
gpu-api=vulkan

#icc-profile="G27q-20 #1 2022-05-25 22-12 D6500 2.4 M-S XYZLUT+MTX.icm"
#icc-intent=0

lut=bt1886nocal.cube
lut-type=conversion

keep-open=yes
screenshot-format=png

3dlut

I think configs should be correct that way and, as you can see, BT.1886 result with mpv's own generated 3D LUT from the ICC pofile is indeed a bit brigther than with DisplayCal's pre-generated 3D LUT from the same ICC profile: Screenshot_20220817_010915 (same ICC profile and 3D LUT as used in my previous posts)

Sample video (I just used the last frame with keep-open=yes): sample.zip

aufkrawall avatar Aug 16 '22 23:08 aufkrawall

Random thought: mpv assumes untagged BT.1886 sources have a contrast of 1000:1 by default. So we actually do tone-mapping of the black point. Might be related, but might be not.

Looking closely at the screenshots though it almost seems as though the black point is shifting color? I'll have to do more in-depth analysis again but it's possible that the black point compensation still isn't working as intended with this profile.

haasn avatar Aug 16 '22 23:08 haasn

BTW, DisplayCAL 3.9.6, is that some kind of beta version?

Hrxn avatar Aug 17 '22 10:08 Hrxn

It's the fork with modernized Python used on Arch, original DisplayCal upstream seems to be abandonware these days. But it doesn't matter, as I used original Windows version 3.8.9.3 both for calibration and generating 3D LUT.

aufkrawall avatar Aug 17 '22 13:08 aufkrawall

@haasn Maybe it's interesting that DisplayCal's XYZ LUT profile type seems to contain something that looks like some kind of obligatory black point compensation, even though the checkbox is unticked? XYZ: xyz

Curves: 3xcurves

Developer of novideo_srgb mentioned this when some issues occurred, no idea if it perhaps can be helpful: https://github.com/ledoge/novideo_srgb

I'm also not getting a noticeable brightness increase with mpv ICC vs. DisplayCal pre-generated 3D LUT with a profile that's 3x curves profile with black point compensation checkbox unticked (it's darker than the other screenshots as it doesn't contain a VCGT; my XYZ matrix profile contains a VCGT that sets display gamma to 2.4 instead of its native 2.2): Screenshot_20220817_173322

Screenshot_20220817_173429

With that profile, it also doesn't seem to make a difference if --icc-intent=0 is set or not. But black isn't 100% black, but instead slightly (1/255?) above black (not noticeable with my relatively poor IPS black level though).

No idea if the missing luminance and black point data in DisplayCal's overview for the 3x curves profile with no VCGT is just some header data or if this has more relevance.

The 3x curves ICC profile without VCGT: G27q-20 #1 2022-05-25 16-04 D6500 S 3xCurve+MTX.zip

aufkrawall avatar Aug 17 '22 15:08 aufkrawall

Maybe it's interesting that DisplayCal's XYZ LUT profile type seems to contain something that looks like some kind of obligatory black point compensation

The default is to create colorimetric and perceptual tables. I think perceptual is equal to relative color + black point compensation. If you manually check BPC on perceptual it does pretty much nothing.

Also for the other guy, DisplayCal official build should be enough, it's pretty much a GUI for Argyll CMS.

OrangeColaJuice avatar Aug 18 '22 18:08 OrangeColaJuice

Re-ran calibration with gamma 2.4 VCGT in curves format, and indeed the brightness difference between mpv ICC and DisplayCal 3D LUT is still there.

aufkrawall avatar Aug 19 '22 13:08 aufkrawall