wayland: add support for wp-color-representation-v1

[Dudemanguy]

Alternative to #14917. The downside is that this protocol is ungodly complicated. Upside is that more people seem to implement and in theory when the color management protocol lands upstream next decade it will be like this. So probably longer term this is better.

Weston atm doesn't support the minimum features we need so it's not good for testing other than making sure I didn't make a protocol error. kwin is probably the best but xx-color-management-v4 is only supported in kwin master at the moment and not at release (release supports v2). So not really tested yet until I go compile a bunch of kde junk later. Mutter 47 seems to support xx-color-management-v4 though.

TODO:

• Add ICC. Weston advertises ICC support at least.
• Check the multipliers. I think everything is supposed to be multiplied by 10000 (should be correct now).

[github-actions]

Download the artifacts for this pull request:

Windows

• mpv-i686-w64-mingw32
• mpv-x86_64-w64-mingw32
• mpv-x86_64-windows-msvc

macOS

• mpv-macos-13-intel
• mpv-macos-14-arm
• mpv-macos-15-arm

[Dudemanguy]

So kwin master doesn't support setting the transfer function which means this will do absolutely nothing (yay) because xx_image_description_v4 requires at least setting primaries and transfer function. Oh well, I guess on the backburner for now until we have something more testable.

Edit: Scratch that. I think this is just a small kwin bug with regards to the feature support it advertises. I think this can work there. Will double check later.

[Dudemanguy]

So it turns out what was happening here is that kwin master supports set_tf_named but not set_tf_power which is why it reported features the way it did. We eventually would need set_tf_power, but at least now it should work for some things. I confirmed that it did set metadata. I dunno if it actually works given my crap hardware but in theory.

[Dudemanguy]

I also added wp-color-representation-v1 to this. It's a lot easier in comparison. Tested briefly against this weston MR.

[ruihe774]

FWIW can we generate an ICC profile based on primary, transfer, etc on the fly and pass it to the compositor that does not support them?

[Dudemanguy]

ICC profile generation isn't in scope or supported by this protocol if that's what you're asking. It supports sending an icc profile to the compositor.

[ruihe774]

ICC profile generation isn't in scope or supported by this protocol if that's what you're asking. It supports sending an icc profile to the compositor.

I mean generating an ICC profile from our side and sending the generated ICC profile to the compositor.

[Dudemanguy]

Not really sure why mpv should generate ICC profiles. Wouldn't that be slow? Out of scope for what I want to do here though in any case.

[ruihe774]

Not really sure why mpv should generate ICC profiles.

I would say it is in order to be compatible with more compositors. Of course, there is the opposite opinion that it is the duty of the compositor to be compatible with more applications, not the duty of the application.

Wouldn't that be slow?

AFAIK we just need to fill up some fields according to the ICC spec. We are not generating complex ICC like the kind that contains a LUT.

Out of scope for what I want to do here though in any case.

Agree.

[llyyr]

AFAIK we just need to fill up some fields according to the ICC spec. We are not generating complex ICC like the kind that contains a LUT.

HDR metadata can change from frame to frame, so we'd be generating a new ICC profile for each frame and the compositor would also need to apply it for each frame. This just doesn't work.

[kasper93]

Do we plan to merge it or wait for protocol to become stable first? I think we can iterate on master with it.

[Dudemanguy]

I have no problem with merging before it becomes stable. But someone needs to verify on some compositor that this actually works and has a desirable outcome. I lack the hardware to really test anything meaningfully.

[kasper93]

I have no problem with merging before it becomes stable. But someone needs to verify on some compositor that this actually works and has a desirable outcome. I lack the hardware to really test anything meaningfully.

I think it is mostly on compositor side anyway, as long as mpv sends the metadata there is not much else we can do.

[Dudemanguy]

I think it is mostly on compositor side anyway, as long as mpv sends the metadata there is not much else we can do.

That's true but e.g. I have no idea how this works with whatever kwin currently does with vulkan for hdr. Also now that I think about it, color representation isn't in any compositor yet and I dunno if people are going to keep the wp namespace for that one or make it xx like the color management one is.

[denkdran]

I could test this, however when I try to build I currently get this (gcc 14.2.1, linux)

../video/out/vo_dmabuf_wayland.c: In function 'reconfig':
../video/out/vo_dmabuf_wayland.c:708:43: error: 'struct priv' has no member named 'color'
  708 |     const struct pl_hdr_metadata *hdr = &p->color.hdr;
      |                                           ^~
../video/out/vo_dmabuf_wayland.c:709:63: error: assignment of read-only location 'hdr->scene_max[2]'
  709 |     hdr->scene_max[0] = hdr->scene_max[1] = hdr->scene_max[2] = 0;
      |                                                               ^
../video/out/vo_dmabuf_wayland.c:710:52: error: assignment of member 'avg_pq_y' in read-only object
  710 |     hdr->scene_avg = hdr->max_pq_y = hdr->avg_pq_y = 0;   vo->target_params = &p->target_params;
      |                                                    ^

Not sure if it's me or really a problem with the code... I also see it for some of the PR voters.

[kasper93]

I could test this, however when I try to build I currently get this (gcc 14.2.1, linux)

I think I baited @Dudemanguy to commit broken code, sorry.

Changing this one line to the following should work.

struct pl_hdr_metadata *hdr = &p->target_params->color.hdr;

[Dudemanguy]

Yeah oops. Should be fixed now.

[denkdran]

Thanks! Alright, I gave this a test. Used Mutter 47.0, and then via looking glass I enabled both hdr and color_management:

