fix(529): floor QSV extra hardware frames so an unthrottled read can't exhaust the pool #530

Merged

timothy merged 3 commits from fix/529-qsv-extra-hw-frames-floor into main 2026-07-21 16:41:16 +02:00

Conversation 1 Commits 3 Files Changed 9

+273 -31

timothy commented 2026-07-21 16:22:21 +02:00

Owner

Copy Link

Copy Source

fixes #529

A stored qsvExtraHardwareFrames of 0 reached FFmpeg as hwupload=extra_hw_frames=0, leaving the QSV upload pool no headroom for frames in flight through the filter graph. Any input that is not throttled then exhausts it: the graph fails with -12 (Cannot allocate memory), h264_qsv reports "Could not open encoder before EOF", and zero segments are written — a dead channel.

This is a live production bug, not a #516 regression

It was found while verifying #516 (#519), and filed as a regression. That framing was wrong, and the correction matters for how it's fixed:

readrate	extra_hw_frames	result
1.05, no burst	0	exit 0, 14 segments
1.05 + burst 2 / 4 / 8	0	ENOMEM, 0 segments
no readrate at all	0	ENOMEM, 0 segments
1.05 + burst 8	64	exit 0, 14 segments
no readrate at all	64	exit 0, 14 segments

A work-ahead start takes no -readrate at all, so this was already failing before #516 — intermittently, only for the session that won the work-ahead slot. #516 removed the throttle on every realtime session, turning an intermittent failure into a near-deterministic one, which is how it surfaced.

Production confirmed it independently: 21 × Cannot allocate memory and 6 × Could not open encoder before EOF in the last week on v26.11.0 (no burst), all six at 00:16–00:22 on 2026-07-21 — ~13 h before any of this investigation touched the host.

-readrate was doing load-bearing work nobody had written down: besides live-TV pacing, it incidentally bounded how fast decoded frames enter the filter graph. That is why #350's FFmpeg-level benchmark and #516's argument-generation tests were both green and neither could see this — the burst is bounded in seconds of input, which is not a bound on memory or hardware surfaces.

The fix

FFmpegState.QsvExtraHardwareFrames floors at MinimumQsvExtraHardwareFrames (64) — the value IfNone already used for an unset profile, and the seeded profile default. Values above the floor are honored unchanged.

Fixed at the single point every QSV upload site reads, so one guard covers HardwareUploadFilter, HardwareUploadQsvFilter, WatermarkHardwareUploadFilter, ScaleQsvFilter and DeinterlaceQsvFilter rather than five call sites that can drift apart. (Both reviewers independently verified no bypass exists.)

Supporting changes:

• QsvPipelineBuilder logs a warning naming configured + applied values — raising a deliberately small pool costs additional surfaces (~190 MiB at 1080p, ~760 MiB at 4K), so the override is discoverable rather than silent.
• Create/UpdateFFmpegProfileHandler normalize on save, so stored rows converge on what the pipeline runs instead of displaying a value the server overrides.
• The SPA field gets min={64} and a validate() branch — min alone only bounds the spinner arrows, since save is an onClick with no <form>.

The floor is deliberately wider than the evidence: only 0 (fails) and 64 (works) were measured, so 1..63 are untested, not known-bad. They're raised because the risk is a channel serving nothing. Recorded as a stated over-reach in docs/decisions.md, with the residual (render-time flooring means un-resaved rows still display the old value) written down rather than glossed.

Verification

Verified in production, not asserted. Prod's profile was set to 64 (operator-approved), then I drove the exposed pipeline directly via the troubleshooting playback API on an mpeg4 .avi (forces software decode + hwupload):

hwupload=extra_hw_frames=64,vpp_qsv=w=1875:h=1080   exit 0, speed 12.0x, 0 ENOMEM

Negative control on prod's own hardware — same command, only the pool differing:

extra_hw_frames	exit	segments	ENOMEM
64	0	8	0
0	244	0	3

That reproduces the six overnight production failures and confirms the mitigation.

