mirror of
https://github.com/nestriness/nestri.git
synced 2026-09-19 17:25:19 +03:00
fix(nescapture): tag encoded streams full-range to match the samples written (#313)
## The bug
`nescapture` sets the colour converter full-range unconditionally, but
the video usability information carried pixelforge's **default
limited-range flag**. A compliant decoder then expanded 16–235 out of
samples that already covered 0–255 — darkening midtones and clipping
both ends.
pixelforge keeps two separate flags for this, one on the converter and
one on the colour description, and its own documentation says they must
agree. Only the first was being set.
The two lines are about forty apart, each is correct on its own, and the
comment above the second states the right intent while the call below it
does the opposite:
```rust
// GPU framebuffer captures are always full-range — use BT.709 full-range
// so the decoder doesn't apply limited-range expansion.
enc_cfg = enc_cfg.with_color_description(ColorDescription::bt709());
// ^ this constructor is limited-range
```
## Evidence
Measured on a Radeon RX 9060 XT, comparing the encoded result against
the compositor's own readback of the same frames:
| ground truth = `51` | before | after |
|---|---|---|
| flat background, decoded | **`38`** | `49–51` |
| mean luma, capture path vs readback | **10.41 apart** | **0.55 apart**
|
| luma histogram intersection | **0.090** | **0.913** |
| declared `color_range` | `tv` | `pc` |
**The encoded luma is byte-identical before and after** — `Y = 51.00`,
standard deviation `0.00` on both runs. Only the tag changed, which is
what identifies this as a signalling bug rather than a conversion one,
and why nothing short of a comparison against ground truth could see it:
the stream was valid, the frame rate was right, the picture was
recognisable, and every liveness check passed.
The HDR arm (`bt2020_pq`) carried the same defect and is fixed the same
way, but **has not been run** — no 10-bit verification here.
## `scripts/verify-chain.sh`
Runs a Vulkan workload under `nescope` with the layer active and
compares the encoded output against `nescope-shot`'s readback of the
same frames. Two paths that share almost no code see the same content,
so disagreement localises the fault; a single path cannot tell a correct
frame from a plausible-looking wrong one.
**Confirmed it fails when this change is reverted** — both the tag check
and the brightness-agreement check fire.
One note on its thresholds, since it is easy to get backwards: the
not-blank check is a low absolute floor plus a comparison against the
readback's own structure, rather than a fixed number. A fixed number was
tried first and was wrong in the worst way — the **broken** build scored
20.49 on it and the **fixed** build 17.74, because the range defect
stretched contrast and that reads as more detail. How much structure a
correct frame carries depends on what the workload drew, so the only
stable reference is ground truth measured in the same run.
## Not covered
`vkcube` rather than a real workload; 720p, H.264, 8-bit; one card, one
driver. XWayland, HUD detection and real swapchain formats are
untouched.
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
The PR aligns encoded-stream color metadata with the full-range samples
produced by nescapture and updates the CPU fallback to BT.709 full-range
conversion.
- Updates pixelforge and configures matching converter color space,
range, and SDR reference white.
- Corrects Vulkan color-space mapping and adds regression tests for SDR,
HDR, and CPU fallback behavior.
- Adds SDR capture-chain and HDR comparison verification scripts.
<h3>Confidence Score: 5/5</h3>
The PR appears safe to merge.
No blocking failure remains.
<h3>Important Files Changed</h3>
| Filename | Overview |
|----------|----------|
| apps/nescapture/src/encode.rs | Aligns GPU and CPU conversion output
with encoded color metadata and adds focused regression coverage. |
| apps/nescapture/scripts/verify-chain.sh | Adds an end-to-end SDR
verifier using a static corner patch to avoid the previously reported
temporal mismatch. |
| apps/nescapture/scripts/verify-hdr.sh | Adds an HDR comparison harness
for inspecting conversion behavior across builds. |
| apps/nescapture/Cargo.toml | Advances pixelforge to the revision
providing the required color-conversion configuration. |
| Cargo.lock | Records the pixelforge update and resulting transitive
dependency refresh. |
<sub>Reviews (5): Last reviewed commit: ["test(nescapture): check the
HDR
conversi..."](2f9773c4b7)
| [Re-trigger
Greptile](https://app.greptile.com/api/retrigger?id=60147076)</sub>
**Context used:**
- Knowledge Base — [Vulkan capture
layer](https://app.greptile.com/nestri/-/custom-context/knowledge-base/nestrilabs/nestri/-/docs/capture-layer.md)
<!-- /greptile_comment -->
---------
Co-authored-by: DatCaptainHorse <DatCaptainHorse@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -32,7 +32,7 @@ toml = "0.8"
|
||||
serde = { version = "1", features = ["derive"] }
|
||||
|
||||
# Vulkan Video hardware encoding.
|
||||
pixelforge = { git = "https://github.com/hgaiser/pixelforge.git", rev = "85c8654d383e194b984e27b319de96e4335e2b4f", features = ["dmabuf"] }
|
||||
pixelforge = { git = "https://github.com/hgaiser/pixelforge.git", rev = "2ee4d7ac6e470cc34270e48d6e9d6f3fc1d7c379", features = ["dmabuf"] }
|
||||
|
||||
# libc for DMA-BUF OS primitives
|
||||
libc = "0.2"
|
||||
|
||||
186
apps/nescapture/scripts/verify-chain.sh
Executable file
186
apps/nescapture/scripts/verify-chain.sh
Executable file
@@ -0,0 +1,186 @@
|
||||
#!/usr/bin/env bash
|
||||
# Verify the capture chain end to end on this machine's GPU.
|
||||
#
|
||||
# Runs a Vulkan workload under the compositor with the layer active, then checks
|
||||
# the encoded result against the compositor's own readback of the same frames.
|
||||
# Two independent paths see the same content: the compositor reads the surface
|
||||
# back to the CPU, the layer exports it as a DMA-BUF and encodes it on the GPU.
|
||||
# Agreement between them is the evidence; a single path cannot tell a correct
|
||||
# frame from a plausible-looking wrong one.
|
||||
#
|
||||
# The failure this is really aimed at is silent: a black or mis-levelled frame
|
||||
# arrives as a valid stream at the right frame rate, and every liveness check
|
||||
# passes. So the checks below are about pixel values, not about whether bytes
|
||||
# moved.
|
||||
#
|
||||
# This covers the SDR path only. The HDR arms need a swapchain this workload
|
||||
# cannot ask for, and the check that matters there is a different one — absolute
|
||||
# sample values against the standard, rather than two instruments against each
|
||||
# other. See verify-hdr.sh.
|
||||
#
|
||||
# Usage: apps/nescapture/scripts/verify-chain.sh [seconds]
|
||||
set -euo pipefail
|
||||
|
||||
SECS="${1:-16}"
|
||||
ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../../.." && pwd)"
|
||||
WORK="$(mktemp -d)"
|
||||
trap 'rm -rf "$WORK"; kill $(jobs -p) 2>/dev/null || true' EXIT
|
||||
|
||||
: "${XDG_RUNTIME_DIR:=/run/user/$(id -u)}"
|
||||
export XDG_RUNTIME_DIR
|
||||
|
||||
for tool in ffmpeg ffprobe vkcube python3; do
|
||||
command -v "$tool" >/dev/null || { echo "missing required tool: $tool" >&2; exit 1; }
|
||||
done
|
||||
|
||||
echo "building…"
|
||||
cargo build --release -p nescope -p nescapture --manifest-path "$ROOT/Cargo.toml" >/dev/null
|
||||
|
||||
LAYER="$ROOT/target/release/libnescapture_layer.so"
|
||||
MANIFEST_DIR="$WORK/implicit_layer.d"
|
||||
mkdir -p "$MANIFEST_DIR"
|
||||
sed "s#\"library_path\": \".*\"#\"library_path\": \"$LAYER\"#" \
|
||||
"$ROOT/apps/nescapture/manifest/VK_LAYER_nescapture.json" > "$MANIFEST_DIR/VK_LAYER_nescapture.json"
|
||||
export VK_ADD_IMPLICIT_LAYER_PATH="$MANIFEST_DIR"
|
||||
|
||||
VIDEO_SOCK="$WORK/video.sock"
|
||||
SHOT_SOCK="$WORK/shot.sock"
|
||||
STREAM="$WORK/capture.h264"
|
||||
|
||||
cat > "$WORK/recv.py" <<'PY'
|
||||
import os, socket, struct, sys, time
|
||||
sock, out, secs = sys.argv[1], sys.argv[2], float(sys.argv[3])
|
||||
s = socket.socket(socket.AF_UNIX, socket.SOCK_DGRAM)
|
||||
s.setsockopt(socket.SOL_SOCKET, socket.SO_RCVBUF, 8 << 20)
|
||||
s.bind(sock); os.chmod(sock, 0o777); s.settimeout(1.0)
|
||||
n = 0; end = time.time() + secs
|
||||
with open(out, "wb") as f:
|
||||
while time.time() < end:
|
||||
try: buf = s.recv(8 << 20)
|
||||
except socket.timeout: continue
|
||||
if len(buf) < 20 or buf[:4] != b"NSTR" or buf[4] != 0: continue
|
||||
(_, _, _, dl) = struct.unpack("<IHHI", buf[8:20])
|
||||
f.write(buf[20:20 + dl]); n += 1
|
||||
print(n)
|
||||
PY
|
||||
|
||||
echo "capturing for ${SECS}s…"
|
||||
"$ROOT/target/release/nescope-shot" --socket "$SHOT_SOCK" --watch --interval 1000 \
|
||||
--keep 3 --out "$WORK/shot.ppm" >/dev/null 2>&1 &
|
||||
python3 "$WORK/recv.py" "$VIDEO_SOCK" "$STREAM" "$SECS" > "$WORK/frames.txt" &
|
||||
RECV=$!
|
||||
sleep 1
|
||||
|
||||
NESCAPTURE_ENABLE=1 NESCAPTURE_CODEC=h264 NESCAPTURE_BITRATE=20000 NESCAPTURE_FPS=60 \
|
||||
NESCAPTURE_IPC_PATH="$VIDEO_SOCK" RUST_LOG=nescapture_layer=debug \
|
||||
timeout "$((SECS - 2))" "$ROOT/target/release/nescope" \
|
||||
--width 1280 --height 720 --fps 60 --screenshot-ipc "$SHOT_SOCK" \
|
||||
-- vkcube --c 100000 > "$WORK/run.log" 2>&1 || true
|
||||
wait $RECV || true
|
||||
|
||||
FRAMES="$(cat "$WORK/frames.txt")"
|
||||
echo
|
||||
echo "frames encoded: $FRAMES"
|
||||
grep -m1 "First import" "$WORK/run.log" || echo " (no DMA-BUF import logged)"
|
||||
|
||||
python3 - "$WORK" "$STREAM" "$FRAMES" <<'PY'
|
||||
import glob, subprocess, sys
|
||||
import numpy as np
|
||||
from PIL import Image
|
||||
|
||||
work, stream, frames = sys.argv[1], sys.argv[2], int(sys.argv[3])
|
||||
fails = []
|
||||
|
||||
if frames < 30:
|
||||
fails.append(f"only {frames} frames encoded (want >= 30)")
|
||||
|
||||
probe = subprocess.run(
|
||||
["ffprobe", "-v", "error", "-select_streams", "v:0", "-show_entries",
|
||||
"stream=color_range", "-of", "default=noprint_wrappers=1:nokey=1", stream],
|
||||
capture_output=True, text=True).stdout.strip()
|
||||
print(f"declared range: {probe or '(none)'}")
|
||||
if probe != "pc":
|
||||
fails.append(f"stream declares color_range={probe or 'unset'}; the converter "
|
||||
"writes full-range samples, so the tag must be 'pc'")
|
||||
|
||||
subprocess.run(["ffmpeg", "-v", "error", "-y", "-i", stream, "-vf",
|
||||
r"select='eq(n\,120)+eq(n\,240)'", "-fps_mode", "passthrough",
|
||||
f"{work}/dec_%02d.png"], check=True)
|
||||
|
||||
def luma(a):
|
||||
return 0.2126 * a[..., 0] + 0.7152 * a[..., 1] + 0.0722 * a[..., 2]
|
||||
|
||||
dec = [np.asarray(Image.open(p).convert("RGB")).astype(float)
|
||||
for p in sorted(glob.glob(f"{work}/dec_*.png"))]
|
||||
shots = [np.asarray(Image.open(p).convert("RGB")).astype(float)
|
||||
for p in sorted(glob.glob(f"{work}/shot-*.ppm"))]
|
||||
|
||||
if not dec:
|
||||
fails.append("nothing decoded from the stream")
|
||||
if not shots:
|
||||
fails.append("compositor readback produced no frames")
|
||||
|
||||
if dec:
|
||||
worst = min(luma(d).std() for d in dec)
|
||||
print(f"decoded luma std: {worst:.2f}")
|
||||
# An absolute floor only has to catch a blank frame, which sits near zero.
|
||||
# How much structure a *correct* frame carries depends entirely on what the
|
||||
# workload drew, so the real check is the relative one below, against the
|
||||
# readback of the same content.
|
||||
if worst < 5.0:
|
||||
fails.append(f"decoded frames are near-uniform (luma std {worst:.2f}) — "
|
||||
"the classic silent failure is a blank frame at full frame rate")
|
||||
|
||||
if dec and shots:
|
||||
ds, ss = min(luma(d).std() for d in dec), min(luma(s).std() for s in shots)
|
||||
print(f"readback luma std:{ss:.2f}")
|
||||
if ss > 1.0 and abs(ds - ss) / ss > 0.25:
|
||||
fails.append(f"decoded structure {ds:.2f} vs readback {ss:.2f} — the two "
|
||||
"paths saw the same frames, so they should carry the same detail")
|
||||
|
||||
if dec and shots:
|
||||
# Compare a corner, not the whole frame. The two instruments sample at
|
||||
# different moments -- the readback is on a 1 s timer, the decoded frames are
|
||||
# picked by index -- so any whole-frame statistic also carries whatever the
|
||||
# workload was doing at each instant. The workload draws a centred object on
|
||||
# a flat background, so a corner patch is the same colour in every frame and
|
||||
# the comparison stops depending on lining them up.
|
||||
#
|
||||
# This is the measurement that catches a range or matrix error: a flat patch
|
||||
# of known colour, decoded, against the same patch read back from the
|
||||
# compositor. It is where a full-range/limited-range mismatch shows up as a
|
||||
# constant offset.
|
||||
def corner(a):
|
||||
return a[8:72, 8:72]
|
||||
|
||||
def spread(patches):
|
||||
m = [luma(p).mean() for p in patches]
|
||||
return max(m) - min(m)
|
||||
|
||||
dc, sc = [corner(d) for d in dec], [corner(s) for s in shots]
|
||||
a = float(np.mean([luma(p).mean() for p in sc]))
|
||||
b = float(np.mean([luma(p).mean() for p in dc]))
|
||||
print(f"readback corner: {a:.2f}")
|
||||
print(f"decoded corner: {b:.2f}")
|
||||
print(f"difference: {abs(a - b):.2f}")
|
||||
|
||||
# If the corner is not actually flat across frames, the assumption above does
|
||||
# not hold for this workload and the comparison would be measuring animation.
|
||||
# Say so rather than reporting a number that means nothing.
|
||||
drift = max(spread(dc), spread(sc))
|
||||
if drift > 3.0:
|
||||
fails.append(f"the corner patch varies by {drift:.2f} between frames, so it "
|
||||
"is not background here; the brightness check assumes a "
|
||||
"workload that leaves its corners alone")
|
||||
elif abs(a - b) > 4.0:
|
||||
fails.append(f"the two paths disagree on brightness by {abs(a-b):.2f}; "
|
||||
"they are looking at the same content, so one of them is wrong")
|
||||
|
||||
print()
|
||||
if fails:
|
||||
print("FAIL")
|
||||
for f in fails:
|
||||
print(f" - {f}")
|
||||
sys.exit(1)
|
||||
print("PASS")
|
||||
PY
|
||||
173
apps/nescapture/scripts/verify-hdr.sh
Executable file
173
apps/nescapture/scripts/verify-hdr.sh
Executable file
@@ -0,0 +1,173 @@
|
||||
#!/usr/bin/env bash
|
||||
# Verify that the HDR path *converts*, not merely that it says it did.
|
||||
#
|
||||
# The defect this exists to catch: the colour space reached the encoder's
|
||||
# configuration and its VUI, but not the conversion shader. The stream then
|
||||
# declares BT.2020 NCL while the samples under it were written with the BT.709
|
||||
# matrix, and nothing downstream can tell.
|
||||
#
|
||||
# Two rules follow from that, and they are the whole design:
|
||||
#
|
||||
# 1. Compare raw Y/U/V samples, never a decode to RGB. If the shader and the
|
||||
# VUI agree on the *wrong* matrix, an RGB round trip inverts exactly what
|
||||
# it applied and returns the original colour. It scores the broken build
|
||||
# perfect.
|
||||
# 2. Use a saturated colour, never grey. Achromatic input gives identical
|
||||
# results under every matrix here, so a grey patch cannot see this defect
|
||||
# at any tolerance.
|
||||
#
|
||||
# `ffprobe` output is byte-identical between a correct and a broken build --
|
||||
# it reads the declaration, which is the half that was already right.
|
||||
#
|
||||
# Needs a probe that can drive an HDR swapchain on purpose. It is not vendored:
|
||||
# it pulls winit and ash, which is a lot of build for a test fixture. Point
|
||||
# HDRPROBE at one that accepts `--color R,G,B`, `--width`, `--height`, `--sdr`.
|
||||
#
|
||||
# Usage: HDRPROBE=/path/to/hdrprobe apps/nescapture/scripts/verify-hdr.sh
|
||||
set -euo pipefail
|
||||
|
||||
ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../../.." && pwd)"
|
||||
WORK="$(mktemp -d /tmp/nshdr.XXXXXX)" # short path: an AF_UNIX socket has ~108 bytes
|
||||
trap 'rm -rf "$WORK"; kill $(jobs -p) 2>/dev/null || true' EXIT
|
||||
|
||||
: "${XDG_RUNTIME_DIR:=/run/user/$(id -u)}"
|
||||
export XDG_RUNTIME_DIR
|
||||
: "${SECS:=12}"
|
||||
W=1920; H=1080
|
||||
|
||||
if [ -z "${HDRPROBE:-}" ] || [ ! -x "${HDRPROBE:-}" ]; then
|
||||
echo "set HDRPROBE to a probe that can present a known colour in a chosen colour space" >&2
|
||||
exit 2
|
||||
fi
|
||||
for t in ffmpeg python3; do
|
||||
command -v "$t" >/dev/null || { echo "missing required tool: $t" >&2; exit 1; }
|
||||
done
|
||||
python3 -c "import numpy" 2>/dev/null || { echo "missing python numpy" >&2; exit 1; }
|
||||
|
||||
echo "building…"
|
||||
cargo build --release -p nescope -p nescapture --manifest-path "$ROOT/Cargo.toml" >/dev/null
|
||||
|
||||
# Point the loader at this build. A stale layer installed system-wide otherwise
|
||||
# wins the lookup and the run silently measures whatever is in /usr/lib.
|
||||
mkdir -p "$WORK/lay"
|
||||
sed "s#\"library_path\": \".*\"#\"library_path\": \"$ROOT/target/release/libnescapture_layer.so\"#" \
|
||||
"$ROOT/apps/nescapture/manifest/VK_LAYER_nescapture.json" > "$WORK/lay/VK_LAYER_nescapture.json"
|
||||
export VK_ADD_IMPLICIT_LAYER_PATH="$WORK/lay"
|
||||
|
||||
run_case() {
|
||||
local tag=$1 color=$2 mode=$3
|
||||
local sock="$WORK/$tag.sock" out="$WORK/$tag.h264"
|
||||
local hdrflag="" probeflag=""
|
||||
[ "$mode" = "hdr" ] && hdrflag="--hdr" || probeflag="--sdr"
|
||||
|
||||
python3 - "$sock" "$out" "$SECS" > "$WORK/$tag.frames" <<'PY' &
|
||||
import os, socket, struct, sys, time
|
||||
sock, out, secs = sys.argv[1], sys.argv[2], float(sys.argv[3])
|
||||
s = socket.socket(socket.AF_UNIX, socket.SOCK_DGRAM)
|
||||
s.setsockopt(socket.SOL_SOCKET, socket.SO_RCVBUF, 8 << 20)
|
||||
s.bind(sock); os.chmod(sock, 0o777); s.settimeout(1.0)
|
||||
n = 0; end = time.time() + secs
|
||||
with open(out, "wb") as f:
|
||||
while time.time() < end:
|
||||
try: b = s.recv(8 << 20)
|
||||
except socket.timeout: continue
|
||||
if len(b) < 20 or b[:4] != b"NSTR" or b[4] != 0: continue
|
||||
f.write(b[20:20 + struct.unpack("<I", b[16:20])[0]]); n += 1
|
||||
print(n)
|
||||
PY
|
||||
local recv=$!
|
||||
sleep 1
|
||||
NESCAPTURE_ENABLE=1 NESCAPTURE_CODEC=h264 NESCAPTURE_BITRATE=20000 NESCAPTURE_FPS=60 \
|
||||
NESCAPTURE_IPC_PATH="$sock" RUST_LOG=nescapture_layer=info \
|
||||
timeout "$((SECS - 2))" "$ROOT/target/release/nescope" \
|
||||
--width $W --height $H --fps 60 $hdrflag \
|
||||
-- "$HDRPROBE" --width $W --height $H --color "$color" $probeflag --frames 100000 \
|
||||
> "$WORK/$tag.log" 2>&1 || true
|
||||
wait $recv || true
|
||||
|
||||
# Decode to raw planes in the format the stream already is, so ffmpeg inserts
|
||||
# no scaler and applies no matrix. What is compared is what the shader wrote.
|
||||
ffmpeg -v error -y -i "$out" -pix_fmt yuv420p -f rawvideo "$WORK/$tag.yuv" 2>/dev/null || true
|
||||
printf " %-10s %s frames, %s\n" "$tag" "$(cat "$WORK/$tag.frames")" \
|
||||
"$(grep -m1 -o 'CHOSEN: .*' "$WORK/$tag.log" || echo 'no format line')"
|
||||
}
|
||||
|
||||
echo "running ${SECS}s per case…"
|
||||
run_case red_hdr 255,0,0 hdr
|
||||
run_case green_hdr 0,255,0 hdr
|
||||
run_case red_sdr 255,0,0 sdr
|
||||
|
||||
python3 - "$WORK" "$W" "$H" <<'PY'
|
||||
import os, sys
|
||||
import numpy as np
|
||||
|
||||
work, W, H = sys.argv[1], int(sys.argv[2]), int(sys.argv[3])
|
||||
FRAME = W * H * 3 // 2
|
||||
|
||||
# Luma coefficients are the published ones (ITU-R BT.709-6, BT.2020-2 Table 4).
|
||||
# Cb/Cr come from the standard relations rather than from the shader's own
|
||||
# constants, so agreement is a cross-check and not a restatement.
|
||||
KR_KB = {"bt709": (0.2126, 0.0722), "bt2020": (0.2627, 0.0593)}
|
||||
|
||||
def expect(rgb, matrix):
|
||||
kr, kb = KR_KB[matrix]
|
||||
r, g, b = (c / 255.0 for c in rgb)
|
||||
y = kr * r + (1.0 - kr - kb) * g + kb * b
|
||||
q = lambda x: min(255.0, max(0.0, x * 255.0))
|
||||
return q(y), q((b - y) / (2 * (1 - kb)) + 0.5), q((r - y) / (2 * (1 - kr)) + 0.5)
|
||||
|
||||
def centre(a):
|
||||
h, w = a.shape
|
||||
return a[h // 2 - 100:h // 2 + 100, w // 2 - 100:w // 2 + 100]
|
||||
|
||||
def decide(tag, rgb, want):
|
||||
path = f"{work}/{tag}.yuv"
|
||||
n = os.path.getsize(path) // FRAME if os.path.exists(path) else 0
|
||||
if n < 3:
|
||||
return tag, "INCONCLUSIVE", f"only {n} decoded frames"
|
||||
ys, us, vs = [], [], []
|
||||
for i in (n - 3, n - 2, n - 1):
|
||||
buf = np.fromfile(path, dtype=np.uint8, count=FRAME, offset=i * FRAME)
|
||||
ys.append(centre(buf[:W * H].reshape(H, W).astype(float)))
|
||||
us.append(centre(buf[W * H:W * H + W * H // 4].reshape(H // 2, W // 2).astype(float)))
|
||||
vs.append(centre(buf[W * H + W * H // 4:].reshape(H // 2, W // 2).astype(float)))
|
||||
y = float(np.mean([p.mean() for p in ys]))
|
||||
std = max(p.std() for p in ys)
|
||||
u = float(np.mean([p.mean() for p in us]))
|
||||
v = float(np.mean([p.mean() for p in vs]))
|
||||
|
||||
e709, e2020 = expect(rgb, "bt709"), expect(rgb, "bt2020")
|
||||
d709, d2020 = abs(y - e709[0]), abs(y - e2020[0])
|
||||
print(f" {tag:10} Y={y:7.2f} U={u:6.2f} V={v:6.2f} (std {std:.2f})")
|
||||
print(f" {'':10} BT.709 Y={e709[0]:6.2f} off {d709:5.2f} | "
|
||||
f"BT.2020 Y={e2020[0]:6.2f} off {d2020:5.2f}")
|
||||
|
||||
# The shader quantises with uint(), which truncates rather than rounds, so a
|
||||
# correct sample sits up to one code low. The two hypotheses are ~13 codes
|
||||
# apart, so a 2-code window cannot admit both.
|
||||
if std >= 2.0:
|
||||
return tag, "INCONCLUSIVE", f"centre patch not flat (std {std:.2f})"
|
||||
hit, miss = (d2020, d709) if want == "bt2020" else (d709, d2020)
|
||||
other = "bt709" if want == "bt2020" else "bt2020"
|
||||
if hit < 2.0 and miss > 6.0:
|
||||
return tag, "PASS", f"converted on the {want} matrix (off {hit:.2f})"
|
||||
if miss < 2.0:
|
||||
return tag, "FAIL", f"converted with the {other} matrix (off {miss:.2f}), not {want}"
|
||||
return tag, "INCONCLUSIVE", f"near neither (off {hit:.2f} / {miss:.2f})"
|
||||
|
||||
print()
|
||||
results = [
|
||||
decide("red_hdr", (255, 0, 0), "bt2020"),
|
||||
decide("green_hdr", (0, 255, 0), "bt2020"),
|
||||
decide("red_sdr", (255, 0, 0), "bt709"),
|
||||
]
|
||||
print("-" * 60)
|
||||
for tag, verdict, why in results:
|
||||
print(f" {verdict:13} {tag:10} {why}")
|
||||
print()
|
||||
if all(v == "PASS" for _, v, _ in results):
|
||||
print("PASS")
|
||||
sys.exit(0)
|
||||
print("FAIL")
|
||||
sys.exit(1)
|
||||
PY
|
||||
@@ -49,13 +49,24 @@ use pixelforge::{
|
||||
|
||||
use crate::dmabuf_import::{DmaBufImporter, DmaBufPlane};
|
||||
|
||||
// ── VkColorSpaceKHR constants (raw values, matches ash/Vulkan spec) ───────────
|
||||
const VK_COLOR_SPACE_SRGB_NONLINEAR_KHR: u32 = 0;
|
||||
const VK_COLOR_SPACE_HDR10_ST2084_EXT: u32 = 1_000_104_002;
|
||||
const VK_COLOR_SPACE_EXTENDED_SRGB_LINEAR_EXT: u32 = 1_000_104_003;
|
||||
const VK_COLOR_SPACE_BT2020_LINEAR_EXT: u32 = 1_000_104_007;
|
||||
const VK_COLOR_SPACE_DOLBYVISION_EXT: u32 = 1_000_104_009;
|
||||
const VK_COLOR_SPACE_HDR10_HLG_EXT: u32 = 1_000_104_010;
|
||||
// ── VkColorSpaceKHR constants ────────────────────────────────────────────────
|
||||
//
|
||||
// Taken from `ash` rather than written out. They were transcribed by hand once
|
||||
// and two of them were wrong: HDR10 ST2084 was given the value of extended-sRGB
|
||||
// linear, and extended-sRGB linear the value of Display-P3 linear. Both are HDR
|
||||
// entry points, so every HDR swapchain fell through to the SDR arm and was
|
||||
// converted and tagged BT.709 — a silent, total loss of the colour volume the
|
||||
// workload asked for. Deriving them here means a wrong value cannot be written.
|
||||
const fn colorspace(c: ash::vk::ColorSpaceKHR) -> u32 {
|
||||
c.as_raw() as u32
|
||||
}
|
||||
const VK_COLOR_SPACE_SRGB_NONLINEAR_KHR: u32 = colorspace(ash::vk::ColorSpaceKHR::SRGB_NONLINEAR);
|
||||
const VK_COLOR_SPACE_HDR10_ST2084_EXT: u32 = colorspace(ash::vk::ColorSpaceKHR::HDR10_ST2084_EXT);
|
||||
const VK_COLOR_SPACE_EXTENDED_SRGB_LINEAR_EXT: u32 =
|
||||
colorspace(ash::vk::ColorSpaceKHR::EXTENDED_SRGB_LINEAR_EXT);
|
||||
const VK_COLOR_SPACE_BT2020_LINEAR_EXT: u32 = colorspace(ash::vk::ColorSpaceKHR::BT2020_LINEAR_EXT);
|
||||
const VK_COLOR_SPACE_DOLBYVISION_EXT: u32 = colorspace(ash::vk::ColorSpaceKHR::DOLBYVISION_EXT);
|
||||
const VK_COLOR_SPACE_HDR10_HLG_EXT: u32 = colorspace(ash::vk::ColorSpaceKHR::HDR10_HLG_EXT);
|
||||
|
||||
pub fn vk_format_to_input_format(vk_format: u32) -> Option<InputFormat> {
|
||||
match vk_format {
|
||||
@@ -76,14 +87,33 @@ pub fn vk_colorspace_to_color_space(vk_colorspace: u32) -> ColorSpace {
|
||||
| VK_COLOR_SPACE_DOLBYVISION_EXT
|
||||
| VK_COLOR_SPACE_HDR10_HLG_EXT => ColorSpace::Bt2020,
|
||||
|
||||
// Both are linear, so the inverse sRGB EOTF that `SrgbToBt2020Pq` applies
|
||||
// would decode data that was never encoded. `Bt709LinearToBt2020Pq` is
|
||||
// documented for `EXTENDED_SRGB_LINEAR_EXT` exactly. `BT2020_LINEAR_EXT`
|
||||
// is linear on BT.2020 primaries and there is no arm for that yet, so it
|
||||
// borrows this one and takes a gamut error rather than a gamma one.
|
||||
VK_COLOR_SPACE_EXTENDED_SRGB_LINEAR_EXT | VK_COLOR_SPACE_BT2020_LINEAR_EXT => {
|
||||
ColorSpace::SrgbToBt2020Pq
|
||||
ColorSpace::Bt709LinearToBt2020Pq
|
||||
}
|
||||
|
||||
_ => ColorSpace::Bt709,
|
||||
}
|
||||
}
|
||||
|
||||
/// SDR reference white for the PQ conversions, in nits.
|
||||
///
|
||||
/// Only the two arms that write PQ consume this. They disagree on what 1.0 means:
|
||||
/// sRGB content is gamma-encoded and its white sits at the BT.2408 reference of
|
||||
/// 203 nits, while scRGB is linear and IEC 61966-2-2 puts 1.0 at 80 nits.
|
||||
/// pixelforge defaults to 203 for both, which maps scRGB white about 2.5x too
|
||||
/// bright.
|
||||
pub fn sdr_reference_white_nits(color_space: ColorSpace) -> f32 {
|
||||
match color_space {
|
||||
ColorSpace::Bt709LinearToBt2020Pq => 80.0,
|
||||
_ => 203.0,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn vk_format_to_bit_depth(vk_format: u32) -> EncodeBitDepth {
|
||||
match vk_format {
|
||||
64 | 58 | 97 => EncodeBitDepth::Ten,
|
||||
@@ -92,12 +122,25 @@ pub fn vk_format_to_bit_depth(vk_format: u32) -> EncodeBitDepth {
|
||||
}
|
||||
|
||||
pub fn vk_colorspace_to_color_description(vk_colorspace: u32) -> Option<ColorDescription> {
|
||||
match vk_colorspace {
|
||||
VK_COLOR_SPACE_HDR10_ST2084_EXT
|
||||
| VK_COLOR_SPACE_DOLBYVISION_EXT
|
||||
| VK_COLOR_SPACE_HDR10_HLG_EXT => Some(ColorDescription::bt2020_pq()),
|
||||
_ => Some(ColorDescription::bt709()),
|
||||
}
|
||||
// Derived from the conversion rather than matched separately: the VUI has to
|
||||
// describe what the shader actually wrote, and two independent matches on the
|
||||
// same input drift the moment one gains an arm the other doesn't.
|
||||
//
|
||||
// Full range on every arm, because the converter is configured full-range
|
||||
// unconditionally. pixelforge's constructors are limited-range per
|
||||
// ITU-R BT.709-6, so leaving the flag off tags full-range luma as limited and
|
||||
// every compliant decoder expands it again — darkening midtones and clipping
|
||||
// both ends.
|
||||
let desc = match vk_colorspace_to_color_space(vk_colorspace) {
|
||||
ColorSpace::Bt709 => ColorDescription::bt709(),
|
||||
// BT.2020 passthrough carries PQ-encoded input; the other two write PQ.
|
||||
// HLG swapchains are tagged PQ here because pixelforge has no HLG transfer
|
||||
// constant — a pre-existing approximation, not a consequence of this.
|
||||
ColorSpace::Bt2020 | ColorSpace::SrgbToBt2020Pq | ColorSpace::Bt709LinearToBt2020Pq => {
|
||||
ColorDescription::bt2020_pq()
|
||||
}
|
||||
};
|
||||
Some(desc.with_full_range(true))
|
||||
}
|
||||
|
||||
pub fn output_format(pixel_fmt: PixelFormat, bit_depth: EncodeBitDepth) -> OutputFormat {
|
||||
@@ -773,7 +816,8 @@ impl PerFrameEncoder {
|
||||
} else {
|
||||
// GPU framebuffer captures are always full-range — use BT.709 full-range
|
||||
// so the decoder doesn't apply limited‑range expansion.
|
||||
enc_cfg = enc_cfg.with_color_description(ColorDescription::bt709());
|
||||
enc_cfg =
|
||||
enc_cfg.with_color_description(ColorDescription::bt709().with_full_range(true));
|
||||
}
|
||||
enc_cfg = if let Some(q) = qp {
|
||||
enc_cfg
|
||||
@@ -794,14 +838,20 @@ impl PerFrameEncoder {
|
||||
let encoder =
|
||||
Encoder::new(ctx.clone(), enc_cfg).map_err(|e| format!("Encoder::new: {e}"))?;
|
||||
|
||||
let conv_cfg =
|
||||
ColorConverterConfig::new(width, height, /*color_space,*/ input_fmt, out_fmt);
|
||||
let mut conv_cfg = ColorConverterConfig::new(width, height, input_fmt, out_fmt);
|
||||
// The matrix the shader applies has to be the one the VUI declares. The
|
||||
// colour space was previously computed, logged and then dropped on the
|
||||
// floor, so BT.2020 captures were converted with the BT.709 matrix and
|
||||
// the scRGB→PQ arm never ran at all.
|
||||
conv_cfg.color_space = color_space;
|
||||
conv_cfg.sdr_reference_white_nits = sdr_reference_white_nits(color_space);
|
||||
// Capture is always full-range; `vk_colorspace_to_color_description` tags
|
||||
// the stream to match.
|
||||
conv_cfg.full_range = true;
|
||||
|
||||
let mut converter = ColorConverter::new(ctx.clone(), conv_cfg)
|
||||
let converter = ColorConverter::new(ctx.clone(), conv_cfg)
|
||||
.map_err(|e| format!("ColorConverter::new: {e}"))?;
|
||||
|
||||
converter.set_full_range(true);
|
||||
|
||||
Ok(Self {
|
||||
encoder,
|
||||
converter,
|
||||
@@ -906,15 +956,24 @@ fn bgra_to_yuv420(pixels: &[u8], width: u32, height: u32, vk_format: u32) -> Vec
|
||||
break;
|
||||
}
|
||||
let (r, g, b) = if bgra {
|
||||
(pixels[i + 2] as i32, pixels[i + 1] as i32, pixels[i] as i32)
|
||||
(pixels[i + 2] as f32, pixels[i + 1] as f32, pixels[i] as f32)
|
||||
} else {
|
||||
(pixels[i] as i32, pixels[i + 1] as i32, pixels[i + 2] as i32)
|
||||
(pixels[i] as f32, pixels[i + 1] as f32, pixels[i + 2] as f32)
|
||||
};
|
||||
y[row * w + col] = (((66 * r + 129 * g + 25 * b + 128) >> 8) + 16).clamp(16, 235) as u8;
|
||||
// BT.709, full range — the same thing the GPU converter is configured
|
||||
// to produce and the same thing the stream's colour description
|
||||
// declares. This used to be BT.601 limited range, which disagreed with
|
||||
// the declaration on both counts: a decoder expanded 16-235 that was
|
||||
// never compressed, using the wrong matrix to do it. The fallback is
|
||||
// rare enough that nobody would have noticed it looking different
|
||||
// from the GPU path.
|
||||
let yf = 0.2126 * r + 0.7152 * g + 0.0722 * b;
|
||||
y[row * w + col] = yf.round().clamp(0.0, 255.0) as u8;
|
||||
if row % 2 == 0 && col % 2 == 0 {
|
||||
let ci = (row / 2) * (w / 2) + col / 2;
|
||||
u[ci] = (((-38 * r - 74 * g + 112 * b + 128) >> 8) + 128).clamp(16, 240) as u8;
|
||||
v[ci] = (((112 * r - 94 * g - 18 * b + 128) >> 8) + 128).clamp(16, 240) as u8;
|
||||
// Cb, Cr from the same primaries: (B-Y)/(2(1-Kb)), (R-Y)/(2(1-Kr)).
|
||||
u[ci] = ((b - yf) / 1.8556 + 128.0).round().clamp(0.0, 255.0) as u8;
|
||||
v[ci] = ((r - yf) / 1.5748 + 128.0).round().clamp(0.0, 255.0) as u8;
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1101,3 +1160,153 @@ fn stats_sender_thread(
|
||||
|
||||
log::info!("stats sender exited");
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use ash::vk::ColorSpaceKHR as Cs;
|
||||
|
||||
/// The colour space values were once written out by hand and two were wrong,
|
||||
/// which routed every HDR swapchain into the SDR arm silently. Deriving them
|
||||
/// from `ash` is the fix; this pins the behaviour that depended on them.
|
||||
#[test]
|
||||
fn hdr_colour_spaces_select_the_hdr_arm() {
|
||||
for cs in [Cs::HDR10_ST2084_EXT, Cs::DOLBYVISION_EXT, Cs::HDR10_HLG_EXT] {
|
||||
let raw = cs.as_raw() as u32;
|
||||
assert_eq!(
|
||||
vk_colorspace_to_color_space(raw),
|
||||
ColorSpace::Bt2020,
|
||||
"{cs:?} must convert as BT.2020, not BT.709"
|
||||
);
|
||||
let desc = vk_colorspace_to_color_description(raw).expect("a description");
|
||||
assert_eq!(
|
||||
desc,
|
||||
ColorDescription::bt2020_pq().with_full_range(true),
|
||||
"{cs:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// Linear swapchains must not be run through an inverse sRGB EOTF on the way
|
||||
/// to PQ; `Bt709LinearToBt2020Pq` is the arm that skips it.
|
||||
#[test]
|
||||
fn scrgb_and_bt2020_linear_select_the_pq_conversion() {
|
||||
for cs in [Cs::EXTENDED_SRGB_LINEAR_EXT, Cs::BT2020_LINEAR_EXT] {
|
||||
assert_eq!(
|
||||
vk_colorspace_to_color_space(cs.as_raw() as u32),
|
||||
ColorSpace::Bt709LinearToBt2020Pq,
|
||||
"{cs:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// scRGB is linear with 1.0 at 80 nits; everything else that reaches PQ is
|
||||
/// gamma-encoded sRGB with white at the BT.2408 reference of 203.
|
||||
#[test]
|
||||
fn scrgb_white_is_not_the_srgb_reference() {
|
||||
assert_eq!(
|
||||
sdr_reference_white_nits(ColorSpace::Bt709LinearToBt2020Pq),
|
||||
80.0
|
||||
);
|
||||
assert_eq!(sdr_reference_white_nits(ColorSpace::SrgbToBt2020Pq), 203.0);
|
||||
for cs in [Cs::EXTENDED_SRGB_LINEAR_EXT, Cs::BT2020_LINEAR_EXT] {
|
||||
let nits = sdr_reference_white_nits(vk_colorspace_to_color_space(cs.as_raw() as u32));
|
||||
assert_eq!(nits, 80.0, "{cs:?}");
|
||||
}
|
||||
}
|
||||
|
||||
/// The matrix and transfer the shader applies and the ones the VUI declares
|
||||
/// come from the same match, so they cannot disagree.
|
||||
#[test]
|
||||
fn conversion_and_declaration_agree() {
|
||||
for cs in [
|
||||
Cs::SRGB_NONLINEAR,
|
||||
Cs::PASS_THROUGH_EXT,
|
||||
Cs::HDR10_ST2084_EXT,
|
||||
Cs::DOLBYVISION_EXT,
|
||||
Cs::HDR10_HLG_EXT,
|
||||
Cs::EXTENDED_SRGB_LINEAR_EXT,
|
||||
Cs::BT2020_LINEAR_EXT,
|
||||
] {
|
||||
let raw = cs.as_raw() as u32;
|
||||
let desc = vk_colorspace_to_color_description(raw).expect("a description");
|
||||
let writes_bt2020 = vk_colorspace_to_color_space(raw) != ColorSpace::Bt709;
|
||||
assert_eq!(desc.is_hdr(), writes_bt2020, "{cs:?}");
|
||||
assert_eq!(
|
||||
desc,
|
||||
if writes_bt2020 {
|
||||
ColorDescription::bt2020_pq().with_full_range(true)
|
||||
} else {
|
||||
ColorDescription::bt709().with_full_range(true)
|
||||
},
|
||||
"{cs:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sdr_colour_spaces_stay_on_bt709() {
|
||||
for cs in [Cs::SRGB_NONLINEAR, Cs::PASS_THROUGH_EXT] {
|
||||
assert_eq!(
|
||||
vk_colorspace_to_color_space(cs.as_raw() as u32),
|
||||
ColorSpace::Bt709,
|
||||
"{cs:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// The CPU fallback has to agree with the declaration too. A flat grey of 51
|
||||
/// is the value that exposed the GPU path's range bug on real hardware: full
|
||||
/// range encodes it as Y=51, limited range as Y=60. The stream says full.
|
||||
#[test]
|
||||
fn cpu_fallback_is_full_range_bt709() {
|
||||
// 4x2 of solid RGB(51,51,51); vk_format 44 selects the BGRA branch.
|
||||
let px = vec![51u8; 4 * 2 * 4];
|
||||
let yuv = bgra_to_yuv420(&px, 4, 2, 44);
|
||||
for (i, &y) in yuv[..8].iter().enumerate() {
|
||||
assert_eq!(
|
||||
y, 51,
|
||||
"luma[{i}] should be 51 full-range, not 60 limited-range"
|
||||
);
|
||||
}
|
||||
// Achromatic input must sit at the chroma centre.
|
||||
for &c in &yuv[8..] {
|
||||
assert!((c as i32 - 128).abs() <= 1, "grey must be neutral, got {c}");
|
||||
}
|
||||
}
|
||||
|
||||
/// Saturated primaries must not be clamped into the limited-range box.
|
||||
#[test]
|
||||
fn cpu_fallback_uses_the_full_code_range() {
|
||||
let white = vec![255u8; 4 * 2 * 4];
|
||||
assert_eq!(
|
||||
bgra_to_yuv420(&white, 4, 2, 44)[0],
|
||||
255,
|
||||
"white must reach 255"
|
||||
);
|
||||
let black = vec![0u8; 4 * 2 * 4];
|
||||
assert_eq!(bgra_to_yuv420(&black, 4, 2, 44)[0], 0, "black must reach 0");
|
||||
}
|
||||
|
||||
/// The converter is configured full-range unconditionally, so every colour
|
||||
/// description handed to the encoder has to say so. A limited-range tag over
|
||||
/// full-range samples is expanded again by the decoder.
|
||||
#[test]
|
||||
fn every_colour_description_is_full_range() {
|
||||
for cs in [
|
||||
Cs::SRGB_NONLINEAR,
|
||||
Cs::HDR10_ST2084_EXT,
|
||||
Cs::DOLBYVISION_EXT,
|
||||
Cs::HDR10_HLG_EXT,
|
||||
Cs::EXTENDED_SRGB_LINEAR_EXT,
|
||||
Cs::PASS_THROUGH_EXT,
|
||||
] {
|
||||
let desc =
|
||||
vk_colorspace_to_color_description(cs.as_raw() as u32).expect("a description");
|
||||
assert!(
|
||||
desc.full_range,
|
||||
"{cs:?} produced a limited-range description"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user