fix(505): tonemap QSV HDR through OpenCL — vpp_qsv=tonemap is a silent no-op #654

Merged

timothy merged 4 commits from fix/505-qsv-native-decode-tonemap into main 2026-07-26 21:56:02 +02:00

Conversation 1 Commits 4 Files Changed 8

+501 -56

timothy commented 2026-07-26 21:17:11 +02:00

Owner

Copy Link

Copy Source

fixes #505

The finding that changed the issue

#505 asked to route the #498 native-decode path through TonemapQsvFilter to move HDR tonemapping off the CPU. Measured on the Intel host (jazz: FFmpeg 8.1.2 / iHD 25.1.4 / UHD 630) against real HDR HEVC Main10 (bt2020nc/bt2020/smpte2084), vpp_qsv=tonemap=1 is a silent no-op: a graph ending in it returns a frame byte-identical (same md5) to the same graph with no tonemap step, with no warning and no error. True with the explicit hevc_qsv decoder, on genuine QSV video-memory surfaces, before the scale, after the scale, and with format=nv12. QSV VPP tonemapping needs Gen11+; pre-Gen11 iHD ignores the option.

Same frame, luma reference: software tonemap YAVG=26.6 · no tonemap 44.3 · vpp_qsv=tonemap=1 → 44.3.

So the issue's premise was inverted, and the branch it wanted to extend was already broken: the pre-existing DecoderHardwareAccelerationMode == Qsv path shipped untonemapped HDR whenever QsvPreferNativeDecoder was off — which is exactly the escape hatch #498/#523 recommend. Prod was not affected (native-decode is the default and took the working software branch), so this is a latent bug, not a live incident.

What ships

GPU tonemapping via OpenCL, the route VaapiPipelineBuilder already uses:

format=nv12|p010le|vaapi,hwupload=derive_device=vaapi,
scale_vaapi=1280:720:force_divisible_by=2:format=p010,setsar=1,
hwmap=derive_device=opencl,tonemap_opencl=tonemap=linear:format=nv12,
hwdownload,format=nv12

• Scale must precede tonemap. Tonemapping full-size is slower than the software path it replaces (15.5s wall for 12.5s of content — below realtime). Scale-first cuts total CPU ~60% and lands at the no-tonemap wall-clock floor.
• A QSV surface is a dead end — it maps to neither OpenCL nor VA-API (hwmap → -38). So the gate requires frames still in software; the QSV decoder, deinterlace_qsv and anamorphic sources all fall back to the software tonemap: slower, but correct.
• TonemapQsvFilter is deleted. A filter that silently does nothing is worse than no filter — it looks like coverage.

Exact generated chains, old vs new, 2 reproducible rounds, 300 frames 3840x1608 → 1280x720 h264_qsv:

	user CPU	total CPU	wall
OLD software tonemap	33.9s	35.8s	10.0s
NEW OpenCL tonemap	11.1s	14.2s	8.8s

