Enable experimental HDR on macOS 10.15+ by anastasiuspernat · Pull Request #8387 · mpv-player/mpv

anastasiuspernat · 2020-12-11T02:18:38Z

Main discussion:
#7341

Added two new flags --tone-mapping=hdrpass to enable full HDR range display on supported HDR monitors and --tone-mapping=hdrscale to enable HDR and maximize brightness.

Using either of these flags activates HDR mode, disables clipping in video shader and rescales rgb values to the whole HDR range. I couldn't find a way to pass NSScreen into pass_draw_to_screen in video.c. So using 3.0 as a hard-coded max value. It should be set to 5.0 for Apple XDR Monitor or taken from NSScreen.maximumPotentialExtendedDynamicRangeColorComponentValue.

haasn

Overall summery is that this highlights a shortcoming of the mpv tone mapping algorithm (lack of sig_scale), but this is not the way to implement it, IMHO.

haasn · 2020-12-11T08:14:48Z

video/out/gpu/video.c

+            // HDR Passthrough - render HDR content based on peak level
+            {"hdrpass",  TONE_MAPPING_HDR_PASSTHROUGH},
+            // HDR Scale - render HDR content and ignore peak level, this way any content can become exended
+            {"hdrscale", TONE_MAPPING_HDR_SCALE})},


