Skip to content

mp_image: handle XYZ, and forward to libplacebo properly #18124

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
cyanreg wants to merge 1 commit into
base: master
Choose a base branch
from
+32 −14
Open

mp_image: handle XYZ, and forward to libplacebo properly #18124

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
46 changes: 32 additions & 14 deletions video/mp_image.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1008,23 +1008,31 @@ void mp_image_params_guess_csp(struct mp_image_params *params)
params->color.transfer = params->repr.levels == PL_COLOR_LEVELS_LIMITED ?
PL_COLOR_TRC_BT_1886 : PL_COLOR_TRC_SRGB;
} else if (forced_csp == PL_COLOR_SYSTEM_RGB) {
params->repr.sys = PL_COLOR_SYSTEM_RGB;
params->repr.levels = PL_COLOR_LEVELS_FULL;

// The majority of RGB content is either sRGB or (rarely) some other
// color space which we don't even handle, like AdobeRGB or
// ProPhotoRGB. The only reasonable thing we can do is assume it's
// sRGB and hope for the best, which should usually just work out fine.
// Note: sRGB primaries = BT.709 primaries
if (params->color.primaries == PL_COLOR_PRIM_UNKNOWN)
params->color.primaries = PL_COLOR_PRIM_BT_709;
if (params->color.transfer == PL_COLOR_TRC_UNKNOWN)
params->color.transfer = PL_COLOR_TRC_SRGB;
// Respect an explicit XYZ system tagging
if (params->repr.sys != PL_COLOR_SYSTEM_XYZ) {

@kasper93 kasper93 Jun 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is dead code, no?

mp_image_params_get_forced_csp already does:

    if (csp == PL_COLOR_SYSTEM_RGB && params->repr.sys == PL_COLOR_SYSTEM_XYZ)
        csp = PL_COLOR_SYSTEM_XYZ;

params->repr.sys = PL_COLOR_SYSTEM_RGB;
params->repr.levels = PL_COLOR_LEVELS_FULL;

// The majority of RGB content is either sRGB or (rarely) some other
// color space which we don't even handle, like AdobeRGB or
// ProPhotoRGB. The only reasonable thing we can do is assume it's
// sRGB and hope for the best, which should usually just work out fine.
// Note: sRGB primaries = BT.709 primaries
if (params->color.primaries == PL_COLOR_PRIM_UNKNOWN)
params->color.primaries = PL_COLOR_PRIM_BT_709;
if (params->color.transfer == PL_COLOR_TRC_UNKNOWN)
params->color.transfer = PL_COLOR_TRC_SRGB;
} else {
params->repr.levels = PL_COLOR_LEVELS_FULL;
}
} else if (forced_csp == PL_COLOR_SYSTEM_XYZ) {
params->repr.sys = PL_COLOR_SYSTEM_XYZ;
params->repr.levels = PL_COLOR_LEVELS_FULL;
// Force gamma to ST428 as this is the only correct for DCDM X'Y'Z'
params->color.transfer = PL_COLOR_TRC_ST428;
// Default the gamma to ST428, the only correct value for DCDM
// X'Y'Z', but respect an explicit transfer tag, since linear-light
// XYZ (e.g. scene-referred camera raw) must keep TRC=LINEAR.
if (params->color.transfer == PL_COLOR_TRC_UNKNOWN)
params->color.transfer = PL_COLOR_TRC_ST428;
// Don't care about primaries, they shouldn't be used, or if anything
// MP_CSP_PRIM_ST428 should be defined.
} else {
Expand Down Expand Up @@ -1118,6 +1126,16 @@ struct mp_image *mp_image_from_av_frame(struct AVFrame *src)
.transfer = pl_transfer_from_av(src->color_trc),
};

// SMPTE 428-1 / CIE XYZ tristimulus inputs aren't an RGB primary set,
// so route through PL_COLOR_SYSTEM_XYZ so libplacebo applies the XYZ to RGB
// decode, and tag post-decode primaries as DCI-P3 to match what its XYZ
// handler bakes in. Don't override the TRC: linear-light XYZ keeps
// TRC=LINEAR, DCDM-encoded XYZ keeps TRC=ST428.
if (src->color_primaries == AVCOL_PRI_SMPTE428) {

@kasper93 kasper93 Jun 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

XYZ is almost never tagged, as such.

@ValeZAA ValeZAA Jun 14, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

XYZ is typically «fake» tagged as Identity (Identity matrix means either BGR or YZX) and primaries of white point 1/3, 1/3, unity, and gamma 2.6. Primaries is what signals that Identity here means YZX and not BGR. DCI-P3 would be what encoding to linear RGB from XYZ NPM matrix is, I believe. FFmpeg does not preserve WCG here encoding using BT.709 NPM. New patch apparently adds support for new NPMs. So you decode YZX using 2.6 gamma and then convert to linear RGB (e.g. of P3 with DCI white NPM) and then encode with PQ or whatever. Tag it all in the end with PQ/DCI-P3.

@kasper93 kasper93 Jun 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I know, there is big gaps in DCDM support in ffmpeg. I made it work somehow good in mpv/libplacebo to get satisfactory rendering.

dst->params.repr.sys = PL_COLOR_SYSTEM_XYZ;
dst->params.color.primaries = PL_COLOR_PRIM_BT_2020;
}

dst->params.chroma_location = pl_chroma_from_av(src->chroma_location);

if (src->opaque_ref) {
Expand Down
Loading