mpv-player / mpv

🎥 Command line video player
https://mpv.io
Other
27.84k stars 2.87k forks source link

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

Closed aufkrawall closed 1 year ago

aufkrawall commented 2 years ago

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

haasn commented 2 years ago

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)

aufkrawall commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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.

haasn commented 2 years ago

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

aufkrawall commented 2 years ago

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. :)

haasn commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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).

aufkrawall commented 2 years ago

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?

haasn commented 2 years ago

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.

aufkrawall commented 2 years ago

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).

haasn commented 2 years ago

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 commented 2 years ago

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.

OrangeColaJuice commented 2 years ago

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

haasn commented 2 years ago

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)

aufkrawall commented 2 years ago

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

haasn commented 2 years ago

@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.

aufkrawall commented 2 years ago

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).

haasn commented 2 years ago

@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.

aufkrawall commented 2 years ago

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?).

haasn commented 2 years ago

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.

aufkrawall commented 2 years ago

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 commented 2 years ago

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

haasn commented 2 years ago

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.

Hrxn commented 2 years ago

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

aufkrawall commented 2 years ago

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 commented 2 years ago

@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

OrangeColaJuice commented 2 years ago

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.

aufkrawall commented 2 years ago

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.

haasn commented 2 years ago

@aufkrawall I submitted https://code.videolan.org/videolan/libplacebo/-/merge_requests/281 which adds (optional, disabled by default) code to libplacebo to forcibly fix the black point by making the lower part of the ICC 3DLUT into a linear ramp.

This would fix your issue by brute force. Although in my own testing, I still didn't need it at all for your profile when using --icc-intent=0, since that already maps black to black. But give it a try.

You'll need to enable the feature manually to test it:

diff --git a/video/out/vo_gpu_next.c b/video/out/vo_gpu_next.c
index 8b2d783727..84d3684ecb 100644
--- a/video/out/vo_gpu_next.c
+++ b/video/out/vo_gpu_next.c
@@ -1532,6 +1532,7 @@ static void update_icc_opts(struct priv *p, const struct mp_icc_opts *opts)
     p->icc.size_r = s_r;
     p->icc.size_g = s_g;
     p->icc.size_b = s_b;