Test gates: 4095 .NET + 891 web green; build 0 errors; dotnet format exit 0; BOM clean; decisions_validate.py OK; catalog up to date.

Negative-controlled twice, both with Build succeeded verified first (the stale-build trap that produced a false pass on #350/PR #516): reverting the FFmpegState floor fails 5 pipeline tests; reverting both handlers fails 5 handler tests.

Reviews

Two independent cold-context adversarial rounds. Round one returned BLOCKED on two HIGH documentation findings (the decision record opted out of the lifecycle schema #521 mandated the same day, and the entry claimed to correct the #350 record while leaving it untouched) — both fixed, plus an overstated claim narrowed and a factual error corrected (SubtitleScaleQsvFilter is dead code and is not covered by the guard). Round two reviewed the fix commit as new code and returned MERGEABLE; its Medium (untested handler normalization) and three Lows are all addressed in 11b78bfc.

Follow-up

#519's end-to-end re-verification on ersatztv-test stays open until :latest carries this fix.

fixes #529 A stored `qsvExtraHardwareFrames` of `0` reached FFmpeg as `hwupload=extra_hw_frames=0`, leaving the QSV upload pool no headroom for frames in flight through the filter graph. Any input that is **not throttled** then exhausts it: the graph fails with `-12 (Cannot allocate memory)`, `h264_qsv` reports "Could not open encoder before EOF", and **zero segments are written** — a dead channel. ## This is a live production bug, not a #516 regression It was found while verifying #516 (#519), and filed as a regression. That framing was **wrong**, and the correction matters for how it's fixed: | readrate | extra_hw_frames | result | |---|---|---| | `1.05`, no burst | 0 | exit 0, 14 segments | | `1.05` + burst **2 / 4 / 8** | 0 | **ENOMEM, 0 segments** | | **no readrate at all** | 0 | **ENOMEM, 0 segments** | | `1.05` + burst 8 | 64 | exit 0, 14 segments | | **no readrate at all** | 64 | exit 0, 14 segments | A **work-ahead start takes no `-readrate` at all**, so this was already failing before #516 — intermittently, only for the session that won the work-ahead slot. #516 removed the throttle on *every* realtime session, turning an intermittent failure into a near-deterministic one, which is how it surfaced. Production confirmed it independently: 21 × `Cannot allocate memory` and 6 × `Could not open encoder before EOF` in the last week on `v26.11.0` (no burst), all six at 00:16–00:22 on 2026-07-21 — ~13 h before any of this investigation touched the host. `-readrate` was doing load-bearing work nobody had written down: besides live-TV pacing, it *incidentally* bounded how fast decoded frames enter the filter graph. That is why #350's FFmpeg-level benchmark and #516's argument-generation tests were both green and neither could see this — the burst is bounded in **seconds of input**, which is not a bound on **memory or hardware surfaces**. ## The fix `FFmpegState.QsvExtraHardwareFrames` floors at `MinimumQsvExtraHardwareFrames` (64) — the value `IfNone` already used for an unset profile, and the seeded profile default. Values above the floor are honored unchanged. Fixed at the **single point** every QSV upload site reads, so one guard covers `HardwareUploadFilter`, `HardwareUploadQsvFilter`, `WatermarkHardwareUploadFilter`, `ScaleQsvFilter` and `DeinterlaceQsvFilter` rather than five call sites that can drift apart. (Both reviewers independently verified no bypass exists.) Supporting changes: - `QsvPipelineBuilder` **logs a warning** naming configured + applied values — raising a deliberately small pool costs additional surfaces (~190 MiB at 1080p, ~760 MiB at 4K), so the override is discoverable rather than silent. - `Create`/`UpdateFFmpegProfileHandler` **normalize on save**, so stored rows converge on what the pipeline runs instead of displaying a value the server overrides. - The SPA field gets `min={64}` **and** a `validate()` branch — `min` alone only bounds the spinner arrows, since save is an `onClick` with no `<form>`. **The floor is deliberately wider than the evidence**: only `0` (fails) and `64` (works) were measured, so `1..63` are *untested, not known-bad*. They're raised because the risk is a channel serving nothing. Recorded as a stated over-reach in `docs/decisions.md`, with the residual (render-time flooring means un-resaved rows still display the old value) written down rather than glossed. ## Verification Verified in production, not asserted. Prod's profile was set to 64 (operator-approved), then I drove the exposed pipeline directly via the troubleshooting playback API on an mpeg4 `.avi` (forces software decode + `hwupload`): ``` hwupload=extra_hw_frames=64,vpp_qsv=w=1875:h=1080 exit 0, speed 12.0x, 0 ENOMEM ``` Negative control on prod's own hardware — same command, only the pool differing: | extra_hw_frames | exit | segments | ENOMEM | |---|---|---|---| | 64 | 0 | **8** | 0 | | 0 | 244 | **0** | 3 | That reproduces the six overnight production failures and confirms the mitigation. Test gates: **4095 .NET + 891 web** green; build 0 errors; `dotnet format` exit 0; BOM clean; `decisions_validate.py` OK; catalog up to date. Negative-controlled twice, both with `Build succeeded` verified first (the stale-build trap that produced a false pass on #350/PR #516): reverting the `FFmpegState` floor fails 5 pipeline tests; reverting both handlers fails 5 handler tests. ## Reviews Two independent cold-context adversarial rounds. Round one returned **BLOCKED** on two HIGH documentation findings (the decision record opted out of the lifecycle schema #521 mandated the same day, and the entry claimed to correct the #350 record while leaving it untouched) — both fixed, plus an overstated claim narrowed and a factual error corrected (`SubtitleScaleQsvFilter` is dead code and is **not** covered by the guard). Round two reviewed the fix commit as new code and returned **MERGEABLE**; its Medium (untested handler normalization) and three Lows are all addressed in `11b78bfc`. ## Follow-up #519's end-to-end re-verification on `ersatztv-test` stays open until `:latest` carries this fix.

timothy added 3 commits 2026-07-21 16:22:21 +02:00

fix(529): floor QSV extra hardware frames so an unthrottled read can't exhaust the pool ... bbd7356f81

A stored qsvExtraHardwareFrames of 0 reached FFmpeg as hwupload=extra_hw_frames=0,
leaving the QSV upload pool no headroom for frames in flight through the filter graph.
Any input that is not throttled then exhausts it: the graph fails with -12 (Cannot
allocate memory), h264_qsv reports "Could not open encoder before EOF", and zero
segments are written.

Measured against the deployed FFmpeg 8.1.2, one real logged command, only the marked
tokens differing:

  readrate 1.05, no burst   + frames 0  -> exit 0,   14 segments
  readrate 1.05 + burst 2/4/8 + frames 0 -> exit 244, ENOMEM, 0 segments
  no readrate at all        + frames 0  -> exit 244, ENOMEM, 0 segments
  readrate 1.05 + burst 8   + frames 64 -> exit 0,   14 segments
  no readrate at all        + frames 64 -> exit 0,   14 segments

So the defect predates #350's cold-start burst: a work-ahead start takes no -readrate
and was already failing on a profile with 0. The burst removed the throttle on every
realtime session, turning an intermittent failure into a near-deterministic one, which
is how it surfaced. Input throttling was doing load-bearing allocation-bounding work
that nobody had written down -- which is why the FFmpeg-level benchmark in #350 and the
argument-generation tests in #516 were both green and neither could see it.

Fixed at FFmpegState.QsvExtraHardwareFrames, the single point every QSV upload site
reads, so one guard covers HardwareUploadFilter, HardwareUploadQsvFilter,
WatermarkHardwareUploadFilter, ScaleQsvFilter, DeinterlaceQsvFilter and
SubtitleScaleQsvFilter rather than six call sites that can drift apart. Values above
the floor are still honored.

Tests negative-controlled: reverting the floor fails exactly the three sub-minimum
cases, with the build verified succeeded first.

Refs #350, #516, #519.

fix(529): address review — decision schema, #350 forward pointer, warning log, save-time normalization [decisions-edit] ... b27c950943

Cold adversarial review returned BLOCKED on the documentation half. Addressed:

- The new decision record carried no lifecycle metadata block, taking the repo from
  82/82 to 83/82 and making it invisible to the by-key catalog lookup that #521
  established the same day. Added key/status/since/supersedes/superseded-by
  (ffmpeg.qsv-extra-hw-frames-floor) and regenerated docs/decisions/README.md;
  decisions_validate.py now reports OK with no legacy-unmigrated notice.
- The entry claimed to correct the #350 record but left that record untouched, so the
  stale "the burst is bounded" claim stayed authoritative for anyone resolving
  ffmpeg.hls-cold-start-burst. Added a forward-pointing correction note there (hence
  the [decisions-edit] token on this commit).
- The floor was applied silently. QsvPipelineBuilder.SetAccelState now logs a warning
  naming both the configured and applied value, because raising a deliberately small
  pool costs additional surfaces (64 NV12 1080p surfaces is roughly 190 MiB, 760 MiB
  at 4K) on memory-constrained iGPUs.
- Narrowed an overstated claim in the entry: 1..63 are untested, not known-bad. We
  raise them because the risk is a channel serving nothing, not because asking for
  less is illegitimate. Recorded as a deliberate over-reach with a stated cost.
- Corrected a factual error: SubtitleScaleQsvFilter also formats extra_hw_frames but
  is dead code with no construction site, so it is NOT covered by the guard.
- Config-vs-behavior mismatch: Create/UpdateFFmpegProfileHandler now normalize on
  save so stored rows converge on what the pipeline runs, and the SPA field carries
  min=64 rather than defaulting the display to 0. Render-time flooring is kept as the
  net that fixes existing deployments with no migration; the remaining gap for
  un-resaved rows is recorded as an accepted residual.
- Tests strengthened: pinned to the literal measured 64 rather than to the constant
  (so lowering the floor cannot quietly satisfy them), added a negative-value case,
  added a deinterlace-upload case, and replaced the narrow ShouldNotContain with a
  regex asserting EVERY extra_hw_frames occurrence in the command is >= the minimum.

Negative control re-run against the strengthened tests: reverting the floor fails 5,
with the build verified succeeded first. Full suite green (4086 .NET, 891 web).

Review finding that needed no change: the "single point" claim was independently
verified — no bypass exists, every FFmpegState construction routes through
MaybeQsvExtraHardwareFrames.

Refs #350, #516, #519.

fix(529): round-two review — cover the handlers with tests, validate in the SPA [decisions-edit] ... 11b78bfcbd

Second review returned MERGEABLE with one Medium and three Lows. Addressed all four:

- Medium: the save-time normalization had zero test coverage, so a later refactor
  dropping Math.Max would leave the suite green (the FFmpegState floor keeps the
  pipeline correct, hiding the regression until someone reads a stored 0 back).
  Added Create/Update_Should_Floor_QsvExtraHardwareFrames over 0, -8, 63, 64 and 128,
  plus Create_Should_Leave_Null_QsvExtraHardwareFrames_Null for the null-passthrough
  branch, following the existing QsvPreferNativeDecoder tests' seed/handle/re-read
  shape. Negative-controlled: reverting both handlers fails exactly 5.

- Low: the SPA `min` was cosmetic. Input does forward it to the DOM, but there is no
  <form> — save is an onClick gated only on validate(), which had no branch for this
  field, so a typed 10 submitted fine and was silently changed to 64 with a 200 and no
  message. validate() now rejects it client-side.

- Low: the warning fires at the top of SetAccelState, before we know whether the
  pipeline uploads at all, so a fully-hardware path could be told "using 64 instead"
  when nothing consumed either value. Reworded to "will use ... wherever frames are
  uploaded".

- Low: recorded in the decision entry that the save-time normalization is
  unconditional on hardwareAcceleration (a non-QSV profile's stored value moves too),
  and that a client PUTting 0 reads back 64 — a transform the OpenAPI description does
  not advertise.

Verified in production, not just asserted. Set prod's profile to 64 (operator-approved)
and drove the exposed pipeline myself via the troubleshooting playback API on an mpeg4
.avi, which forces software decode + hwupload:

  hwupload=extra_hw_frames=64,vpp_qsv=w=1875:h=1080   exit 0, speed 12.0x, 0 ENOMEM

Then the negative control on prod's own hardware, same command, only the pool differing:

  extra_hw_frames=64 -> exit 0,   8 segments, 0 ENOMEM
  extra_hw_frames=0  -> exit 244, 0 segments, 3 ENOMEM

which reproduces the six overnight production failures and confirms the fix.

Full suite green: 4095 .NET, 891 web.

Refs #350, #516, #519.

timothy commented 2026-07-21 16:33:33 +02:00

Author

Owner

Copy Link

Copy Source

Independent review: three cold-context rounds, all findings resolved

Round	Target	Verdict
1	bbd7356f (original fix)	BLOCKED — 2 HIGH (documentation), 2 MEDIUM, 2 LOW
2	b27c9509 (response to round 1, as new code)	MERGEABLE — 1 MEDIUM, 3 LOW
3	11b78bfc (response to round 2, as new code)	MERGEABLE — 0 Blocker/High/Medium

Each round reviewed the fix commit as code no reviewer had seen, rather than re-reading the original diff — the #242 lesson, which is what caught the round-two Medium.

Round 3 verified by measurement, not inspection

Gates re-run independently by the reviewer: build 0 warnings/0 errors · 4095 .NET + 891 web green · typecheck/lint clean · decisions_validate.py OK · catalog up to date · kickoff guard OK · dotnet format clean · BOM clean.

The two things I most wanted checked, both confirmed:

• The shared MakeCreate/MakeUpdate factory change landed in the correct positional slot in both records (CreateFFmpegProfile param 9, UpdateFFmpegProfile param 10 — int? QsvExtraHardwareFrames, between string VaapiDevice and int ResolutionId). The new parameter defaults to null, byte-identical to the literal it replaced, so no pre-existing test changed behavior. 16 tests in the file = 7 pre-existing + 9 new.
• The negative control's "exactly 5" claim is accurate — reverted both handlers, touched to force a real rebuild (the stale-build trap), got Failed: 5, Passed: 11: Create(0, -8, 63) + Update(0, -8), with the 64→64 and 128→128 identity cases correctly staying green.

Also confirmed the SPA branch uses != null rather than a falsy check — which matters, because 0 is exactly production's value and !draft.qsvExtraHardwareFrames would have let it through.

Accepted residuals (stated, not silent)

1. The SPA validate() branch ships without test coverage. There is no FFmpegProfilesScreen.test.tsx and validate is not exported, so there is no harness to extend. Accepted: the server-side floor is the actual guarantee and it is negative-controlled; the client check is a convenience message. A later refactor dropping it would leave the 891 web tests green — that is the known cost.
2. The client gate is narrower than the server transform — it gates on hardwareAcceleration === 'Qsv' (mirroring the field's render gate) while the handlers normalize unconditionally, so switching a legacy sub-64 QSV profile to None saves without a client message and reads back 64. Disclosed in docs/decisions.md; harmless, since only the QSV path reads the field.
3. MINIMUM_QSV_EXTRA_HARDWARE_FRAMES in the SPA duplicates FFmpegState.MinimumQsvExtraHardwareFrames with no drift guard.

Per the skip rubric this diff was not eligible for a review exemption — it touches an FFmpeg pipeline and two API write-path handlers — so all three rounds were mandatory rather than optional.

Review-verdict: MERGEABLE @ 11b78bfcbd

## Independent review: three cold-context rounds, all findings resolved | Round | Target | Verdict | |---|---|---| | 1 | `bbd7356f` (original fix) | **BLOCKED** — 2 HIGH (documentation), 2 MEDIUM, 2 LOW | | 2 | `b27c9509` (response to round 1, as new code) | **MERGEABLE** — 1 MEDIUM, 3 LOW | | 3 | `11b78bfc` (response to round 2, as new code) | **MERGEABLE** — 0 Blocker/High/Medium | Each round reviewed the *fix commit* as code no reviewer had seen, rather than re-reading the original diff — the #242 lesson, which is what caught the round-two Medium. ### Round 3 verified by measurement, not inspection Gates re-run independently by the reviewer: build 0 warnings/0 errors · **4095 .NET** + **891 web** green · typecheck/lint clean · `decisions_validate.py` OK · catalog up to date · kickoff guard OK · `dotnet format` clean · BOM clean. The two things I most wanted checked, both confirmed: - **The shared `MakeCreate`/`MakeUpdate` factory change landed in the correct positional slot** in both records (`CreateFFmpegProfile` param 9, `UpdateFFmpegProfile` param 10 — `int? QsvExtraHardwareFrames`, between `string VaapiDevice` and `int ResolutionId`). The new parameter defaults to `null`, byte-identical to the literal it replaced, so **no pre-existing test changed behavior**. 16 tests in the file = 7 pre-existing + 9 new. - **The negative control's "exactly 5" claim is accurate** — reverted both handlers, `touch`ed to force a real rebuild (the stale-build trap), got `Failed: 5, Passed: 11`: `Create`(0, -8, 63) + `Update`(0, -8), with the `64→64` and `128→128` identity cases correctly staying green. Also confirmed the SPA branch uses `!= null` rather than a falsy check — which matters, because `0` is exactly production's value and `!draft.qsvExtraHardwareFrames` would have let it through. ### Accepted residuals (stated, not silent) 1. **The SPA `validate()` branch ships without test coverage.** There is no `FFmpegProfilesScreen.test.tsx` and `validate` is not exported, so there is no harness to extend. Accepted: the server-side floor is the actual guarantee and it *is* negative-controlled; the client check is a convenience message. A later refactor dropping it would leave the 891 web tests green — that is the known cost. 2. **The client gate is narrower than the server transform** — it gates on `hardwareAcceleration === 'Qsv'` (mirroring the field's render gate) while the handlers normalize unconditionally, so switching a legacy sub-64 QSV profile to `None` saves without a client message and reads back `64`. Disclosed in `docs/decisions.md`; harmless, since only the QSV path reads the field. 3. `MINIMUM_QSV_EXTRA_HARDWARE_FRAMES` in the SPA duplicates `FFmpegState.MinimumQsvExtraHardwareFrames` with no drift guard. Per the skip rubric this diff was **not** eligible for a review exemption — it touches an FFmpeg pipeline and two API write-path handlers — so all three rounds were mandatory rather than optional. Review-verdict: MERGEABLE @ 11b78bfcbda7b9e7fe4082372ac3c9177883f2b5

timothy referenced this pull request 2026-07-21 16:34:18 +02:00

QSV `extra_hw_frames=0` kills transcodes on any unthrottled read — live in prod, and made near-deterministic by #516's burst #529

timothy merged commit 2834605d63 into main 2026-07-21 16:41:16 +02:00

timothy deleted branch fix/529-qsv-extra-hw-frames-floor 2026-07-21 16:41:16 +02:00

timothy referenced this issue from a commit 2026-07-21 16:41:18 +02:00

Merge pull request 'fix(529): floor QSV extra hardware frames so an unthrottled read can't exhaust the pool' (#530) from fix/529-qsv-extra-hw-frames-floor into main

timothy referenced this pull request 2026-07-21 16:42:01 +02:00

QSV `extra_hw_frames=0` kills transcodes on any unthrottled read — live in prod, and made near-deterministic by #516's burst #529

timothy referenced this pull request 2026-07-21 17:02:29 +02:00

Verify the #350 cold-start burst end-to-end on test, then re-measure on prod #519

timothy referenced this pull request 2026-07-22 19:40:56 +02:00

QSV native-decode (#498) hwupload fails to allocate a QSV surface for certain content — "Cannot allocate memory" kills the stream #523

timothy referenced this pull request 2026-08-04 21:28:26 +02:00

Embedded BITMAP subtitle (PGS/DVD) burn-in + `-readrate` halves transcode to 0.53x realtime — Live TV buffering #726

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#530