global.context.get_debug_control().enable_hdr = true
global.context.get_debug_control().color_management_protocol = true

In general, I use this config:

vo=gpu-next
hwdec=vulkan
gpu-api=vulkan
gpu-context=waylandvk

With this change, it's possible to play HDR content with proper colors by simply starting mpv like this:
./mpv a-file.mkv --target-prim=bt.2020 --target-trc=pq

Without this change, the colors will look washed out / wrong.

I compared it to what's possible right now (without this change), i.e. with vk_hdr_layer, like this:
ENABLE_HDR_WSI=1 mpv a-file.mkv --target-prim=bt.2020 --target-trc=pq and, as expected, it looks exactly the same.
So this PR removes the need to install vk_hdr_layer + setting the env var.

This change definitely does something, and I see it as great value already.

Of course, it would be great if mpv could somehow detect and set target-prim + target-trc automatically. Any chance this is possible?

[Dudemanguy]

Thanks. Nice to hear this actually works. I think I will probably split out the xx-color-management-v4 addition in a separate PR since I know that one is currently in use by several compositors. The color representation change can live on in draft form for now. I merged the two things together since they are closely related changes and it was getting annoying to have to rebase all the time while I was working on this.

if mpv could somehow detect and set target-prim + target-trc automatically

The default for both of these is auto actually which I think doesn't actually do anything. Can't remember if there's a reason for that or not.

Edit: Out of curiosity, does it work if you use opengl, vo_gpu, or vo_dmabuf_wayland? The last one might not work anyway because of some things still lacking on the compositor side.

[denkdran]

Edit: Out of curiosity, does it work if you use opengl, vo_gpu, or vo_dmabuf_wayland? The last one might not work anyway because of some things still lacking on the compositor side.

vo_gpu constantly flickers between correct colors and washed out colors (with all the other settings I posted above). If I don't set hwdec, gpu-api and gpu-context it does not flicker anymore, but the colors are just washed out.

What do you mean with opengl? I tried to set gpu-api=opengl together with vo=gpu and vo=gpu-next (and defaults instead of vulkan), both resulted in washed out colors.

vo_dmabuf_wayland has washed out colors as well (in combination with my default settings, i.e. vulkan).

It seems like gpu-next, my default settings but hwdec=vaapi instead of hwdec=vulkan works as well, but the playback glitches for a few seconds at the beginning. Once it has settled it seems to be fine though. Colors are accurate.

If more testing is needed you can let me know specific combinations of settings and I'll try them out.

[Dudemanguy]

So it seems we are still dependent on vulkan for correct colors for now. That's OK. Mutter must strictly only use the vulkan extension under the hood.

vo_gpu constantly flickers between correct colors and washed out colors (with all the other settings I posted above). If I don't set hwdec, gpu-api and gpu-context it does not flicker anymore, but the colors are just washed out.

This is kind of not good though. Can you confirm which setting actually makes the difference here? I assume only --gpu-api=vulkan causes the flickering and --gpu-api=opengl is always washed out.

[kasper93]

Of course, it would be great if mpv could somehow detect and set target-prim + target-trc automatically. Any chance this is possible?

This would be --target-colorspace-hint for gpu-next, normally it will output whatever defaults is, so in fact SDR bt.709. With target colorspace hint it tries to configure the swapchain to match video colorspace and signal that. It already works with vulkan, but now this PR also signals directly to compositor.

What I would focus on first is vo_dmabuf_wayland , because it is most direct output of the video to the compositor. gpu/gpu-next do lots of other stuff, that may/should be configured. vo_dmabuf_wayland should just write the video to dmabuf and let compositor what colorspace it has, from this point it is compositor job to enable things correctly.

EDIT:

@denkdran I could reproduce the same results as yours. I will debug it, because from quick look, seems that something is off with metadata. But I did quick test only, don't have time rn.

[denkdran]

@Dudemanguy You are correct, the flickering only occurs with --gpu-api=vulkan

@kasper93 Yes! Right, with --target-colorspace-hint everything just works if ENABLE_HDR_WSI=1 is used (mpv 0.39.0). With the changes in this PR it does not work, it just sets SDR bt.709.

BTW, if I use ENABLE_HDR_WSI=1 with this PR, I get an error:

xx_color_manager_v4#37: error 1: surface already requested
[vo/gpu-next/libplacebo] vk->GetPhysicalDeviceSurfacePresentModesKHR(vk->physd, p->surf, &num_modes, NULL): VK_ERROR_SURFACE_LOST_KHR (../src/vulkan/swapchain.c:364)
[vo/gpu-next] Failed initializing any suitable GPU context!
Error opening/initializing the selected video_out (--vo) device.

EDIT: While testing dmabuf-wayland I noticed the following log messages (although I specified hwdec=vulkan). The infos in the video seem to indicate that bt.2020 is chosen, but colors look washed out.

[autoconvert] HW-uploading to vaapi
[autoconvert] Converting yuv420p10 -> p010
[hwupload] upload p010 -> vaapi[p010]

target-colorspace-hint does not seem to make a difference with dmabuf-wayland.
Not sure if it is relevant, but I noticed that in the display infos it says levels=limited. When I use gpu-next it says levels=full.

[Dudemanguy]

BTW, if I use ENABLE_HDR_WSI=1 with this PR, I get an error:

Yeah that would be because both mpv and that layer are attempting to to use the protocol on the same surface which is a hard error. I guess a workaround would be to not use the protocol on our side if that environment variable is set.

[Dudemanguy]

This has been reduced to just wp-color-representation now. Not sure if any compositor will actually implement this one, but I'll leave the branch here so anybody can easily test against mpv.