Skip to content

Conversation

@mahkoh
Copy link
Contributor

@mahkoh mahkoh commented Dec 19, 2025

This PR contains the first commit from #17070 with some small changes.

Comment on lines +2157 to +2158
float a = wd->min_luma;
float b = (PL_COLOR_SDR_WHITE - PL_COLOR_HDR_BLACK) / (wd->ref_luma - a);
Copy link
Contributor Author

@mahkoh mahkoh Dec 19, 2025

Choose a reason for hiding this comment

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

I'm just mapping primary color volume min_luma to PL_COLOR_HDR_BLACK now. I don't know if there is anything more sensible to do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You might want to test this with some dark videos to see if the blacks come out as the proper shade of black.

Copy link
Member

Choose a reason for hiding this comment

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

PL_COLOR_HDR_BLACK is effectively 0, it's just sentinel value to discriminate from "undefined" which is 0.

It shouldn't make a difference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is so, but the goal of this mapping is to ensure that 0 in nominal space A maps to 0 in nominal space B. But 0 in nominal space is usually not 0 luminance. For most SDR transfer functions 0 in nominal space is 0.2 cd/m^2 and for PQ 0 in nominal space is 0.005 cd/m^2. I'd have to know what 0 is in the nominal space assumed by libplacebo.

Copy link
Member

@kasper93 kasper93 Dec 19, 2025

Choose a reason for hiding this comment

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

Note that it depends on the values in csp. If things are set, they will be used, if not. Default is used instead, which is 0.203 (203/1000) for SDR and 0 for PQ. Generally internal absolute black is always 0 in libplacebo and SDR is shifted according to min_luma value.

float min, max;
pl_color_space_nominal_luma_ex(pl_nominal_luma_params(
    .color    = &csp,
    .metadata = PL_HDR_METADATA_HDR10,
    .scaling  = PL_HDR_NITS,
    .out_min  = &min,
    .out_max  = &max,
));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So something like this: afaedac?

wd->csp.hdr.max_fall = MPMIN(wd->csp.hdr.max_fall, wd->csp.hdr.max_luma);
}
wl->preferred_csp = wd->csp;
if (wd->csp.hdr.max_luma != PL_COLOR_SDR_WHITE) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed this to != since that is the only case where it is ok for the rest of mpv to ignore the max_luma.

Copy link
Member

@kasper93 kasper93 left a comment

Choose a reason for hiding this comment

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

I think it make sense. We can always improve on case by case basis, should there be any issues.

@github-actions
Copy link

@sfan5 sfan5 merged commit ae211f7 into mpv-player:master Dec 19, 2025
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants