vo_gpu_next: implement target_csp for wayland and automatic color space switching #16321
Conversation
This implements target_csp in terms of the preferred color description of the video surface.
| : (struct pl_color_space){ .transfer = PL_COLOR_TRC_PQ }; | ||
| ? sw->fns->target_csp(sw, &target_csp_is_preferred) | ||
| : DEFAULT_TARGET_CSP; | ||
| if (!pl_color_transfer_is_hdr(target_csp.transfer)) { |
There was a problem hiding this comment.
Not sure if this should also be guarded by !target_csp_is_preferred. My concern here is that these values are not accurate for SDR displays.
| struct vo_wayland_preferred_description_info *wd = data; | ||
| if (!wd->is_parametric) | ||
| return; | ||
| wd->csp.primaries = map_primaries(primaries); |
There was a problem hiding this comment.
KDE does not send named primaries so this is ineffective there. Maybe pl_color_space could be enhanced to also support primaries by value.
There was a problem hiding this comment.
Seems GNOME also only supports a very limited number of named primaries: https://gitlab.gnome.org/GNOME/mutter/-/blob/main/src/wayland/meta-wayland-color-management.c#L244
I guess support for value-based primaries is a must.
There was a problem hiding this comment.
Btw I think there should be very less cases that we can get monitor's primaries in named format, since monitor's EDID would only provide chromaticity coordinates unless the monitor is pure sRGB.
| bool target_csp_is_preferred = false; | ||
| struct pl_color_space target_csp; | ||
| // Assume HDR is supported, if query is not available | ||
| // TODO: Implement this for all backends |
There was a problem hiding this comment.
This basically resolves this TODO
|
Fixes: #16305 |
This patch allows vo_gpu_next to automatically switch to the preferred primaries and transfer function of the current output without the user having to specify them on the command line. Without this patch, libplacebo will always pick an HDR color space that will require the compositor to perform tone and gamut mapping on SDR outputs. This will likely be of lower quality than what libplacebo can achieve when it instead picks an SDR color space.
| enum pl_color_primaries primaries = opts->target_prim ? opts->target_prim : target_csp.primaries; | ||
| if (primaries) | ||
| hint.primaries = primaries; | ||
| enum pl_color_transfer transfer = opts->target_trc ? opts->target_trc : target_csp.transfer; | ||
| if (transfer) | ||
| hint.transfer = transfer; |
There was a problem hiding this comment.
Would it also make sense to copy the hdr metadata from the target_csp?
|
Download the artifacts for this pull request: Windows |
| wp_image_description_info_v1_destroy(image_description_info); | ||
| if (!wl->icc_file) | ||
| MP_VERBOSE(wl, "No ICC profile retrieved from the compositor.\n"); | ||
| if (wd->is_parametric) { |
There was a problem hiding this comment.
Maybe we should prefer ICC file first? Do we need the extra information provided by ICC file?
|
So sorry for leaving it not merged for so long. I have taken a look at the topic and if you don't mind I follow up on this in #16512. With the new option, some more docs and support for Vulkan on Windows. I've cherry-picked your Wayland changes. Renamed commit message and skipped gpu-next changes, because I did it differently with the new option. |
|
Sorry on my end for not ever getting around to looking at it as well... |
|
Merged as 39c9d1a and other vo_gpu_next changes. Should be fine now, we target prefered color space by default, if possible, i.e. if there is compatible surface format. Can be controlled with Thank you for contribution. Hopefully now most of those things work out of the box. |
4 participants
This patch allows vo_gpu_next to automatically switch to the preferred
primaries and transfer function of the current output without the user
having to specify them on the command line.
Without this patch, libplacebo will always pick an HDR color space that
will require the compositor to perform tone and gamut mapping on SDR
outputs. This will likely be of lower quality than what libplacebo can
achieve when it instead picks an SDR color space.
I've tested this in Jay and KDE with https://4kmedia.org/lg-new-york-hdr-uhd-4k-demo. Jay currently does not do any tone mapping but even the tone mapping on KDE is not as good as libplacebo. At the start of the video, you can see the clouds getting blown out without this patch. With this patch, the clouds look good.
When moving the window between outputs with different color spaces, you should see logs similar to