From 2efda9eb17eb787e3c08e35efbbd53e388bb1efe Mon Sep 17 00:00:00 2001 From: Mariano Abad Date: Fri, 26 Jun 2026 16:48:33 -0300 Subject: [PATCH 1/2] hdr: add correct HDR10 (PQ/BT.2020) -> SDR tone-map primitive + test First piece of #16. drmtap_convert_format's AR30/XR30 path is a naive >>2 bit-shift: fine for SDR 10-bit, but for HDR10 it just keeps the top 8 of the 10 PQ-encoded bits with no PQ decode / gamut / tone-map, so an HDR scanout captures washed out (a 1000-nit highlight came out ~53% gray). Add drmtap_tonemap_hdr10(): PQ (SMPTE ST 2084) EOTF to linear light, BT.2020 -> BT.709 gamut, a highlight-preserving tone-map to the SDR range (diffuse white = 203 nits per BT.2408, highlights rolled off toward white), then the sRGB OETF back to 8-bit. PQ and sRGB are table-driven (built once via pthread_once); only the 3x3 gamut matrix and the tone curve run per pixel. test_hdr.c validates the curve without HDR hardware: it PQ-encodes known luminances and checks the 8-bit result is monotonic, dark stays dark, SDR white maps bright (250, not crushed), and highlights reach white. This is the conversion primitive only. Deciding when to apply it (HDR vs plain SDR 10-bit, from the connector Colorspace / HDR_OUTPUT_METADATA, since the pixel format alone does not say) and wiring it into the capture pipeline is the next step on #16; the EGL readback path and the XR48/AR48/P010 formats also remain. Part of #16 --- bindings/rust/libdrmtap-sys/build.rs | 2 + bindings/rust/libdrmtap-sys/csrc/drmtap.h | 27 ++++ .../rust/libdrmtap-sys/csrc/pixel_convert.c | 125 ++++++++++++++++++ include/drmtap.h | 27 ++++ meson.build | 11 +- src/pixel_convert.c | 125 ++++++++++++++++++ tests/test_hdr.c | 101 ++++++++++++++ 7 files changed, 417 insertions(+), 1 deletion(-) create mode 100644 tests/test_hdr.c diff --git a/bindings/rust/libdrmtap-sys/build.rs b/bindings/rust/libdrmtap-sys/build.rs index 2e9c424..be340ff 100644 --- a/bindings/rust/libdrmtap-sys/build.rs +++ b/bindings/rust/libdrmtap-sys/build.rs @@ -59,6 +59,8 @@ fn main() { println!("cargo:rustc-link-lib=GLESv2"); println!("cargo:rustc-link-lib=seccomp"); println!("cargo:rustc-link-lib=cap"); + // libm: HDR tone-mapping in pixel_convert.c uses pow()/tanh(). + println!("cargo:rustc-link-lib=m"); // Compile drmtap-helper as a standalone executable. // It inherits a socketpair fd from the parent, opens the DRM device with diff --git a/bindings/rust/libdrmtap-sys/csrc/drmtap.h b/bindings/rust/libdrmtap-sys/csrc/drmtap.h index 432e966..ed812d6 100644 --- a/bindings/rust/libdrmtap-sys/csrc/drmtap.h +++ b/bindings/rust/libdrmtap-sys/csrc/drmtap.h @@ -317,6 +317,33 @@ int drmtap_convert_format(const void *src, void *dst, uint32_t src_stride, uint32_t dst_stride, uint32_t src_format, uint32_t dst_format); +/** + * @brief Tone-map an HDR10 (PQ, BT.2020) framebuffer to SDR 8-bit XRGB8888. + * + * Unlike drmtap_convert_format()'s naive bit-shift (which is only correct for + * SDR 10-bit), this applies the real HDR10 transfer: PQ (SMPTE ST 2084) EOTF + * to linear light, BT.2020 -> BT.709 gamut mapping, a highlight-preserving + * tone-map down to the SDR range, and the sRGB OETF back to 8-bit. Use it only + * when the scanout is actually HDR (decided from the connector Colorspace / + * HDR_OUTPUT_METADATA, not from the pixel format alone — XR30/AR30 is also used + * for plain SDR 10-bit). + * + * Supported src_format: XR30/AR30 (ARGB2101010). Others return -ENOTSUP. + * + * @param src Source pixel data + * @param dst Destination buffer (XRGB8888) + * @param width Frame width in pixels + * @param height Frame height in pixels + * @param src_stride Source stride (bytes per row) + * @param dst_stride Destination stride (bytes per row) + * @param src_format Source DRM fourcc (DRM_FORMAT_XRGB2101010 / ARGB2101010) + * @return 0 on success, -ENOTSUP for an unsupported format, -EINVAL on bad args + */ +int drmtap_tonemap_hdr10(const void *src, void *dst, + uint32_t width, uint32_t height, + uint32_t src_stride, uint32_t dst_stride, + uint32_t src_format); + /* ========================================================================= */ /* Frame Differencing (optional utility) */ /* ========================================================================= */ diff --git a/bindings/rust/libdrmtap-sys/csrc/pixel_convert.c b/bindings/rust/libdrmtap-sys/csrc/pixel_convert.c index 0fab80c..a4f9c38 100644 --- a/bindings/rust/libdrmtap-sys/csrc/pixel_convert.c +++ b/bindings/rust/libdrmtap-sys/csrc/pixel_convert.c @@ -25,6 +25,8 @@ #include #include #include +#include +#include #include "drmtap.h" @@ -324,6 +326,129 @@ int drmtap_deswizzle(const void *src, void *dst, /* Public API — Format Conversion */ /* ========================================================================= */ +/* ========================================================================= */ +/* HDR10 (PQ / BT.2020) -> SDR (BT.709 / sRGB) tone mapping */ +/* ========================================================================= */ + +/* BT.2408 reference graphics/diffuse white. HDR diffuse white sits here; this + * is the luminance we map to SDR "1.0" so SDR-range content keeps its normal + * brightness and only true highlights above it get rolled off. */ +#define DRMTAP_SDR_WHITE_NITS 203.0 + +#define DRMTAP_PQ_LUT_N 1024 /* one entry per 10-bit code value */ +#define DRMTAP_SRGB_LUT_N 4096 /* linear [0,1] -> 8-bit sRGB */ + +static float g_pq_lut[DRMTAP_PQ_LUT_N]; /* code -> luminance (nits) */ +static uint8_t g_srgb_lut[DRMTAP_SRGB_LUT_N]; /* linear [0,1] -> 8-bit sRGB */ +static pthread_once_t g_hdr_once = PTHREAD_ONCE_INIT; + +/* SMPTE ST 2084 (PQ) EOTF. Input: normalized code value [0,1]. + * Output: absolute display luminance in nits (cd/m^2), 0..10000. */ +static double pq_eotf_nits(double e) { + const double m1 = 2610.0 / 16384.0; + const double m2 = 2523.0 / 4096.0 * 128.0; + const double c1 = 3424.0 / 4096.0; + const double c2 = 2413.0 / 4096.0 * 32.0; + const double c3 = 2392.0 / 4096.0 * 32.0; + if (e <= 0.0) return 0.0; + double ep = pow(e, 1.0 / m2); + double num = ep - c1; + if (num < 0.0) num = 0.0; + double den = c2 - c3 * ep; + if (den <= 0.0) return 10000.0; + return 10000.0 * pow(num / den, 1.0 / m1); +} + +/* sRGB OETF: linear [0,1] -> non-linear sRGB [0,1]. */ +static double srgb_oetf(double c) { + if (c <= 0.0) return 0.0; + if (c >= 1.0) return 1.0; + if (c <= 0.0031308) return 12.92 * c; + return 1.055 * pow(c, 1.0 / 2.4) - 0.055; +} + +static void hdr_lut_init(void) { + for (int i = 0; i < DRMTAP_PQ_LUT_N; i++) { + g_pq_lut[i] = (float)pq_eotf_nits((double)i / (DRMTAP_PQ_LUT_N - 1)); + } + for (int i = 0; i < DRMTAP_SRGB_LUT_N; i++) { + double s = srgb_oetf((double)i / (DRMTAP_SRGB_LUT_N - 1)); + int v = (int)(s * 255.0 + 0.5); + if (v < 0) v = 0; + if (v > 255) v = 255; + g_srgb_lut[i] = (uint8_t)v; + } +} + +/* Highlight-preserving tone curve. Input x is linear light normalized so that + * 1.0 == SDR diffuse white: x <= knee passes through (SDR content keeps its + * brightness), and x above the knee rolls off smoothly toward 1.0 (HDR + * highlights, which an SDR display cannot show brighter than white). */ +static double tonemap_softknee(double x) { + const double knee = 0.80; + if (x <= knee) return x; + if (x >= 100.0) return 1.0; + return knee + (1.0 - knee) * tanh((x - knee) / (1.0 - knee)); +} + +/* sRGB-encode a linear [0,inf) channel via the precomputed LUT. */ +static uint8_t to_srgb8(double linear) { + if (linear <= 0.0) return g_srgb_lut[0]; + if (linear >= 1.0) return g_srgb_lut[DRMTAP_SRGB_LUT_N - 1]; + int idx = (int)(linear * (DRMTAP_SRGB_LUT_N - 1) + 0.5); + return g_srgb_lut[idx]; +} + +/* Map one HDR10 (PQ, BT.2020) ARGB2101010 pixel to SDR 8-bit R/G/B. */ +static void tonemap_ar30_pixel(uint32_t pixel, + uint8_t *r8, uint8_t *g8, uint8_t *b8) { + /* 10-bit channels, ARGB2101010: R[29:20] G[19:10] B[9:0]. */ + double Rl = g_pq_lut[(pixel >> 20) & 0x3FF]; /* BT.2020 linear nits */ + double Gl = g_pq_lut[(pixel >> 10) & 0x3FF]; + double Bl = g_pq_lut[(pixel) & 0x3FF]; + + /* BT.2020 -> BT.709 in linear light. */ + double r = 1.660491 * Rl - 0.587641 * Gl - 0.072850 * Bl; + double g = -0.124550 * Rl + 1.132900 * Gl - 0.008349 * Bl; + double b = -0.018151 * Rl - 0.100579 * Gl + 1.118730 * Bl; + if (r < 0.0) r = 0.0; + if (g < 0.0) g = 0.0; + if (b < 0.0) b = 0.0; + + /* Normalize to SDR white, roll off highlights, sRGB-encode. */ + *r8 = to_srgb8(tonemap_softknee(r / DRMTAP_SDR_WHITE_NITS)); + *g8 = to_srgb8(tonemap_softknee(g / DRMTAP_SDR_WHITE_NITS)); + *b8 = to_srgb8(tonemap_softknee(b / DRMTAP_SDR_WHITE_NITS)); +} + +int drmtap_tonemap_hdr10(const void *src, void *dst, + uint32_t width, uint32_t height, + uint32_t src_stride, uint32_t dst_stride, + uint32_t src_format) { + if (!src || !dst || width == 0 || height == 0) { + return -EINVAL; + } + /* XR30 = XRGB2101010 ('XR30'), AR30 = ARGB2101010 ('AR30'). */ + if (src_format != 0x30335258u && src_format != 0x30335241u) { + return -ENOTSUP; + } + + pthread_once(&g_hdr_once, hdr_lut_init); + + for (uint32_t y = 0; y < height; y++) { + const uint32_t *s = (const uint32_t *) + ((const uint8_t *)src + (size_t)y * src_stride); + uint32_t *d = (uint32_t *)((uint8_t *)dst + (size_t)y * dst_stride); + for (uint32_t x = 0; x < width; x++) { + uint8_t r, g, b; + tonemap_ar30_pixel(s[x], &r, &g, &b); + d[x] = (0xFFu << 24) | ((uint32_t)r << 16) | + ((uint32_t)g << 8) | b; + } + } + return 0; +} + int drmtap_convert_format(const void *src, void *dst, uint32_t width, uint32_t height, uint32_t src_stride, uint32_t dst_stride, diff --git a/include/drmtap.h b/include/drmtap.h index 432e966..ed812d6 100644 --- a/include/drmtap.h +++ b/include/drmtap.h @@ -317,6 +317,33 @@ int drmtap_convert_format(const void *src, void *dst, uint32_t src_stride, uint32_t dst_stride, uint32_t src_format, uint32_t dst_format); +/** + * @brief Tone-map an HDR10 (PQ, BT.2020) framebuffer to SDR 8-bit XRGB8888. + * + * Unlike drmtap_convert_format()'s naive bit-shift (which is only correct for + * SDR 10-bit), this applies the real HDR10 transfer: PQ (SMPTE ST 2084) EOTF + * to linear light, BT.2020 -> BT.709 gamut mapping, a highlight-preserving + * tone-map down to the SDR range, and the sRGB OETF back to 8-bit. Use it only + * when the scanout is actually HDR (decided from the connector Colorspace / + * HDR_OUTPUT_METADATA, not from the pixel format alone — XR30/AR30 is also used + * for plain SDR 10-bit). + * + * Supported src_format: XR30/AR30 (ARGB2101010). Others return -ENOTSUP. + * + * @param src Source pixel data + * @param dst Destination buffer (XRGB8888) + * @param width Frame width in pixels + * @param height Frame height in pixels + * @param src_stride Source stride (bytes per row) + * @param dst_stride Destination stride (bytes per row) + * @param src_format Source DRM fourcc (DRM_FORMAT_XRGB2101010 / ARGB2101010) + * @return 0 on success, -ENOTSUP for an unsupported format, -EINVAL on bad args + */ +int drmtap_tonemap_hdr10(const void *src, void *dst, + uint32_t width, uint32_t height, + uint32_t src_stride, uint32_t dst_stride, + uint32_t src_format); + /* ========================================================================= */ /* Frame Differencing (optional utility) */ /* ========================================================================= */ diff --git a/meson.build b/meson.build index 952d63c..91520e8 100644 --- a/meson.build +++ b/meson.build @@ -59,6 +59,9 @@ seccomp_dep = dependency('libseccomp', required: false) # Optional: libcap for capability management libcap_dep = dependency('libcap', required: false) +# libm: HDR tone-mapping in pixel_convert.c uses pow() (PQ EOTF / sRGB OETF). +m_dep = meson.get_compiler('c').find_library('m', required: false) + # ============================================================================ # Library sources # ============================================================================ @@ -72,7 +75,7 @@ lib_sources = files( 'src/privilege_helper.c', ) -lib_deps = [libdrm_dep, threads_dep] +lib_deps = [libdrm_dep, threads_dep, m_dep] # GPU backends: Intel and AMD (always compiled — match functions needed by drm_grab.c) lib_sources += files( @@ -205,11 +208,17 @@ test_helper = executable('test_helper', dependencies: [libdrmtap_dep], include_directories: lib_include, ) +test_hdr = executable('test_hdr', + 'tests/test_hdr.c', + dependencies: [libdrmtap_dep, m_dep], + include_directories: lib_include, +) # Unit tests (no hardware needed) test('formats', test_formats, suite: 'unit') test('deswizzle', test_deswizzle, suite: 'unit') test('helper', test_helper, suite: 'unit') +test('hdr', test_hdr, suite: 'unit') # Integration tests (need vkms or real GPU) # LSan: suppress the deliberate process-lifetime EGL/Mesa display allocation diff --git a/src/pixel_convert.c b/src/pixel_convert.c index 0fab80c..a4f9c38 100644 --- a/src/pixel_convert.c +++ b/src/pixel_convert.c @@ -25,6 +25,8 @@ #include #include #include +#include +#include #include "drmtap.h" @@ -324,6 +326,129 @@ int drmtap_deswizzle(const void *src, void *dst, /* Public API — Format Conversion */ /* ========================================================================= */ +/* ========================================================================= */ +/* HDR10 (PQ / BT.2020) -> SDR (BT.709 / sRGB) tone mapping */ +/* ========================================================================= */ + +/* BT.2408 reference graphics/diffuse white. HDR diffuse white sits here; this + * is the luminance we map to SDR "1.0" so SDR-range content keeps its normal + * brightness and only true highlights above it get rolled off. */ +#define DRMTAP_SDR_WHITE_NITS 203.0 + +#define DRMTAP_PQ_LUT_N 1024 /* one entry per 10-bit code value */ +#define DRMTAP_SRGB_LUT_N 4096 /* linear [0,1] -> 8-bit sRGB */ + +static float g_pq_lut[DRMTAP_PQ_LUT_N]; /* code -> luminance (nits) */ +static uint8_t g_srgb_lut[DRMTAP_SRGB_LUT_N]; /* linear [0,1] -> 8-bit sRGB */ +static pthread_once_t g_hdr_once = PTHREAD_ONCE_INIT; + +/* SMPTE ST 2084 (PQ) EOTF. Input: normalized code value [0,1]. + * Output: absolute display luminance in nits (cd/m^2), 0..10000. */ +static double pq_eotf_nits(double e) { + const double m1 = 2610.0 / 16384.0; + const double m2 = 2523.0 / 4096.0 * 128.0; + const double c1 = 3424.0 / 4096.0; + const double c2 = 2413.0 / 4096.0 * 32.0; + const double c3 = 2392.0 / 4096.0 * 32.0; + if (e <= 0.0) return 0.0; + double ep = pow(e, 1.0 / m2); + double num = ep - c1; + if (num < 0.0) num = 0.0; + double den = c2 - c3 * ep; + if (den <= 0.0) return 10000.0; + return 10000.0 * pow(num / den, 1.0 / m1); +} + +/* sRGB OETF: linear [0,1] -> non-linear sRGB [0,1]. */ +static double srgb_oetf(double c) { + if (c <= 0.0) return 0.0; + if (c >= 1.0) return 1.0; + if (c <= 0.0031308) return 12.92 * c; + return 1.055 * pow(c, 1.0 / 2.4) - 0.055; +} + +static void hdr_lut_init(void) { + for (int i = 0; i < DRMTAP_PQ_LUT_N; i++) { + g_pq_lut[i] = (float)pq_eotf_nits((double)i / (DRMTAP_PQ_LUT_N - 1)); + } + for (int i = 0; i < DRMTAP_SRGB_LUT_N; i++) { + double s = srgb_oetf((double)i / (DRMTAP_SRGB_LUT_N - 1)); + int v = (int)(s * 255.0 + 0.5); + if (v < 0) v = 0; + if (v > 255) v = 255; + g_srgb_lut[i] = (uint8_t)v; + } +} + +/* Highlight-preserving tone curve. Input x is linear light normalized so that + * 1.0 == SDR diffuse white: x <= knee passes through (SDR content keeps its + * brightness), and x above the knee rolls off smoothly toward 1.0 (HDR + * highlights, which an SDR display cannot show brighter than white). */ +static double tonemap_softknee(double x) { + const double knee = 0.80; + if (x <= knee) return x; + if (x >= 100.0) return 1.0; + return knee + (1.0 - knee) * tanh((x - knee) / (1.0 - knee)); +} + +/* sRGB-encode a linear [0,inf) channel via the precomputed LUT. */ +static uint8_t to_srgb8(double linear) { + if (linear <= 0.0) return g_srgb_lut[0]; + if (linear >= 1.0) return g_srgb_lut[DRMTAP_SRGB_LUT_N - 1]; + int idx = (int)(linear * (DRMTAP_SRGB_LUT_N - 1) + 0.5); + return g_srgb_lut[idx]; +} + +/* Map one HDR10 (PQ, BT.2020) ARGB2101010 pixel to SDR 8-bit R/G/B. */ +static void tonemap_ar30_pixel(uint32_t pixel, + uint8_t *r8, uint8_t *g8, uint8_t *b8) { + /* 10-bit channels, ARGB2101010: R[29:20] G[19:10] B[9:0]. */ + double Rl = g_pq_lut[(pixel >> 20) & 0x3FF]; /* BT.2020 linear nits */ + double Gl = g_pq_lut[(pixel >> 10) & 0x3FF]; + double Bl = g_pq_lut[(pixel) & 0x3FF]; + + /* BT.2020 -> BT.709 in linear light. */ + double r = 1.660491 * Rl - 0.587641 * Gl - 0.072850 * Bl; + double g = -0.124550 * Rl + 1.132900 * Gl - 0.008349 * Bl; + double b = -0.018151 * Rl - 0.100579 * Gl + 1.118730 * Bl; + if (r < 0.0) r = 0.0; + if (g < 0.0) g = 0.0; + if (b < 0.0) b = 0.0; + + /* Normalize to SDR white, roll off highlights, sRGB-encode. */ + *r8 = to_srgb8(tonemap_softknee(r / DRMTAP_SDR_WHITE_NITS)); + *g8 = to_srgb8(tonemap_softknee(g / DRMTAP_SDR_WHITE_NITS)); + *b8 = to_srgb8(tonemap_softknee(b / DRMTAP_SDR_WHITE_NITS)); +} + +int drmtap_tonemap_hdr10(const void *src, void *dst, + uint32_t width, uint32_t height, + uint32_t src_stride, uint32_t dst_stride, + uint32_t src_format) { + if (!src || !dst || width == 0 || height == 0) { + return -EINVAL; + } + /* XR30 = XRGB2101010 ('XR30'), AR30 = ARGB2101010 ('AR30'). */ + if (src_format != 0x30335258u && src_format != 0x30335241u) { + return -ENOTSUP; + } + + pthread_once(&g_hdr_once, hdr_lut_init); + + for (uint32_t y = 0; y < height; y++) { + const uint32_t *s = (const uint32_t *) + ((const uint8_t *)src + (size_t)y * src_stride); + uint32_t *d = (uint32_t *)((uint8_t *)dst + (size_t)y * dst_stride); + for (uint32_t x = 0; x < width; x++) { + uint8_t r, g, b; + tonemap_ar30_pixel(s[x], &r, &g, &b); + d[x] = (0xFFu << 24) | ((uint32_t)r << 16) | + ((uint32_t)g << 8) | b; + } + } + return 0; +} + int drmtap_convert_format(const void *src, void *dst, uint32_t width, uint32_t height, uint32_t src_stride, uint32_t dst_stride, diff --git a/tests/test_hdr.c b/tests/test_hdr.c new file mode 100644 index 0000000..af73eca --- /dev/null +++ b/tests/test_hdr.c @@ -0,0 +1,101 @@ +/* + * libdrmtap — DRM/KMS screen capture library for Linux + * https://github.com/fxd0h/libdrmtap + * + * Copyright (c) 2026 Mariano Abad + * SPDX-License-Identifier: MIT + */ + +/** + * @file test_hdr.c + * @brief Unit test — HDR10 (PQ/BT.2020) -> SDR tone mapping. + * + * No HDR hardware needed: we PQ-encode known absolute luminances (nits) into an + * AR30 buffer, run drmtap_tonemap_hdr10(), and check the resulting 8-bit curve + * is sane — SDR diffuse white stays bright (the bug the naive >>2 shift caused), + * the mapping is monotonic, dark stays dark, and peak highlights reach white. + */ + +#include +#include +#include +#include +#include + +#include "drmtap.h" + +#define TEST_ASSERT(cond) do { \ + if (!(cond)) { \ + fprintf(stderr, "FAIL: %s:%d: %s\n", __FILE__, __LINE__, #cond); \ + exit(1); \ + } \ +} while (0) + +/* Inverse PQ (nits -> normalized code [0,1]), to build known HDR inputs. */ +static double pq_inverse(double nits) { + const double m1 = 2610.0 / 16384.0; + const double m2 = 2523.0 / 4096.0 * 128.0; + const double c1 = 3424.0 / 4096.0; + const double c2 = 2413.0 / 4096.0 * 32.0; + const double c3 = 2392.0 / 4096.0 * 32.0; + double Lp = nits / 10000.0; + if (Lp < 0.0) Lp = 0.0; + double Lm = pow(Lp, m1); + return pow((c1 + c2 * Lm) / (1.0 + c3 * Lm), m2); +} + +/* One gray AR30 pixel (R=G=B) at the given absolute luminance. */ +static uint32_t ar30_gray(double nits) { + double e = pq_inverse(nits); + uint32_t c = (uint32_t)(e * 1023.0 + 0.5); + if (c > 1023) c = 1023; + return (c << 20) | (c << 10) | c; +} + +/* Tone-map a gray HDR pixel of `nits` and return its 8-bit value. */ +static int map_nits(double nits) { + uint32_t in = ar30_gray(nits); + uint32_t out = 0; + int r = drmtap_tonemap_hdr10(&in, &out, 1, 1, 4, 4, 0x30335258u /* XR30 */); + TEST_ASSERT(r == 0); + return (int)((out >> 16) & 0xFF); /* gray -> all channels equal */ +} + +static void test_curve(void) { + int n5 = map_nits(5); + int n100 = map_nits(100); + int n203 = map_nits(203); /* BT.2408 SDR diffuse white */ + int n600 = map_nits(600); + int n1000 = map_nits(1000); + printf(" nits->8bit: 5=%d 100=%d 203(SDR white)=%d 600=%d 1000=%d\n", + n5, n100, n203, n600, n1000); + + /* Monotonic non-decreasing. */ + TEST_ASSERT(n5 <= n100 && n100 <= n203 && n203 <= n600 && n600 <= n1000); + /* The fix: SDR diffuse white maps bright, not crushed to mid-gray like the + * naive top-8-of-10-bits shift did. */ + TEST_ASSERT(n203 >= 230); + /* Dark stays dark. */ + TEST_ASSERT(n5 < 60); + /* Peak highlight reaches white. */ + TEST_ASSERT(n1000 >= 250); + printf(" PASS: tone-map curve sane (SDR white bright, highlights -> white)\n"); +} + +static void test_unsupported(void) { + uint32_t a = 0, b = 0; + /* XR24 (8-bit XRGB8888) is not an HDR source -> -ENOTSUP. */ + int r = drmtap_tonemap_hdr10(&a, &b, 1, 1, 4, 4, 0x34325258u); + TEST_ASSERT(r == -ENOTSUP); + /* Bad args. */ + TEST_ASSERT(drmtap_tonemap_hdr10(NULL, &b, 1, 1, 4, 4, 0x30335258u) == -EINVAL); + printf(" PASS: unsupported format / bad args rejected\n"); +} + +int main(void) { + printf("Running HDR tone-map tests...\n"); + test_curve(); + test_unsupported(); + printf("All HDR tests passed!\n"); + return 0; +} From 9224c8c9c687bea084449708221bb1a7dc7c5df9 Mon Sep 17 00:00:00 2001 From: Mariano Abad Date: Fri, 26 Jun 2026 16:56:36 -0300 Subject: [PATCH 2/2] hdr: validate strides and rename channel temporaries Review feedback. drmtap_tonemap_hdr10() now rejects src/dst strides that cannot hold a full width*4 row (-EINVAL), matching the guards the other conversions use, so a bogus caller stride can't over-read/over-write. Added a test case for it. Renamed the per-pixel linear channel temporaries to snake_case (r_lin/g_lin/b_lin). --- .../rust/libdrmtap-sys/csrc/pixel_convert.c | 19 +++++++++++++------ src/pixel_convert.c | 19 +++++++++++++------ tests/test_hdr.c | 6 +++++- 3 files changed, 31 insertions(+), 13 deletions(-) diff --git a/bindings/rust/libdrmtap-sys/csrc/pixel_convert.c b/bindings/rust/libdrmtap-sys/csrc/pixel_convert.c index a4f9c38..f040538 100644 --- a/bindings/rust/libdrmtap-sys/csrc/pixel_convert.c +++ b/bindings/rust/libdrmtap-sys/csrc/pixel_convert.c @@ -403,14 +403,14 @@ static uint8_t to_srgb8(double linear) { static void tonemap_ar30_pixel(uint32_t pixel, uint8_t *r8, uint8_t *g8, uint8_t *b8) { /* 10-bit channels, ARGB2101010: R[29:20] G[19:10] B[9:0]. */ - double Rl = g_pq_lut[(pixel >> 20) & 0x3FF]; /* BT.2020 linear nits */ - double Gl = g_pq_lut[(pixel >> 10) & 0x3FF]; - double Bl = g_pq_lut[(pixel) & 0x3FF]; + double r_lin = g_pq_lut[(pixel >> 20) & 0x3FF]; /* BT.2020 linear nits */ + double g_lin = g_pq_lut[(pixel >> 10) & 0x3FF]; + double b_lin = g_pq_lut[(pixel) & 0x3FF]; /* BT.2020 -> BT.709 in linear light. */ - double r = 1.660491 * Rl - 0.587641 * Gl - 0.072850 * Bl; - double g = -0.124550 * Rl + 1.132900 * Gl - 0.008349 * Bl; - double b = -0.018151 * Rl - 0.100579 * Gl + 1.118730 * Bl; + double r = 1.660491 * r_lin - 0.587641 * g_lin - 0.072850 * b_lin; + double g = -0.124550 * r_lin + 1.132900 * g_lin - 0.008349 * b_lin; + double b = -0.018151 * r_lin - 0.100579 * g_lin + 1.118730 * b_lin; if (r < 0.0) r = 0.0; if (g < 0.0) g = 0.0; if (b < 0.0) b = 0.0; @@ -432,6 +432,13 @@ int drmtap_tonemap_hdr10(const void *src, void *dst, if (src_format != 0x30335258u && src_format != 0x30335241u) { return -ENOTSUP; } + /* Both buffers are 4 bytes/pixel; a stride that cannot hold a full row + * would make the per-row indexing below over-read or over-write. */ + size_t row_bytes = (size_t)width * 4u; + if (row_bytes > UINT32_MAX || src_stride < row_bytes || + dst_stride < row_bytes) { + return -EINVAL; + } pthread_once(&g_hdr_once, hdr_lut_init); diff --git a/src/pixel_convert.c b/src/pixel_convert.c index a4f9c38..f040538 100644 --- a/src/pixel_convert.c +++ b/src/pixel_convert.c @@ -403,14 +403,14 @@ static uint8_t to_srgb8(double linear) { static void tonemap_ar30_pixel(uint32_t pixel, uint8_t *r8, uint8_t *g8, uint8_t *b8) { /* 10-bit channels, ARGB2101010: R[29:20] G[19:10] B[9:0]. */ - double Rl = g_pq_lut[(pixel >> 20) & 0x3FF]; /* BT.2020 linear nits */ - double Gl = g_pq_lut[(pixel >> 10) & 0x3FF]; - double Bl = g_pq_lut[(pixel) & 0x3FF]; + double r_lin = g_pq_lut[(pixel >> 20) & 0x3FF]; /* BT.2020 linear nits */ + double g_lin = g_pq_lut[(pixel >> 10) & 0x3FF]; + double b_lin = g_pq_lut[(pixel) & 0x3FF]; /* BT.2020 -> BT.709 in linear light. */ - double r = 1.660491 * Rl - 0.587641 * Gl - 0.072850 * Bl; - double g = -0.124550 * Rl + 1.132900 * Gl - 0.008349 * Bl; - double b = -0.018151 * Rl - 0.100579 * Gl + 1.118730 * Bl; + double r = 1.660491 * r_lin - 0.587641 * g_lin - 0.072850 * b_lin; + double g = -0.124550 * r_lin + 1.132900 * g_lin - 0.008349 * b_lin; + double b = -0.018151 * r_lin - 0.100579 * g_lin + 1.118730 * b_lin; if (r < 0.0) r = 0.0; if (g < 0.0) g = 0.0; if (b < 0.0) b = 0.0; @@ -432,6 +432,13 @@ int drmtap_tonemap_hdr10(const void *src, void *dst, if (src_format != 0x30335258u && src_format != 0x30335241u) { return -ENOTSUP; } + /* Both buffers are 4 bytes/pixel; a stride that cannot hold a full row + * would make the per-row indexing below over-read or over-write. */ + size_t row_bytes = (size_t)width * 4u; + if (row_bytes > UINT32_MAX || src_stride < row_bytes || + dst_stride < row_bytes) { + return -EINVAL; + } pthread_once(&g_hdr_once, hdr_lut_init); diff --git a/tests/test_hdr.c b/tests/test_hdr.c index af73eca..d068707 100644 --- a/tests/test_hdr.c +++ b/tests/test_hdr.c @@ -89,7 +89,11 @@ static void test_unsupported(void) { TEST_ASSERT(r == -ENOTSUP); /* Bad args. */ TEST_ASSERT(drmtap_tonemap_hdr10(NULL, &b, 1, 1, 4, 4, 0x30335258u) == -EINVAL); - printf(" PASS: unsupported format / bad args rejected\n"); + /* Strides that cannot hold a full row (2 px need 8 bytes). */ + uint32_t row[2] = {0, 0}; + TEST_ASSERT(drmtap_tonemap_hdr10(row, row, 2, 1, 4, 8, 0x30335258u) == -EINVAL); + TEST_ASSERT(drmtap_tonemap_hdr10(row, row, 2, 1, 8, 4, 0x30335258u) == -EINVAL); + printf(" PASS: unsupported format / bad args / bad stride rejected\n"); } int main(void) {