Skip to content

wayland: luminance improvements #17070

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+96 −19
Open

wayland: luminance improvements #17070

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5151955
wayland: convert luminance values to libplacebo space
mahkoh Nov 22, 2025
c5066b3
wayland: switch to PQ when necessary
mahkoh Nov 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion video/out/d3d11/context.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,8 @@ static int d3d11_color_depth(struct ra_swapchain *sw)
return MPMIN(ra_fmt->component_depth[0], desc1.BitsPerColor);
}

static struct pl_color_space d3d11_target_color_space(struct ra_swapchain *sw)
static struct pl_color_space d3d11_target_color_space(struct ra_swapchain *sw,
float source_max_luma)
{
if (sw->ctx->opts.composition)
return (struct pl_color_space){0};
Expand Down
4 changes: 2 additions & 2 deletions video/out/gpu/context.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ struct ra_ctx_params {
int (*color_depth)(struct ra_ctx *ctx);

// Preferred device color space. Optional.
pl_color_space_t (*preferred_csp)(struct ra_ctx *ctx);
pl_color_space_t (*preferred_csp)(struct ra_ctx *ctx, float source_max_luma);

// See ra_swapchain_fns.get_vsync.
void (*get_vsync)(struct ra_ctx *ctx, struct vo_vsync_info *info);
Expand Down Expand Up @@ -114,7 +114,7 @@ struct ra_swapchain_fns {
int (*color_depth)(struct ra_swapchain *sw);

// Target device color space. Optional.
pl_color_space_t (*target_csp)(struct ra_swapchain *sw);
pl_color_space_t (*target_csp)(struct ra_swapchain *sw, float source_max_luma);

// Called when rendering starts. Returns NULL on failure. This must be
// followed by submit_frame, to submit the rendered frame. This function
Expand Down
4 changes: 2 additions & 2 deletions video/out/opengl/context.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,11 +305,11 @@ static void ra_gl_ctx_get_vsync(struct ra_swapchain *sw,
p->params.get_vsync(sw->ctx, info);
}

static pl_color_space_t ra_gl_ctx_target_csp(struct ra_swapchain *sw)
static pl_color_space_t ra_gl_ctx_target_csp(struct ra_swapchain *sw, float source_max_luma)
{
struct priv *p = sw->priv;
if (p->params.preferred_csp)
return p->params.preferred_csp(sw->ctx);
return p->params.preferred_csp(sw->ctx, source_max_luma);
return (pl_color_space_t){0};
}

Expand Down
12 changes: 10 additions & 2 deletions video/out/vo_gpu_next.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1089,8 +1089,16 @@ static bool draw_frame(struct vo *vo, struct vo_frame *frame)
bool pass_colorspace = false;
struct pl_color_space target_csp = {0};
// TODO: Implement this for all backends
if (sw->fns->target_csp)
target_csp = sw->fns->target_csp(sw);
if (sw->fns->target_csp) {
float source_max_luma = 0.0f;
if (frame->current) {
source_max_luma = frame->current->params.color.hdr.max_luma;
Copy link
Member

@kasper93 kasper93 Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This value is base HDR10 value. And won't be in fact used by libplacebo if HDR10+/DV metadata is also available.

Copy link
Contributor Author

@mahkoh mahkoh Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do I get the correct value here?

Copy link
Member

@kasper93 kasper93 Nov 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something like this, but I'm still reluctant of using those values to infer any target parameters. Depending on the source this may or may not be valid/useful value. It may be transfer nominal value, which we have to avoid for example.

            pl_color_space_nominal_luma_ex(pl_nominal_luma_params(
                .color      = &frame->current->params.color,
                .metadata   = PL_HDR_METADATA_ANY,
                .scaling    = PL_HDR_NITS,
                .out_max    = &source_max_luma,
            ));

Copy link
Contributor Author

@mahkoh mahkoh Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At some point libplacebo has to choose some value to determine if and how it needs to do tone mapping. That is the value I'm interested in. It is also ok if this value is too large since in that case the worst that should happen is that we de-optimize by using PQ instead of the preferred SDR transfer function.

Copy link
Contributor Author

@mahkoh mahkoh Nov 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the code below again, we don't actually need the max_luma of the source. We only need to know if libplacebo is going to do tone mapping when targeting an SDR display, because that's the case in which it will need accurate metadata from the target.

            // Otherwise, if the max_luma of the source is brighter than paper white, then
            // libplacebo will always perform tone mapping. To do this correctly, it will
            // need to know the exact max_luma of the display.
            if (source_max_luma > PL_COLOR_SDR_WHITE)
                csp.transfer = PL_COLOR_TRC_PQ;

Copy link
Contributor Author

@mahkoh mahkoh Nov 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the first commit it would have to be

        if (wd->csp.hdr.max_luma != PL_COLOR_SDR_WHITE) {

See below for why not just >.

Also note that if requested luminances is lower than PL_COLOR_SDR_WHITE, we can also switch to PQ, but we can just ignore this. This is what we actually do, because we don't want to map SDR lower than nominal ref.

If the reference luminance is higher than the max target luminance, then the first commit will cause wd->csp.hdr.max_luma < PL_COLOR_SDR_WHITE. In this case I don't understand why you don't want to tone-map to max_luma. This can obviously only happen in weird situations like when the user sets his brightness higher than 100% but it is easy to handle. Does libplacebo not handle tone mapping to below PL_COLOR_SDR_WHITE correctly?

Either way, if ref_lum==max_lum we should be outputting "SDR" regardless of the actual luminances.

The first commit ensures that in that case wd->csp.hdr.max_luma == PL_COLOR_SDR_WHITE.

Copy link
Contributor Author

@mahkoh mahkoh Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason that I don't want to use this coarse hammer is that it means that mpv will always ignore the preferred transfer function when the user is using an HDR display even if mpv is playing an SDR video.

Copy link
Contributor Author

@mahkoh mahkoh Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, if that is a piece of contention, then I can drop the second commit in favor of what you've suggested. The user-facing improvement is in the first commit.

Copy link
Member

@kasper93 kasper93 Nov 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason that I don't want to use this coarse hammer is that it means that mpv will always ignore the preferred transfer function when the user is using an HDR display even if mpv is playing an SDR video.

This is actually expected behavior. I prefer PQ output on HDR targets, even for SDR video. You can change it by using target-mode=source.

This is how =target mode works on Windows. Where target_csp is on the wire format, hence whenever HDR mode is enabled, we output PQ. And the conversion to PQ is done inside mpv.

I know Wayland is much smarter than that, and uses preferred transfer, which most likely is not related to the actual display, but if preferred transfer function is not compatible with mpv definition of HDR, I would prefer to keep it simple and avoid doing "complex" heuristics and fallback to PQ.

Feel free to correct my thinking, I don't have strong opinion how this should work. I just feel like the target_csp should not depend on source. Well the hint can depend on the source, so maybe I'm wrong here.

My thinking is, if Wayland preferred function is "HDR-like" gamma2.2, then we should respect the HDR of it. I understand that in some cases, the definition may be compatible with mpv's SDR output and we can use gamma2.2 in this case, but this is slightly messy with all the heuristics. But I still don't see why it should depend on source itself.

Copy link
Contributor Author

@mahkoh mahkoh Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're not wrong. The problem is pl_color_transfer_is_hdr and the way it is used. As others have already pointed out, there are no HDR transfer functions as far as wayland is concerned. Ideally, all of the uses of that function should be removed from both mpv and libplacebo and the transfer function of the target hint should only be taken into account as late as possible: Once libplacebo knows everything there is to know about the image it is going to render, it decides if the preferred TF is suitable for that and otherwise switches to a different one.

Getting there would be a much larger change and my thinking was that going the other way and passing the source into the windowing backend would get us roughly the same result with much less effort.

if (opts->tone_map.inverse)
// Sentinel value to use the maximum luminance supported by the display.
source_max_luma = INFINITY;
}
target_csp = sw->fns->target_csp(sw, source_max_luma);
}
if (target_csp.primaries == PL_COLOR_PRIM_UNKNOWN)
target_csp.primaries = get_best_prim_container(&target_csp.hdr.prim);
if (!pl_color_transfer_is_hdr(target_csp.transfer)) {
Expand Down
4 changes: 2 additions & 2 deletions video/out/vulkan/context.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,11 +518,11 @@ static void get_vsync(struct ra_swapchain *sw,
p->params.get_vsync(sw->ctx, info);
}

static pl_color_space_t target_csp(struct ra_swapchain *sw)
static pl_color_space_t target_csp(struct ra_swapchain *sw, float soucre_max_luma)
{
struct priv *p = sw->priv;
if (p->params.preferred_csp)
return p->params.preferred_csp(sw->ctx);
return p->params.preferred_csp(sw->ctx, soucre_max_luma);
return (pl_color_space_t){0};
}

Expand Down
4 changes: 2 additions & 2 deletions video/out/vulkan/context_wayland.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ static bool wayland_vk_check_visible(struct ra_ctx *ctx)
return vo_wayland_check_visible(ctx->vo);
}

static pl_color_space_t wayland_vk_preferred_csp(struct ra_ctx *ctx)
static pl_color_space_t wayland_vk_preferred_csp(struct ra_ctx *ctx, float source_max_luma)
{
return vo_wayland_preferred_csp(ctx->vo);
return vo_wayland_preferred_csp(ctx->vo, source_max_luma);
}

static void wayland_vk_swap_buffers(struct ra_ctx *ctx)
Expand Down
2 changes: 1 addition & 1 deletion video/out/vulkan/context_win.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ static int color_depth(struct ra_ctx *ctx)
return -1;
}

static struct pl_color_space preferred_csp(struct ra_ctx *ctx)
static struct pl_color_space preferred_csp(struct ra_ctx *ctx, float source_max_luma)
{
struct priv *p = ctx->priv;

Expand Down
80 changes: 74 additions & 6 deletions video/out/wayland_common.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2146,13 +2146,32 @@ static void info_done(void *data, struct wp_image_description_info_v1 *image_des
struct vo_wayland_state *wl = wd->wl;
wp_image_description_info_v1_destroy(image_description_info);
if (!wd->icc_file) {
wl->preferred_csp = wd->csp;
MP_VERBOSE(wl, "Preferred surface feedback received:\n");
log_color_space(wl->log, wd);
if (wd->csp.hdr.max_luma > wd->ref_luma) {
MP_VERBOSE(wl, "Setting preferred transfer to PQ for HDR output.\n");
wl->preferred_csp.transfer = PL_COLOR_TRC_PQ;
// Wayland luminances are always in reference to the reference luminance. That is,
// if max_luma == 2*ref_luma, then there is 2x headroom above paper white. On the
// other hand, libplacebo hardcodes PL_COLOR_SDR_WHITE as the reference luminance.
// We must scale all wayland values to correspond to the libplacebo scale,
// otherwise libplacebo will assume that there is too little or too much headroom
// when ref_luma != PL_COLOR_SDR_WHITE.
float a = wd->min_luma;
float b = (PL_COLOR_SDR_WHITE - a) / (wd->ref_luma - a);
wd->csp.hdr.min_luma = (wd->csp.hdr.min_luma - a) * b + a;
Copy link
Contributor Author

@mahkoh mahkoh Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This value can realistically become negative and not all components might handle this. So this should probably be wrapped in MPMAX.

Copy link
Contributor Author

@mahkoh mahkoh Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PS: a and b are chosen such that paper white in the compositor space maps to paper white in libplacebo and paper black (SDR black, all bits 0) maps to itself. This is the inverse of how I've implemented this mapping in my compositor and KDE seems to do something similar.

wd->csp.hdr.max_luma = (wd->csp.hdr.max_luma - a) * b + a;
if (wd->csp.hdr.max_cll != 0)
wd->csp.hdr.max_cll = (wd->csp.hdr.max_cll - a) * b + a;
if (wd->csp.hdr.max_fall != 0)
wd->csp.hdr.max_fall = (wd->csp.hdr.max_fall - a) * b + a;
// Since we want to do some exact comparisons of max_luma with PL_COLOR_SDR_WHITE,
// we need to round it.
if (fabsf(wd->csp.hdr.max_luma - PL_COLOR_SDR_WHITE) < 1e-2f) {
wd->csp.hdr.max_luma = PL_COLOR_SDR_WHITE;
if (wd->csp.hdr.max_cll != 0)
wd->csp.hdr.max_cll = MPMIN(wd->csp.hdr.max_cll, wd->csp.hdr.max_luma);
if (wd->csp.hdr.max_fall != 0)
wd->csp.hdr.max_fall = MPMIN(wd->csp.hdr.max_fall, wd->csp.hdr.max_luma);
}
wl->preferred_csp = wd->csp;
} else {
if (wl->icc_size) {
munmap(wl->icc_file, wl->icc_size);
Expand Down Expand Up @@ -3959,10 +3978,59 @@ bool vo_wayland_check_visible(struct vo *vo)
return render;
}

struct pl_color_space vo_wayland_preferred_csp(struct vo *vo)
struct pl_color_space vo_wayland_preferred_csp(struct vo *vo, float source_max_luma)
{
struct vo_wayland_state *wl = vo->wl;
return wl->preferred_csp;
struct pl_color_space csp = wl->preferred_csp;
if (!pl_color_transfer_is_hdr(csp.transfer)) {
// For transfer functions for which pl_color_transfer_is_hdr returns false, mpv
// discards the HDR metadata and assumes a max_luma of PL_COLOR_SDR_WHITE.
// Similarly, mesa discards the HDR metadata for such transfer functions.
// Therefore, if there is any reason for us to preserve the HDR metadata, we need
// to switch to a transfer function for which pl_color_transfer_is_hdr returns
// true. PL_COLOR_TRC_PQ is the most widely supported transfer function of that
// kind. If VK_COLOR_SPACE_HDR10_ST2084_EXT is not available, then libplacebo will
// still try to fall back to another HDR transfer function so that mesa passes the
// metadata on to the compositor. (Therefore, using any HDR transfer function here
// would do since we cannot actually know which transfer function the compositor
// prefers.)

// The default assumption by all components in the pipeline is that the maximum
// luminance for non-HDR transfer functions is paper white. Therefore, we don't
// have to handle this case.
if (csp.hdr.max_luma != PL_COLOR_SDR_WHITE) {
// Otherwise, if we are doing inverse tone mapping, which is indicated by
// source_max_luma == INFINITY, then libplacebo always needs to know the exact
// max_luma of the display since that is the target we want to hit.
if (source_max_luma == INFINITY)
csp.transfer = PL_COLOR_TRC_PQ;
// Otherwise, if the max_luma of the source is brighter than paper white, then
// libplacebo will always perform tone mapping. To do this correctly, it will
// need to know the exact max_luma of the display.
if (source_max_luma > PL_COLOR_SDR_WHITE)
csp.transfer = PL_COLOR_TRC_PQ;
// Otherwise, if max_luma of the display is less than paper white, there are
// two cases:
// 1. source_max_luma > csp.hdr.max_luma: In this case, we want libplacebo
// to tone map to the max_luma of the display.
// 2. source_max_luma <= csp.hdr.max_luma: In this case, we still need to
// tell the compositor that the max_luma of our images is
// < csp.hdr.max_luma since otherwise it will assume that the max_luma of
// or images is paper white and might do its own tone mapping down to
// csp.hdr.max_luma.
// So in both cases we need to pass the actual max_luma of the display through
// the pipeline.
if (csp.hdr.max_luma < PL_COLOR_SDR_WHITE)
csp.transfer = PL_COLOR_TRC_PQ;

// NOTE: If source_max_luma > csp.hdr.max_luma, which seems to be missing from
// the conditions above, then either source_max_luma > PL_COLOR_SDR_WHITE, in
// which case we hit the second branch above, or
// csp.hdr.max_luma < source_max_luma <= PL_COLOR_WHITE in which case we hit
// the third branch above.
}
}
return csp;
}

int vo_wayland_control(struct vo *vo, int *events, int request, void *arg)
Expand Down
2 changes: 1 addition & 1 deletion video/out/wayland_common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ struct vo_wayland_state {
};

bool vo_wayland_check_visible(struct vo *vo);
struct pl_color_space vo_wayland_preferred_csp(struct vo *vo);
struct pl_color_space vo_wayland_preferred_csp(struct vo *vo, float source_max_luma);
bool vo_wayland_valid_format(struct vo_wayland_state *wl, uint32_t drm_format, uint64_t modifier);
bool vo_wayland_init(struct vo *vo);
bool vo_wayland_reconfig(struct vo *vo);
Expand Down
Loading