+    p->icc.force_bpc = true;

     if (!opts->profile || !opts->profile[0]) {
         // No profile enabled, un-load any existing profiles
aufkrawall commented 2 years ago

With these patches applied to libplacebo and mpv

Both --icc-intent=1

iccintent1

and --icc-intent=3 iccintent3

get way darker than DisplayCal BT.1886 3D LUT (absolute colorimetric): 3dcal

--icc-intent=0 seems to be unchanged, i.e. slightly brighter than DisplayCal 3D LUT: iccintent0

aufkrawall commented 2 years ago

As for 100% black: It is indeed now not elevated anymore, also not without --icc-intent=0 (and can also confirm deband black elevation of --vo=gpu-next is fixed too by the other recent libplacebo commits).

So only the slight brightness difference between DisplayCal 3D LUT and mpv ICC remains.

haasn commented 2 years ago

So only the slight brightness difference between DisplayCal 3D LUT and mpv ICC remains.

That one might be taken care of by libplacebo!284?

So do you think we should merge libplacebo!281?

aufkrawall commented 2 years ago

If libplacebo!281 doesn't have any bad side effects, I think it should be merged. As I think otherwise, there will elevated black for lots of users without transparent reason. I can't judge whether the ICC profile generated by DisplayCal is actually "bugged", but it's what most users use for calibration, and ideally things should just work as intended ootb. Hopefully this would also take care of some previous reports with ICC calibration in mpv, I suppose that's quite likely.

As for libplacebo!284 , I can't apply it after !281. :)

haasn commented 2 years ago

@aufkrawall I was actually contemplating switching to perceptual intent by default.

aufkrawall commented 2 years ago

Re-tested with libplacebo 0ce3fa4f28f41b20928476e6d7e09f5e611e7005 + MR 281 + mpv patch:

Solid black is not elevated with any --icc-intent.

A dark scene still shows differences vs. DisplayCal 3D LUT:

DisplayCal: 3dlutdc

--icc-intent=0 is still slightly brighter: 0

--icc-intent=1 is way darker: 1

--icc-intent=2 has similar brightness as 1, but looks more saturated: 2

--icc-intent=3 is once again way darker than DisplayCal 3D LUT: 3

aufkrawall commented 1 year ago

@haasn I wonder what your thoughts are about the forcibly linear ramp for the lower part of the ICC 3DLUT, like you described in #issuecomment-1228788748 ? Would only make sense behind an option, or not suitable at all? (Too late for mpv 0.35?)

aufkrawall commented 1 year ago

@aufkrawall I was actually contemplating switching to perceptual intent by default.

With --icc-intent=0, HDR gets much darker vs. current default intent setting (still same ICC profile as linked in the first post): Screenshot_20221001_043140 Screenshot_20221001_043121

This is with latest mpv and libplacebo git-master without any patches/MRs on top. Used with

tone-mapping=spline
tone-mapping-param=1.0
hdr-compute-peak=no
haasn commented 1 year ago

@aufkrawall is that still happening with https://code.videolan.org/videolan/libplacebo/-/merge_requests/296 ?

aufkrawall commented 1 year ago

Yes, mpv BT.1886 SDR is still a bit brighter than DisplayCal's absolute colorimetric with white point scaling 3D LUT with both --icc-intent=0 and 1, while HDR -> SDR still gets way darker with 0 vs. 1.

aufkrawall commented 1 year ago

With regard to pure black SDR: It is still elevated, i.e. visibly too bright without --icc-intent=0.

haasn commented 1 year ago

I did end up deciding to merge the upstream force_bpc patch, if only because I literally can't think of a better work-around here at the time.

I'll close this issue when the option is exposed in mpv. (Should we default it on or off?)

aufkrawall commented 1 year ago

Could you re-base the mpv patch in https://github.com/mpv-player/mpv/issues/10442#issuecomment-1228788748 ? It doesn't apply anymore.

I can't recall how the patch behaves with the default --icc-intent=1. If the result with the patch and --icc-intent=1 achieves proper black level without making HDR video way darker (like --icc-intent=0does without the patch), I think it would be the best solution (and imho a good default).

For what it's worth, who knows if the 3D LUT generated by Display Cal actually is correct? When the tool dwm_lut still worked with recent Windows versions at that time, I tested some 3D LUTs generated by DisplayCal with its verification feature, and gamma curve deviation was not perfect.

What I can say though is that the gamma correction done by novideo_srgb seems to be very good (and VCGT surprisingly bad), according to verification in DisplayCal: https://github.com/ledoge/novideo_srgb/issues/18#issuecomment-1265931793

Too bad we can't just simply feed it mpv's calibration for verification (or can we?).

haasn commented 1 year ago

Could you re-base the mpv patch in #10442 (comment) ? It doesn't apply anymore.

diff --git a/video/out/vo_gpu_next.c b/video/out/vo_gpu_next.c
index 22950e0b6e..75c2561b6e 100644
--- a/video/out/vo_gpu_next.c
+++ b/video/out/vo_gpu_next.c
@@ -1628,6 +1628,9 @@ static void update_icc_opts(struct priv *p, const struct mp_icc_opts *opts)
     p->icc.cache_save = icc_save;
     p->icc.cache_load = icc_load;
 #endif
+#if PL_API_VER >= 228
+    p->icc.force_bpc = true;
+#endif

     if (!opts->profile || !opts->profile[0]) {
         // No profile enabled, un-load any existing profiles
aufkrawall commented 1 year ago

Weird. Unlike previously https://github.com/mpv-player/mpv/issues/10442#issuecomment-1229341616 , now black is still elevated with that patch without --icc-intent=0.

haasn commented 1 year ago

Stupid me, I typo'd the #if, it should be 228, not 288

haasn commented 1 year ago

Oh, there is also a bug in libplacebo if you're using the icc cache dir option. I'll fix that as well.

aufkrawall commented 1 year ago

Thanks! No more elevated blacks with --icc-intent=1 and HDR still has proper brightness too vs. --icc-intent=0. :) So my 2 cents would be that this is the best deal when using ICC profiles created by DisplayCal/ArgyllCMS.

haasn commented 1 year ago

I will also point out that my "well-calibrated" ICC profile does not visibly get altered by the patch because it's already a near-perfect pure power curve, so the patch (which basically forces the 3DLUT to be a perfect pure power curve near the black point) does nothing.

So turning it on doesn't seem to harm anything, either.