Hardware validation (the #505 Done-when box)

The exact generated command run on jazz against real HDR content: exit 0, decoded output YAVG=26.39 (matches the software reference; untonemapped is 44.3), and ffprobe reports color_space=bt709, color_transfer=bt709, color_primaries=bt709.

Review

Independent cold review (Codex, no implementation role, review-only brief): no Blocker, one High and two Medium — all fixed in ee66cb74.

• High — HDR was re-tagged bt709 only when the profile had NormalizeColors on; with it off, tonemapped pixels stayed tagged bt2020 and the player converted them twice. Guard is now tonemapped || (ColorsAreBt709 && …).
• Medium — image subtitles stopped being resized: the subtitle-scaling predicate recognized only the QSV scale filters, so a 4K HDR + PGS source got a 720p video and a source-size subtitle.
• Medium — anamorphic sources excluded (ScaleVaapiFilter uses ffmpeg's runtime sar, not the SAR VideoStream calculates), leaving them exactly where they were before.
• The remaining Low (deriveDevice's defaulted bool is a future-call-site trap) is an accepted residual in the record.

The reviewer also independently cleared my main worry — the encoder receiving software frames is accepted h264_qsv behavior, the extra_hw_frames floor from ffmpeg.qsv-extra-hw-frames-floor is not bypassed, and this was already true of the previous path.

Each of the three fixes was negative-controlled: reverting it fails exactly one test, and no others.

Tests & docs

75 FFmpeg tests, 652 Core, 7 Architecture green; format gate clean; decisions validator OK. New tests pin the exact validated graph as an ordered substring, assert against the vpp_qsv option rather than a bare tonemap=1 substring, and cover NormalizeColors=false, anamorphic, image subtitles, and the no-tonemap_opencl fallback.

• New record ffmpeg.qsv-hdr-tonemap-opencl (measurements, gate rationale, accepted residuals).
• Corrected ffmpeg.qsv-decode-encode-split: it told HDR-on-QSV users to set the flag off "to keep GPU tonemap" — advice that actually disabled tonemapping. Prose fix only; the decision stands.
• #505's ## Done-when was rewritten, since the original boxes specified the inverted outcome.

🤖 Generated with Claude Code

fixes #505 ## The finding that changed the issue #505 asked to route the #498 native-decode path through `TonemapQsvFilter` to move HDR tonemapping off the CPU. Measured on the Intel host (jazz: FFmpeg 8.1.2 / iHD 25.1.4 / UHD 630) against real HDR HEVC Main10 (`bt2020nc`/`bt2020`/`smpte2084`), **`vpp_qsv=tonemap=1` is a silent no-op**: a graph ending in it returns a frame **byte-identical (same md5)** to the same graph with no tonemap step, with no warning and no error. True with the explicit `hevc_qsv` decoder, on genuine QSV video-memory surfaces, before the scale, after the scale, and with `format=nv12`. QSV VPP tonemapping needs Gen11+; pre-Gen11 iHD ignores the option. Same frame, luma reference: software tonemap `YAVG=26.6` · no tonemap `44.3` · `vpp_qsv=tonemap=1` → **`44.3`**. So the issue's premise was inverted, and the branch it wanted to extend was **already broken**: the pre-existing `DecoderHardwareAccelerationMode == Qsv` path shipped untonemapped HDR whenever `QsvPreferNativeDecoder` was off — which is exactly the escape hatch #498/#523 recommend. **Prod was not affected** (native-decode is the default and took the working software branch), so this is a latent bug, not a live incident. ## What ships GPU tonemapping via OpenCL, the route `VaapiPipelineBuilder` already uses: ``` format=nv12|p010le|vaapi,hwupload=derive_device=vaapi, scale_vaapi=1280:720:force_divisible_by=2:format=p010,setsar=1, hwmap=derive_device=opencl,tonemap_opencl=tonemap=linear:format=nv12, hwdownload,format=nv12 ``` - **Scale must precede tonemap.** Tonemapping full-size is *slower than the software path it replaces* (15.5s wall for 12.5s of content — below realtime). Scale-first cuts total CPU ~60% and lands at the no-tonemap wall-clock floor. - **A QSV surface is a dead end** — it maps to neither OpenCL nor VA-API (`hwmap` → -38). So the gate requires frames still in software; the QSV decoder, `deinterlace_qsv` and anamorphic sources all fall back to the **software** tonemap: slower, but correct. - `TonemapQsvFilter` is **deleted**. A filter that silently does nothing is worse than no filter — it looks like coverage. Exact generated chains, old vs new, 2 reproducible rounds, 300 frames 3840x1608 → 1280x720 `h264_qsv`: | | user CPU | total CPU | wall | |---|---|---|---| | OLD software tonemap | 33.9s | 35.8s | 10.0s | | **NEW OpenCL tonemap** | 11.1s | **14.2s** | **8.8s** | ## Hardware validation (the #505 Done-when box) The **exact generated command** run on jazz against real HDR content: exit 0, decoded output `YAVG=26.39` (matches the software reference; untonemapped is 44.3), and `ffprobe` reports `color_space=bt709`, `color_transfer=bt709`, `color_primaries=bt709`. ## Review Independent cold review (Codex, no implementation role, review-only brief): **no Blocker**, one High and two Medium — all fixed in `ee66cb74`. - **High** — HDR was re-tagged bt709 only when the profile had `NormalizeColors` on; with it off, tonemapped pixels stayed tagged bt2020 and the player converted them twice. Guard is now `tonemapped || (ColorsAreBt709 && …)`. - **Medium** — image subtitles stopped being resized: the subtitle-scaling predicate recognized only the QSV scale filters, so a 4K HDR + PGS source got a 720p video and a source-size subtitle. - **Medium** — anamorphic sources excluded (`ScaleVaapiFilter` uses ffmpeg's runtime `sar`, not the SAR `VideoStream` calculates), leaving them exactly where they were before. - The remaining Low (`deriveDevice`'s defaulted bool is a future-call-site trap) is an accepted residual in the record. The reviewer also independently cleared my main worry — the encoder receiving software frames is accepted `h264_qsv` behavior, the `extra_hw_frames` floor from `ffmpeg.qsv-extra-hw-frames-floor` is not bypassed, and this was already true of the previous path. **Each of the three fixes was negative-controlled**: reverting it fails exactly one test, and no others. ## Tests & docs 75 FFmpeg tests, 652 Core, 7 Architecture green; format gate clean; decisions validator OK. New tests pin the exact validated graph as an ordered substring, assert against the `vpp_qsv` *option* rather than a bare `tonemap=1` substring, and cover `NormalizeColors=false`, anamorphic, image subtitles, and the no-`tonemap_opencl` fallback. - New record `ffmpeg.qsv-hdr-tonemap-opencl` (measurements, gate rationale, accepted residuals). - Corrected `ffmpeg.qsv-decode-encode-split`: it told HDR-on-QSV users to set the flag **off "to keep GPU tonemap"** — advice that actually disabled tonemapping. Prose fix only; the decision stands. - #505's `## Done-when` was rewritten, since the original boxes specified the inverted outcome. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

timothy added 4 commits 2026-07-26 21:17:11 +02:00

fix(505): tonemap QSV HDR through OpenCL; vpp_qsv=tonemap is a silent no-op ... 41e2870113

#505 asked to route the #498 native-decode path through TonemapQsvFilter to
move HDR tonemapping off the CPU. Measured on the Intel host (jazz: FFmpeg
8.1.2 / iHD 25.1.4 / UHD 630) against real HDR HEVC Main10, that filter is a
SILENT no-op: a graph ending in vpp_qsv=tonemap=1 returns a frame byte-identical
(same md5) to the same graph with no tonemap at all, with no warning and no
error. QSV VPP tonemapping needs Gen11+; pre-Gen11 iHD ignores it.

So the issue's premise was inverted, and the branch it wanted to extend was
already broken: the existing DecoderHardwareAccelerationMode == Qsv path shipped
untonemapped HDR whenever QsvPreferNativeDecoder was off -- which is exactly the
escape hatch #498/#523 recommend. Prod was unaffected (native-decode is the
default and took the working software branch).

Tonemap on the GPU via OpenCL instead, the route VaapiPipelineBuilder already
uses and the one Jellyfin uses. The scale has to run first, in scale_vaapi:
tonemapping full-size is slower than the software path it replaces (15.5s wall
for 12.5s of content, below realtime), while scale-first cuts total CPU ~60%
(35.6s -> 14.1s) and lands at the no-tonemap wall-clock floor. A QSV surface
maps to neither OpenCL nor VA-API, so the gate requires software frames: the
QSV decoder and deinterlace_qsv both fall back to the software tonemap, slower
but correct. TonemapQsvFilter is deleted -- a filter that silently does nothing
is worse than no filter.

Also fixes output tagging: the first end-to-end run was correctly tonemapped yet
still announced bt2020 primaries, because SetPixelFormat's usesVppQsv predicate
("did a hardware filter strip color info") listed only the QSV filters. Both new
filters are now in it.

Validated end to end on jazz with the exact generated command: exit 0,
YAVG 26.39 (software reference 26.6, untonemapped 44.3), and ffprobe reports
bt709 space/transfer/primaries.

fixes #505

fix(505): gate the OpenCL tonemap on the ACTUAL frame location, not an enumeration ... 7c075ffa70

UseOpenClTonemap excluded the two known ways frames reach a hardware surface
before the tonemap (the QSV decoder, deinterlace_qsv). That is only correct for
as long as the enumeration stays complete -- and the failure mode if it ever
stops being complete is a second hwupload stacked on frames that are already on
a surface.

Check currentState.FrameDataLocation directly instead. The route begins with
hwupload, so "frames are in software" is the actual precondition; the specific
exclusions stay as documentation of the known cases, but the state check is what
makes it safe. A future filter landing ahead of the tonemap now degrades to the
software tonemap rather than emitting a broken graph.

docs(505): correct the #498 record's escape-hatch claim, which was wrong the dangerous way ... 18c4f4e0b2

The qsv-decode-encode-split record told HDR-on-QSV users to set
QsvPreferNativeDecoder OFF "to keep GPU tonemap". Measured on the Intel host,
vpp_qsv=tonemap=1 is a silent no-op on pre-Gen11 graphics, so following that
advice did not preserve GPU tonemapping -- it disabled tonemapping entirely and
shipped untonemapped HDR.

Prose correction only; the decision itself (the QsvPreferNativeDecoder toggle)
is unchanged and stays active. The software-tonemap half of the bullet was
always accurate and stands.

Decisions-Edit: yes

fix(505): address cold-review findings — retag on tonemap, subtitle scale, anamorphic ... ee66cb7459

Independent cold review (Codex, no implementation role) found no Blocker and
three real defects, all fixed here:

HIGH — HDR was re-tagged bt709 only when the profile had NormalizeColors on.
The colorspace filter sat behind desiredState.ColorsAreBt709, so an operator
with normalization disabled got tonemapped SDR pixels still tagged bt2020 and
the player converted them a second time. The guard is now
"tonemapped || (ColorsAreBt709 && ...)". Deliberately NOT fixed by hoisting
usesVppQsv out of the guard, which would force bt709 on scale-only non-HDR
chains that legitimately opted out.

MEDIUM — image subtitles stopped being resized. The subtitle canvas is scaled
only when the video chain contains a recognized scale filter, and that predicate
listed the QSV filters only; swapping ScaleQsvFilter for ScaleVaapiFilter left a
4K HDR + PGS source with a 720p video and a source-size subtitle overlay.
VaapiPipelineBuilder already listed ScaleVaapiFilter; QsvPipelineBuilder does now.

MEDIUM — anamorphic HDR now falls back to the software tonemap. ScaleQsvFilter
is handed the SAR VideoStream calculates (it has a 0:0 fallback); ScaleVaapiFilter
multiplies by ffmpeg's runtime `sar`, which differs when the decoded frame leaves
SAR unspecified. Rather than ship a graph nobody has run, exclude anamorphic --
which leaves those sources exactly where they were before this change.

LOW — tests now pin the exact validated graph as an ordered substring (the prior
assertions would have passed with setFormat off, hwdownload dropped, or the wrong
tonemap output format), assert against the vpp_qsv OPTION rather than a bare
"tonemap=1" substring, and cover NormalizeColors=false, anamorphic and image
subtitles. Each of the three fixes was negative-controlled: reverting it fails
exactly one test, and no others.

The remaining LOW (deriveDevice's defaulted bool is a future-call-site trap) is
recorded as an accepted residual rather than fixed, since a named factory would
push this diff into the VA-API pipeline for no behavior change.

The record is 86 prose lines, over the 60-line ceiling. Declining to cut: every
bullet is a distinct measured finding, which docs.corpus-size-signal names as a
legitimate decline.

Decisions-Edit: yes

timothy commented 2026-07-26 21:47:57 +02:00

Author

Owner

Copy Link

Copy Source

Review-verdict: MERGEABLE @ ee66cb7

Cold Codex review of 41e28701: no Blocker, 1 High + 2 Medium, all fixed in ee66cb74. Re-review of the fix commits: no findings at any level, all 5 items SOUND. Each fix negative-controlled locally (reverting fails exactly one test). Validated end-to-end on the Intel QSV host with the exact generated command.

Review-verdict: MERGEABLE @ ee66cb7 Cold Codex review of 41e28701: no Blocker, 1 High + 2 Medium, all fixed in ee66cb74. Re-review of the fix commits: no findings at any level, all 5 items SOUND. Each fix negative-controlled locally (reverting fails exactly one test). Validated end-to-end on the Intel QSV host with the exact generated command.

timothy merged commit 6dbc072837 into main 2026-07-26 21:56:02 +02:00

timothy deleted branch fix/505-qsv-native-decode-tonemap 2026-07-26 21:56:03 +02:00

timothy referenced this issue from a commit 2026-07-26 21:56:04 +02:00

Merge pull request 'fix(505): tonemap QSV HDR through OpenCL — vpp_qsv=tonemap is a silent no-op' (#654) from fix/505-qsv-native-decode-tonemap into main

timothy referenced this pull request 2026-07-26 21:56:48 +02:00

QSV native-decode path tonemaps HDR in software (route through tonemap_qsv) #505

timothy referenced this pull request 2026-07-26 22:37:00 +02:00

Anamorphic HDR sources are excluded from the QSV OpenCL tonemap (runtime `sar` vs calculated SAR) #660

timothy referenced this pull request 2026-07-26 22:37:28 +02:00

`HardwareUploadVaapiFilter(setFormat, deriveDevice = false)` — a defaulted bool that silently strands frames on the wrong device #661

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

ad-hoc

One-off / ad-hoc work not tracked by a dedicated issue

api

REST API / HTTP endpoints

bug

Something isn't working

ci-cd

Build, test, deploy pipeline

content

Channel content / schedules / playlists

dependencies

Dependency updates (Renovate)

enhancement

New feature or improvement

frontend

ChicoryTV React SPA frontend

in-progress

Claimed by an active session — do not pick up

jellyfin

Jellyfin tuner / IPTV integration

parked

Excluded from automatic queue pickup; work only when explicitly selected

priority: high

priority: low

priority: medium

review

Adversarial review finding

security

Security / vulnerability fix

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

renovate timothy (Timothy)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: timothy/ersatztv#654