From 5234e305784c7e2ce3f512d4f436355aaf20a9d8 Mon Sep 17 00:00:00 2001 From: David Turner Date: Thu, 8 May 2025 09:53:24 +0100 Subject: [PATCH 1/4] backend/drm/atomic: Always use BT.709 encoding for YUV When we overlay/scanout non-RGB planes we were relying on the default DRM color encoding, which could vary per DRM device. We want this to be consistent across devices and with YUV conversion done by renderers, so change this to always use BT.709 encoding (if the property is available). I've chosen BT.709 because it should be correct for HD video, which is probably most common. --- backend/drm/atomic.c | 9 +++++++++ backend/drm/properties.c | 1 + include/backend/drm/properties.h | 12 ++++++++++++ 3 files changed, 22 insertions(+) diff --git a/backend/drm/atomic.c b/backend/drm/atomic.c index 037c3c23a..63589cfa2 100644 --- a/backend/drm/atomic.c +++ b/backend/drm/atomic.c @@ -378,6 +378,15 @@ static void set_plane_props(struct atomic *atom, struct wlr_drm_backend *drm, atomic_add(atom, id, props->crtc_y, dst_box->y); atomic_add(atom, id, props->crtc_w, dst_box->width); atomic_add(atom, id, props->crtc_h, dst_box->height); + + // Always set the color encoding on every plane to BT.709. This only + // affects YUV planes (eg. scanout video). This won't be correct for + // all video but at least means we will be consistent and most + // importantly should match the GLES2 renderer, so avoids visible + // color-shifts when direct scanout is enabled/disabled. + if (props->color_encoding) { + atomic_add(atom, id, props->color_encoding, DRM_COLOR_YCBCR_BT709); + } } static bool supports_cursor_hotspots(const struct wlr_drm_plane *plane) { diff --git a/backend/drm/properties.c b/backend/drm/properties.c index 0104304e5..91ce8c4dd 100644 --- a/backend/drm/properties.c +++ b/backend/drm/properties.c @@ -48,6 +48,7 @@ static const struct prop_info crtc_info[] = { static const struct prop_info plane_info[] = { #define INDEX(name) (offsetof(struct wlr_drm_plane_props, name) / sizeof(uint32_t)) + { "COLOR_ENCODING", INDEX(color_encoding) }, { "CRTC_H", INDEX(crtc_h) }, { "CRTC_ID", INDEX(crtc_id) }, { "CRTC_W", INDEX(crtc_w) }, diff --git a/include/backend/drm/properties.h b/include/backend/drm/properties.h index 421eb4275..1765a4951 100644 --- a/include/backend/drm/properties.h +++ b/include/backend/drm/properties.h @@ -63,6 +63,18 @@ struct wlr_drm_plane_props { uint32_t hotspot_x; uint32_t hotspot_y; uint32_t in_fence_fd; + + uint32_t color_encoding; // Not guaranteed to exist +}; + +/* + * The encoding and range enums are defined in the kernel but not + * exposed in public headers. + */ +enum drm_color_encoding { + DRM_COLOR_YCBCR_BT601, + DRM_COLOR_YCBCR_BT709, + DRM_COLOR_YCBCR_BT2020, }; bool get_drm_connector_props(int fd, uint32_t id, From ddb4193009c3d9d92721e7e5c6860ff12cdc2b7b Mon Sep 17 00:00:00 2001 From: David Turner Date: Thu, 8 May 2025 10:01:54 +0100 Subject: [PATCH 2/4] backend/drm/libliftoff: Always use BT.709 encoding for YUV In the drm/libliftoff backend, always set DRM planes to use BT.709 color encoding so we can be consistent across backends and with renderers. This only affects YUV formats. --- backend/drm/libliftoff.c | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/backend/drm/libliftoff.c b/backend/drm/libliftoff.c index 12761afd4..fcc9e135d 100644 --- a/backend/drm/libliftoff.c +++ b/backend/drm/libliftoff.c @@ -181,7 +181,8 @@ static uint64_t to_fp16(double v) { static bool set_layer_props(struct wlr_drm_backend *drm, const struct wlr_output_layer_state *state, uint64_t zpos, - struct wl_array *fb_damage_clips_arr) { + struct wl_array *fb_damage_clips_arr, + bool has_color_encoding) { struct wlr_drm_layer *layer = get_drm_layer(drm, state->layer); uint32_t width = 0, height = 0; @@ -232,7 +233,7 @@ static bool set_layer_props(struct wlr_drm_backend *drm, *ptr = fb_damage_clips; } - return + ret = liftoff_layer_set_property(layer->liftoff, "zpos", zpos) == 0 && liftoff_layer_set_property(layer->liftoff, "CRTC_X", crtc_x) == 0 && liftoff_layer_set_property(layer->liftoff, "CRTC_Y", crtc_y) == 0 && @@ -243,6 +244,13 @@ static bool set_layer_props(struct wlr_drm_backend *drm, liftoff_layer_set_property(layer->liftoff, "SRC_W", src_w) == 0 && liftoff_layer_set_property(layer->liftoff, "SRC_H", src_h) == 0 && liftoff_layer_set_property(layer->liftoff, "FB_DAMAGE_CLIPS", fb_damage_clips) == 0; + + if (has_color_encoding) { + ret &= liftoff_layer_set_property(layer->liftoff, + "COLOR_ENCODING", DRM_COLOR_YCBCR_BT709) == 0; + } + + return ret; } static bool devid_from_fd(int fd, dev_t *devid) { @@ -332,6 +340,14 @@ static bool add_connector(drmModeAtomicReq *req, ok = ok && add_prop(req, crtc->id, crtc->props.vrr_enabled, state->vrr_enabled); } + // We want to always set the color encoding for YUV planes to + // BT.709 to be consistent with renderers, but we don't know + // if the chosen plane will have the COLOR_ENCODING property. + // However, empirically, if the primary plane has + // COLOR_ENCODING then all other YUV-supporting planes also do. + bool has_color_encoding = + crtc->primary->props.color_encoding != 0; + ok = ok && set_plane_props(crtc->primary, crtc->primary->liftoff_layer, state->primary_fb, 0, &state->primary_viewport.dst_box, @@ -345,6 +361,12 @@ static bool add_connector(drmModeAtomicReq *req, "FB_DAMAGE_CLIPS", state->fb_damage_clips); liftoff_layer_set_property(crtc->liftoff_composition_layer, "FB_DAMAGE_CLIPS", state->fb_damage_clips); + if (has_color_encoding) { + liftoff_layer_set_property(crtc->primary->liftoff_layer, + "COLOR_ENCODING", DRM_COLOR_YCBCR_BT709); + liftoff_layer_set_property(crtc->liftoff_composition_layer, + "COLOR_ENCODING", DRM_COLOR_YCBCR_BT709); + } if (state->primary_in_fence_fd >= 0) { liftoff_layer_set_property(crtc->primary->liftoff_layer, @@ -361,7 +383,7 @@ static bool add_connector(drmModeAtomicReq *req, for (size_t i = 0; i < state->base->layers_len; i++) { const struct wlr_output_layer_state *layer_state = &state->base->layers[i]; ok = ok && set_layer_props(drm, layer_state, i + 1, - fb_damage_clips_arr); + fb_damage_clips_arr, has_color_encoding); } } From ebf24c2bda7fdb55ded6b4c12265a2ed6ba67ef0 Mon Sep 17 00:00:00 2001 From: David Turner Date: Thu, 8 May 2025 10:49:02 +0100 Subject: [PATCH 3/4] render/egl: Always use BT.709 hint for YUV Always set the BT.709 hint in an attempt to get the gles2 renderer to consistently use this color encoding when converting YUV to RGB. In addition to being consistent across drivers, this is consistent with DRM so should avoid color shifts when enabling/disabling direct scanout. --- render/egl.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/render/egl.c b/render/egl.c index 07d3a1425..f35481393 100644 --- a/render/egl.c +++ b/render/egl.c @@ -745,7 +745,7 @@ EGLImageKHR wlr_egl_create_image_from_dmabuf(struct wlr_egl *egl, } unsigned int atti = 0; - EGLint attribs[50]; + EGLint attribs[52]; attribs[atti++] = EGL_WIDTH; attribs[atti++] = attributes->width; attribs[atti++] = EGL_HEIGHT; @@ -803,6 +803,13 @@ EGLImageKHR wlr_egl_create_image_from_dmabuf(struct wlr_egl *egl, } } + // Always convert YUV to RGB using BT.709 color encoding. This is to + // attempt to be consistent across GL drivers, and also to be + // consistent with DRM scanout so there's no color-shift when direct + // scanout is enabled/disabled. + attribs[atti++] = EGL_YUV_COLOR_SPACE_HINT_EXT; + attribs[atti++] = EGL_ITU_REC709_EXT; + // Our clients don't expect our usage to trash the buffer contents attribs[atti++] = EGL_IMAGE_PRESERVED_KHR; attribs[atti++] = EGL_TRUE; From 37f48a200ecce78e0c28de99ab0939b87a2ef9f1 Mon Sep 17 00:00:00 2001 From: David Turner Date: Thu, 8 May 2025 11:16:19 +0100 Subject: [PATCH 4/4] render/vulkan: Use BT.709 for YUV conversion Change from BT.601 to BT.709 color encoding for YUV conversion in the vulkan renderer. This is to be consistent with the gles2 renderer as well as the DRM scanout path. The latter is important to avoid visible color shifts when direct scanout is enabled/disabled. --- render/vulkan/renderer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/render/vulkan/renderer.c b/render/vulkan/renderer.c index b2feca013..3cca5d6f5 100644 --- a/render/vulkan/renderer.c +++ b/render/vulkan/renderer.c @@ -1956,7 +1956,7 @@ struct wlr_vk_pipeline_layout *get_or_create_pipeline_layout( VkSamplerYcbcrConversionCreateInfo conversion_create_info = { .sType = VK_STRUCTURE_TYPE_SAMPLER_YCBCR_CONVERSION_CREATE_INFO, .format = key->ycbcr_format->vk, - .ycbcrModel = VK_SAMPLER_YCBCR_MODEL_CONVERSION_YCBCR_601, + .ycbcrModel = VK_SAMPLER_YCBCR_MODEL_CONVERSION_YCBCR_709, .ycbcrRange = VK_SAMPLER_YCBCR_RANGE_ITU_NARROW, .xChromaOffset = VK_CHROMA_LOCATION_MIDPOINT, .yChromaOffset = VK_CHROMA_LOCATION_MIDPOINT,