-
Notifications
You must be signed in to change notification settings - Fork 3.2k
vo_gpu_next: hardcode BT.1886 luminance values to Wayland defaults #17329
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@mahkoh: This ugly thing should work... for now. |
859c987 to
3f8c45d
Compare
Mesa's Wayland Vulakn WSI doesn't set luminances for SDR transfer, as they are only set for HDR. Which makes sense. In this case however, we have to use default BT.1886 luminance values as defined by Wayland protocol. I don't like it at all... but will do the job for now.
3f8c45d to
0ca20e9
Compare
|
why ? Edit the next day: I did found why eventually: Mesa WSI does indeed select |
See discussion in #17322, transfer function between mpv and compositor has to agree, else compositor will interpret it incorrectly. |
|
move that to |
So, you use default, and everything works. |
|
I've written a small utility to test that with this change the same output is produced whether the compositor supports bt1886 or not: https://github.com/mahkoh/wl-proxy/tree/master/apps/wl-cm-filter You'd use it like this to filter out the bt1886 transfer function: $ wl-cm-filter --transfer-functions bt1886 mpvHowever, when I did that, I noticed something weird: The OSD still showed that bt1886 was being used. So then I ran ~$ WAYLAND_DEBUG=1 mpv 2>&1 | rg set_tf_named
[3477007.694] {mesa vk display queue} -> wp_image_description_creator_params_v1#87.set_tf_named(9)
[3477051.106] {mesa vk display queue} -> wp_image_description_creator_params_v1#75.set_tf_named(9)And indeed even by default mpv always sets srgb as the transfer function and not bt1886 but still shows bt1886 as being used in the OSD. Can you explain what's going on here? |
but it does use the
Anyway, the mesa vulkan WSI gpu-next / vulkan will only set a tf if a color space other that VK_COLOR_SPACE_PASS_THOUGH is used. |
Is your input bt.1886 by any chance? So when mpv output sRGB, it will still by default not convert bt.1886 to sRGB. Should be fixed by Frankly it's not clear to me what to do with bt.709/bt.1886 content, I'm even inclined to always use infinite contrast (gamma 2.4) for this when converting to anything else. The big issue is that virtually all content, is tagged as bt.1886. PC capture, movies, everything... And for me the adaptation that it does, based on contrast is mostly useful only for TV content. |
I can check double check later what we set, but this is completely separate and different case to |
I tried the PQ test image, a PQ video, a movie, and an OBS screen recording and mpv always shows bt1886 as the display transfer function. For the PQ inputs, it shows PQ as the input transfer function. I tested the code in this PR and it is not being invoked for the test images from #17322 (comment). |
|
Hard to tell, could you dump a Generally flow is like so: select preferred or fallback to sRGB. Old libplacebo though was always using bt.1886 for EDIT: and by |
I don't know how to do that. |
Yes, it uses
Also not sure if you run mpv through your tool here, but |
|
Might want |
|
Note, that you can override preferred transfer with |
|
I can confirm that mpv git + libplacebo git |
Yes, because now it correctly sets bt.1886 transfer if requested. Generally bt.1886 output in libplacebo should only be used if the input is also bt.1886 to roundtrip back to original transfer. We already do tonemapping and bpc, we don't need bt.1886 to mess with this afterwards. Note that we have a single set of min/max lumiance, which is used both for tonemaping and later for encoding. And it looks like without setting the luminances for Wayland we would have to hardcode those values, which has adverse effect on HDR tonemapping. Back to the topic. I've tested some patterns, on some screen with low contrast, and I generally get proper output (tested on kwin) when doing: Now when For So realistically, we probably should set better |
bt1886 shouldn't be involved at all in the screenshots. It takes PQ input and gives srgb output to wayland. |
Which compositor? In this case you likely need I cannot tell you what For me it's |
|
My point is that the first screenshot looks vastly different from the second screenshot. Both were taken with the same set of parameters passed to mpv with the only difference being that one hardcodes 0.203 as hdr.min_luma and the other one hardcodes 10^-6 as hdr.min_luma. |
I don't yet understand what values the compositor would provide. The current behavior seems to be entirely controlled by the offset chosen by the client.
It's because compositors map primary color min luma in color space A to primary color min luma in color space B. I.e. SDR black in one color space is mapped to SDR black in the other. Let's say we did as you say: Most compositors send min=0.2 and ref=80. This would yield a contrast of 203/(0.2 * 203 / 80) = 80 / 0.2 = 400. However, KDE sends min=0.01 and ref=200. This would yield a contrast of 203/(0.01 * 203 / 200) = 200 / 0.01 = 20_000. |
Using Vulkan WSI we wouldn't be able to set values for SDR. But I guess it's not necessary, as we can render to the provided values. Sorry, I'm still procrastinating doing deeper dive into it.
Not sure where is the issue? If compositor define min=0.2 and ref=80, that means they request 400:1 contrast and we should respect that, no? Think about it this way. Let's forget about hardcoded value in libplacebo, assume that libplacebo uses reference_luminance==80. (which in fact it does, because in practice when targeting "SDR" transfer ref_lum==max_lum in libplacebo) Now I have 0-1 float value, which represent 0-10000 nits absolute scale, when What am I missing here? |
I don't believe the developers of those compositors had anything like that in mind when they chose the values. |
|
KDE uses the formula currently used by mpv to scale luminance values. |
Assuming veiling glare is the same the contrast is not that far off: sRGB standard (CRT display with a minimum luminance of 0): (80 + 0) / (0.2 + 0) = 400:1 In practice the current display is likely to be used in an ambient luminance larger than the 16 cd/m2 in the sRGB standard so the effective contrast is even smaller. |
But you cannot assume that... QD-OLEDs are famously bad with ambient light rejection. Also I would argue that if you are watching movies on 400:1, you might prefer black clipping instead of significantly raising whole image luminance, even on such small dynamic range display. |
Are you talking about this? https://invent.kde.org/plasma/kwin/-/blob/b6103beb2ac1561c315e5a272447e213d7f5fbd9/src/core/colorspace.cpp#L582-619 This is mapping the luminances values, based on input / output colorspace. It's not meant to convert min/ref/max values. BPC logic and all tonemapping curve is already implemented. The more I look at this we should just rescale to 203 nits reference, to have SDR values anchored around that. I'm happy to be corrected, but at this point in time, I don't see justification of scaling min_luma to 0 always (which basically disables BPC in libplacebo. |
|
I don't know what the code is. But I've experimentally confirmed that KDE scales luminance values this way. |
What was the test to determine this? |
|
https://github.com/mahkoh/wayland-color-test go to view color description, change type to parametric, enable luminances, change the values. The output remains the same. Therefore the algorithm used by KDE is the same as the algorithm used by the application. (The application currently allows you to set non-integer values for max and ref luminances, this is a bug since the protocol doesn't allow that. If you set those fields to a non-integer value the output will be different.) |
|
I still don't understand why 0.203 -> 10^-6 changes the tone mapping so much. 0.203/203*255 ~= 0.25 and even if we use a pure gamma22 curve that is only ~0.5 in electrical space. I would expect this to change the colors by maybe 1 or 2 in electrical space. But in the sample image, the top left rectangle changes by 9 (!) in electrical space. |
|
Ignore the previous comment. You have to calculate (0.203/203)^(1/2.2) * 255 which is in fact ~11. |
This is hardly saying anything to me. It could just do nothing and the output will also not change. I've created #17339 to fix current scaling. Let me know, if you see any issues with this, preferably with examples and steps to reproduce the wrong image reproduction. |
Yes, this is the effect of BPC, which you can see on the graphs in #17329 (comment) |
|
Just a though, might be wrong: |
This is exactly why this PR exists. But to be quite honest, we should never output bt.1886, except when it's passthrough. It makes no sense to use it as encoding transfer. You can use it when converting to absolute luminance, but that's all that it's useful in our pipeline. |
|
I meant within mpv, inside libplacebo's shader ie: |
You can do that by specifying Also it doesn't make any sense, what are you trying to achieve? |
What's weird to me here is that we're just passing-through the data (for SDR/SDR). The wayland values don't matter, neither does It was just an exploration
That's the way. |
For the same transfer function. And yes, input and output contrast is assumed to be the same, simply because SDR content is display referred, so it applies naturally to input. |
|
That being said, for SDR content on SDR screen, |
It's fair to assume, and this works like this in practice, we confirmed this with compositors. However this is still basing on implementation details. I've already complained about this. But Wayland makes it really not convenient to try to adapt content to target output. It really, really wants to do all the processing itself. Certainly it's impossible when using Vulkan, where you are forced to use protocol directly. And even than, there there is no negotiation, I can't say, that I want PQ output, give me parameters that will not cause compositor to tonemap. All we get is preferred descriptor, which is in most cases sRGB 80/0.2 blend space of compositors. And we can try to infer parameters, based on implementation details of compositors to what we should tonemap to. It's not possible to apply ICC profile, in client, because you have no idea what will compositor do to your image after. And so on. I purposefully, refuse to implement passthrough in vulkan, even though it's currently like 10 lines of code to add this. |
There is no need to infer this. This information is provided in the preferred description via the target_* events. |
Ah, yes the gamma2.2 HDR, which forces us to use 16-bit backbuffer, because gpu bandwidth is free, no? Otherwise it would cause banding, PQ is optimized for lower bits, gamma2.2 was never meant to encode 10000 nits dynamic range, too many bits lost in dark area. Granted of course most modern usecases are in 400-800 nits range, so it kind of dodging the bullet if we limit the the range so much. But with anything else 10-bit unorm is no-go. |
The transfer function has nothing to do with tone mapping. |
I haven't said it does. It however does imply the encoding and when you have 0-1024 values, you have to be smart how to allocate them to your dynamic range. |
|
Will fix this in a different way. Also bt.1886 shouldn't be used as encoding for output in general, so it has little relevance. |





Mesa's Wayland Vulakn WSI doesn't set luminances for SDR transfer, as they are only set for HDR. Which makes sense. In this case however, we have to use default BT.1886 luminance values as defined by Wayland protocol.
I don't like it at all... but will do the job for now.