Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions bindings/rust/libdrmtap-sys/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
27 changes: 27 additions & 0 deletions bindings/rust/libdrmtap-sys/csrc/drmtap.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) */
/* ========================================================================= */
Expand Down
132 changes: 132 additions & 0 deletions bindings/rust/libdrmtap-sys/csrc/pixel_convert.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
#include <string.h>
#include <stdint.h>
#include <stdlib.h>
#include <math.h>
#include <pthread.h>

#include "drmtap.h"

Expand Down Expand Up @@ -324,6 +326,136 @@ 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 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 * 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;

/* 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;
}
/* 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) {
Comment on lines +437 to +439

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Mirror the pre-multiply overflow guard here too.

This Rust sys copy has the same wrapped-row_bytes risk on 32-bit size_t; keep it in lockstep with src/pixel_convert.c by checking width before multiplying.

Proposed fix
-    size_t row_bytes = (size_t)width * 4u;
-    if (row_bytes > UINT32_MAX || src_stride < row_bytes ||
-        dst_stride < row_bytes) {
+    if (width > UINT32_MAX / sizeof(uint32_t)) {
+        return -EINVAL;
+    }
+    size_t row_bytes = (size_t)width * sizeof(uint32_t);
+    if (src_stride < row_bytes || dst_stride < row_bytes) {
         return -EINVAL;
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
size_t row_bytes = (size_t)width * 4u;
if (row_bytes > UINT32_MAX || src_stride < row_bytes ||
dst_stride < row_bytes) {
if (width > UINT32_MAX / sizeof(uint32_t)) {
return -EINVAL;
}
size_t row_bytes = (size_t)width * sizeof(uint32_t);
if (src_stride < row_bytes || dst_stride < row_bytes) {
return -EINVAL;
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bindings/rust/libdrmtap-sys/csrc/pixel_convert.c` around lines 437 - 439, The
row size calculation in pixel_convert.c has the same wrapped-`row_bytes` risk as
the pre-multiply path in src/pixel_convert.c, so keep the guards in sync by
validating `width` before computing `row_bytes` in the conversion logic around
the stride checks. Update the relevant pre-multiply/row-bytes guard near the
existing `row_bytes`, `src_stride`, and `dst_stride` validation so the overflow
protection happens before the multiplication and matches the behavior of the
source implementation.

return -EINVAL;
}

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);
Comment on lines +424 to +448

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Mirror the row-stride validation in the Rust sys copy.

This copy also indexes width 32-bit pixels per row without ensuring src_stride and dst_stride are at least width * 4, so invalid caller strides can overread or overwrite memory.

Proposed validation
     if (src_format != 0x30335258u && src_format != 0x30335241u) {
         return -ENOTSUP;
     }
 
+    size_t row_bytes = (size_t)width * sizeof(uint32_t);
+    if (row_bytes > UINT32_MAX ||
+        src_stride < row_bytes ||
+        dst_stride < row_bytes) {
+        return -EINVAL;
+    }
+
     pthread_once(&g_hdr_once, hdr_lut_init);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
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);
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;
}
size_t row_bytes = (size_t)width * sizeof(uint32_t);
if (row_bytes > UINT32_MAX ||
src_stride < row_bytes ||
dst_stride < row_bytes) {
return -EINVAL;
}
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);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bindings/rust/libdrmtap-sys/csrc/pixel_convert.c` around lines 424 - 441, The
row-stride checks in drmtap_tonemap_hdr10 are missing in this Rust sys copy, so
callers can pass src_stride or dst_stride smaller than width * 4 and trigger
out-of-bounds access when the per-row loop indexes 32-bit pixels. Add the same
validation used elsewhere in pixel_convert.c before pthread_once/hdr_lut_init
and the row loop, rejecting inputs where either stride is too small for the
requested width, and keep the existing drmtap_tonemap_hdr10 argument checks and
format gate intact.

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,
Expand Down
27 changes: 27 additions & 0 deletions include/drmtap.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) */
/* ========================================================================= */
Expand Down
11 changes: 10 additions & 1 deletion meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -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
# ============================================================================
Expand All @@ -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(
Expand Down Expand Up @@ -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
Expand Down
Loading
Loading