feat(735): bound the numeric FFmpeg profile fields with a 422, and expose readrate pacing #847

Merged

timothy merged 1 commits from fix/735-ffmpeg-profile-bounds into main 2026-08-27 00:05:39 +02:00

Conversation 1 Commits 1 Files Changed 40

+15763 -237

timothy commented 2026-08-26 23:35:47 +02:00

Owner

Copy Link

Copy Source

Closes the #529 accepted residual and makes the #726 pacing values operator-tunable.

Problem 1 — a submitted value was silently transformed

The create/update handlers floored qsvExtraHardwareFrames on save, so a client that PUT 0 got a
200 and read back 64 — a substitution the OpenAPI document never advertised. FFmpegProfileBounds
now rejects a newly submitted out-of-range value with a 422 naming the bound and its
consequence. The render-time floor in FFmpegState stays, which is what keeps this migration-free
for rows written before the validation existed.

An unchanged legacy value is still accepted on update: the SPA round-trips the whole profile, so
rejecting a stored-but-out-of-range value would make an old profile uneditable over a field the
operator never touched and — outside QSV — cannot even see. The SPA mirrors that exemption
(validate(draft, stored)), and the add/copy path passes no stored draft so it stays strict, exactly
like the create handler's stored: null.

Problem 2 — pacing was unreachable

-readrate / -readrate_catchup were hardcoded in PipelineBuilderBase. During #726 the only way
to try a different value was to rebuild the app and replay command lines by hand on the Docker host.
They are now nullable ReadRate / ReadRateCatchup profile fields, bounded 1.0–2.0 and 1.0–10.0
plus a cross-field rule that catchup must be greater than the read rate (equal is zero headroom —
functionally no catchup, while still reading as configured). null means unset and resolves to the
values the pipeline used before, so an untouched profile paces identically; catchup stays ON by
default and capability-gated in code.

Deliberately not exposed: -readrate_initial_burst (derived from the HLS segment length, not an
independent dial) and the concat wrapper's -readrate 1.0 (it reads ErsatzTV's own loopback output,
not a media file's demuxer). A raw-args passthrough remains #736's scope.

Verification

• 4,469 .NET tests, 1,233 web tests, 989 script tests; typecheck / lint / check:api clean with no
  generated drift; format + BOM gate on the committed tree; decisions_validate OK.
• Live-E2E against a real instance: create, update and clear-to-null round-trip through the read
  path; every rejection path returns 422 with the bound named and leaves stored state untouched.
• New UI-E2E spec drives the actual screen (fields render, the cross-field bound disables Save,
  a valid pair survives a reload).
• Every new clause is mutation-proved: ignoring the pacing override, disarming each bound,
  weakening <= to <, removing the legacy exemption, and re-adding the QSV gate each redden their
  own tests and nothing else.

Review