These should not be tone mapping curves. Rather, tone mapping should be disabled for true HDR output. (Unless the source peak exceeds the HDR output's target peak, in which case BT.2390 should be used)

Do you think a new switch should be added then?

haasn · 2020-12-11T08:16:23Z

video/out/gpu/video.c

+        if (p->opts.tone_map.curve != TONE_MAPPING_HDR_PASSTHROUGH && p->opts.tone_map.curve != TONE_MAPPING_HDR_SCALE)
+        {
+            GLSL(color.rgb = clamp(color.rgb, 0.0, 1.0);)
+        }


Rather than hackily implementing it like this, we could probably just relax the clamp(color.rgb, 0.0, 1.0); to color.rgb = max(color.rgb, 0.0). The clip to positive values is still needed because gamma functions are undefined for negative values.

Yeah, I see. Sure!

haasn · 2020-12-11T08:18:40Z

video/out/gpu/video.c

+    if (p->opts.tone_map.curve == TONE_MAPPING_HDR_PASSTHROUGH || p->opts.tone_map.curve == TONE_MAPPING_HDR_SCALE)
+    {
+        // TODO: this value (3.0) must be taken from NSScreen.screen.maximumPotentialExtendedDynamicRangeColorComponentValue
+         GLSLF("color.rgb = color.rgb * %f\n;", 3.0);


This is almost definitely not the correct way of doing things. I'd need to read into the extended HDR specs that you're trying to implement to know a better way, but if it's implied that you should be stretching SDR gamma functions onto HDR outputs, it might be a case of adding the concept of sig_scale (similar to the same field in libplacebo) to mpv's color space struct as well.

sry if i misunderstand, but the SDR range should definitely not be stretched to the HDR output but should stay between 0 and 1.0. at least that's how apple wants it.

So probably color space approach then? Set the extended color space on the target layer and keep values within 0 and 1.0? I tried setting itur_709 and itur_2020 color spaces, both resulted in very weird colors. What kind of color space should I apply?

it depends entirely on the source and what we output to to the layer. i didn't look intoit but i believe target-trc and/or target-prim need/can be set appropriately.

we probably need an approach similar to #5804

So far couldn't make it work. I have an .mp4 HDR footage that displays following data in Media Information:

Color primaries: ITU-R BT.2020 Color transfer function: SMPTE ST2084 (PQ) Color space: ITU-R BT.2020 Range

So I assume it means "BT.2020 color space with PQ gamma curve". I tried assigning CGColorSpace.itur_2020_PQ_EOTF and CGColorSpace.extendedLinearITUR_2020 to layer?.colorspace in updateICCProfile as you pointed out. Here please see the attached results:

On the top is correct HDR that is reproduced by QuickTime and also using this code from pull request. Both results on the bottom are reproduced in HDR as well but none of them is right.

I suspect the gamma curve has to be adjusted. Can you please point me in the further direction? Thank you!

~~Nevermind! I think I made it work, seems it's actually much easier than it seems haha. Will add another pull request later on!~~

Unfortunately no. It didn't work.

Here are my findings:
The only target color space that looked HDR is CGColorSpace.itur_2020_PQ_EOTF but it resulted in wrong gamma curve (see image above).
Adding --target-prim=bt.2020 didn't change anything.
Adding --target-trc=pq resulted in non-HDR image.

I still need some support here! Thanks again.

haasn · 2020-12-11T08:19:04Z

video/out/gpu/video_shaders.c

+    if (opts->curve == TONE_MAPPING_HDR_PASSTHROUGH && opts->curve == TONE_MAPPING_HDR_SCALE)
+    {
+        // Disable any clipping on HDR content
+        GLSLF("vec3 sig = color.rgb;\n");


Shouldn't you still clip to the new sig_peak of 3.0?

Yeah, I do that in pass_tone_map.

Akemi · 2020-12-11T16:19:22Z

video/out/cocoa/video_view.m

    }
+    // Experimental #HDR on #macOS        
+    // This must be enabled to be able to display HDR content
+    self.wantsExtendedDynamicRangeOpenGLSurface = true;


you can scrap the changes in this file. it's the old deprecated backend, that is also broken.

Akemi · 2020-12-11T16:44:36Z

I couldn't find a way to pass NSScreen into pass_draw_to_screen in video.c. So using 3.0 as a hard-coded max value. It should be set to 5.0 for Apple XDR Monitor or taken from NSScreen.maximumPotentialExtendedDynamicRangeColorComponentValue

i think that is not true. it should neither be 3.0, 5.0 nor NSScreen.maximumPotentialExtendedDynamicRangeColorComponentValue. it should use the actual and current maximum value. the actual maximum we want to render into is not static and can change dynamically based on the light condition etc for example. this can change on the fly and while watching a video and needs to be changeable at any given time.

this is how i understand it, but don't quote me on that since i still need to test it:
maximumPotentialExtendedDynamicRangeColorComponentValue: just the max value in EDR that is possible (at creation time of the NSScreen object), not the value that is currently used. everything higher than the actual value will activate the system tonemapping and your result might/will be false. so we should never use this value for our maximum. this value is good to check if EDR/HRD is actually support since it will be greater than 1.0

maximumReferenceExtendedDynamicRangeColorComponentValue: this is the current (at the requested time) potential maximum value of the display, which is not determined at creation time of the NSScreen object. this value might differ depending on the display setting and is possibly the same as maximumPotentialExtendedDynamicRangeColorComponentValue. it is not the currently used one.

maximumExtendedDynamicRangeColorComponentValue: this is the actual and currently used maximum value that should be used. it changes dynamically and a change notification can be registered.

anastasiuspernat · 2020-12-12T00:51:18Z

Great! Thanks a lot for the feedback. I was definitely shooting in the dark. To my surprise it worked out so quickly - you have a really well managed codebase.

This was referenced Dec 11, 2020

utilise macOS HDR/EDR functionality #7341

Closed

HDR not being output on an Apple Pro XDR display ? iina/iina#3236

Open

haasn reviewed Dec 11, 2020

View reviewed changes

Akemi reviewed Dec 11, 2020

View reviewed changes

anastasiuspernat closed this Dec 15, 2020

anastasiuspernat force-pushed the master branch from df2878b to 4029a69 Compare December 15, 2020 19:06

Conversation

anastasiuspernat commented Dec 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

haasn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Akemi Dec 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anastasiuspernat Dec 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anastasiuspernat Dec 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Akemi commented Dec 11, 2020

Uh oh!

anastasiuspernat commented Dec 12, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

anastasiuspernat commented Dec 11, 2020 •

edited

Loading

Akemi Dec 11, 2020 •

edited

Loading

anastasiuspernat Dec 15, 2020 •

edited

Loading

anastasiuspernat Dec 15, 2020 •

edited

Loading