Three adversarial rounds (cold, review-only, isolated worktree). Five confirmed defects in round 1,
two in round 2, zero in round 3 — all in durable artefacts (a record naming a test file that never
existed, a conventions bullet contradicted by the file it described, a comment stating a stricter
rule than its comparison, a help panel still teaching the retracted rule, and a duplicate ### 3c).
One low-severity UX residual is deliberately deferred and filed separately: when a legacy non-QSV
profile is copied, the 422 names a control the editor only renders under QSV.

fixes #735

Closes the #529 accepted residual and makes the #726 pacing values operator-tunable. ## Problem 1 — a submitted value was silently transformed The create/update handlers floored `qsvExtraHardwareFrames` on save, so a client that `PUT` 0 got a 200 and read back 64 — a substitution the OpenAPI document never advertised. `FFmpegProfileBounds` now rejects a **newly submitted** out-of-range value with a 422 naming the bound *and its consequence*. The render-time floor in `FFmpegState` stays, which is what keeps this migration-free for rows written before the validation existed. An **unchanged** legacy value is still accepted on update: the SPA round-trips the whole profile, so rejecting a stored-but-out-of-range value would make an old profile uneditable over a field the operator never touched and — outside QSV — cannot even see. The SPA mirrors that exemption (`validate(draft, stored)`), and the add/copy path passes no stored draft so it stays strict, exactly like the create handler's `stored: null`. ## Problem 2 — pacing was unreachable `-readrate` / `-readrate_catchup` were hardcoded in `PipelineBuilderBase`. During #726 the only way to try a different value was to rebuild the app and replay command lines by hand on the Docker host. They are now nullable `ReadRate` / `ReadRateCatchup` profile fields, bounded 1.0–2.0 and 1.0–10.0 plus a cross-field rule that catchup must be **greater than** the read rate (equal is zero headroom — functionally no catchup, while still reading as configured). `null` means unset and resolves to the values the pipeline used before, so an untouched profile paces identically; catchup stays ON by default and capability-gated in code. Deliberately **not** exposed: `-readrate_initial_burst` (derived from the HLS segment length, not an independent dial) and the concat wrapper's `-readrate 1.0` (it reads ErsatzTV's own loopback output, not a media file's demuxer). A raw-args passthrough remains #736's scope. ## Verification - 4,469 .NET tests, 1,233 web tests, 989 script tests; typecheck / lint / `check:api` clean with no generated drift; format + BOM gate on the committed tree; `decisions_validate` OK. - **Live-E2E** against a real instance: create, update and clear-to-null round-trip through the read path; every rejection path returns 422 with the bound named and leaves stored state untouched. - New **UI-E2E** spec drives the actual screen (fields render, the cross-field bound disables Save, a valid pair survives a reload). - Every new clause is **mutation-proved**: ignoring the pacing override, disarming each bound, weakening `<=` to `<`, removing the legacy exemption, and re-adding the QSV gate each redden their own tests and nothing else. ## Review Three adversarial rounds (cold, review-only, isolated worktree). Five confirmed defects in round 1, two in round 2, zero in round 3 — all in durable artefacts (a record naming a test file that never existed, a conventions bullet contradicted by the file it described, a comment stating a stricter rule than its comparison, a help panel still teaching the retracted rule, and a duplicate `### 3c`). One low-severity UX residual is deliberately deferred and filed separately: when a legacy non-QSV profile is copied, the 422 names a control the editor only renders under QSV. fixes #735

timothy added 1 commit 2026-08-26 23:35:47 +02:00

feat(735): bound the numeric FFmpeg profile fields with a 422, and expose readrate pacing ... 5459e21e81

The write path used to accept an out-of-range `qsvExtraHardwareFrames`, store the floored value
instead and return 200 — the accepted residual #529 recorded, where a client PUTs 0 and reads back
64. `FFmpegProfileBounds` now rejects a newly submitted out-of-range value with a 422 naming the
bound and its consequence; the render-time floor in `FFmpegState` stays, which is what keeps the
change migration-free for rows written before this existed. On update an UNCHANGED legacy value is
still accepted, because the SPA round-trips the whole profile and rejecting it would make an old
profile uneditable over a field the operator never touched and, outside QSV, cannot see.

The `-readrate` / `-readrate_catchup` pacing that `PipelineBuilderBase` hardcoded becomes two
nullable profile fields with the same bounded treatment. During #726 the only way to try a
different pacing value was to rebuild the app and replay command lines by hand on the Docker host.
`null` means unset and resolves to the values the pipeline used before, so an untouched profile
paces identically; catchup stays on by default and capability-gated in code.

The draft helpers moved out of `FFmpegProfilesScreen.tsx` into `ffmpegProfileDraft.ts` so the
bounds can be tested directly — a component module that exports non-components trips
react-refresh, and the baseline was warning-free.

Mutation-checked: ignoring the pacing override, disarming each bound, and removing the
unchanged-legacy exemption each redden their own tests.

fixes #735

Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

timothy commented 2026-08-26 23:36:21 +02:00

Author

Owner

Copy Link

Copy Source

Review-verdict: MERGEABLE @ 5459e21

Three cold adversarial rounds (isolated worktree): 5 confirmed defects round 1, 2 round 2, 0 round 3. All fixes mutation-proved. Live-E2E + UI-E2E green.

Review-verdict: MERGEABLE @ 5459e21 Three cold adversarial rounds (isolated worktree): 5 confirmed defects round 1, 2 round 2, 0 round 3. All fixes mutation-proved. Live-E2E + UI-E2E green.

timothy referenced this pull request 2026-08-26 23:36:54 +02:00

The QSV frame-pool 422 can name a control the editor is not rendering #848

timothy merged commit ed8b602445 into main 2026-08-27 00:05:39 +02:00

timothy deleted branch fix/735-ffmpeg-profile-bounds 2026-08-27 00:05:41 +02:00

timothy referenced this pull request 2026-08-27 00:06:37 +02:00

Validate FFmpeg profile numeric fields to sane ranges instead of silently transforming them, and expose readrate pacing as a bounded field #735

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