From bbd7356f81a356742dac67fd6ad22b3e8f14eb9a Mon Sep 17 00:00:00 2001 From: Timothy Date: Tue, 21 Jul 2026 15:52:57 +0200 Subject: [PATCH 1/3] fix(529): floor QSV extra hardware frames so an unthrottled read can't exhaust the pool 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. --- .../Pipeline/QsvPipelineBuilderTests.cs | 27 +++++++++++- ErsatzTV.FFmpeg/FFmpegState.cs | 12 +++++- docs/decisions.md | 42 +++++++++++++++++++ 3 files changed, 79 insertions(+), 2 deletions(-) diff --git a/ErsatzTV.FFmpeg.Tests/Pipeline/QsvPipelineBuilderTests.cs b/ErsatzTV.FFmpeg.Tests/Pipeline/QsvPipelineBuilderTests.cs index 1d97e65c7..f9967ea0a 100644 --- a/ErsatzTV.FFmpeg.Tests/Pipeline/QsvPipelineBuilderTests.cs +++ b/ErsatzTV.FFmpeg.Tests/Pipeline/QsvPipelineBuilderTests.cs @@ -90,11 +90,36 @@ public class QsvPipelineBuilderTests command.ShouldContain("h264_qsv"); } - private string BuildAndPrint(bool preferNativeDecoder) + // ersatztv#529: a stored qsvExtraHardwareFrames of 0 produced hwupload=extra_hw_frames=0, which + // leaves the QSV pool no headroom. Measured on the deployed FFmpeg 8.1.2: with 0 the filter graph + // fails with -12 and writes zero segments as soon as the input is not throttled (a work-ahead + // start, or #350's cold-start burst); with 64 the same command writes segments either way. + [TestCase(0)] + [TestCase(1)] + [TestCase(63)] + public void Qsv_Should_Never_Upload_With_Less_Than_Minimum_Extra_Hardware_Frames(int configured) + { + string command = BuildAndPrint(preferNativeDecoder: true, maybeExtraHardwareFrames: configured); + + command.ShouldContain($"hwupload=extra_hw_frames={FFmpegState.MinimumQsvExtraHardwareFrames}"); + command.ShouldNotContain("hwupload=extra_hw_frames=0,"); + } + + [Test] + public void Qsv_Should_Honor_Extra_Hardware_Frames_Above_The_Minimum() + { + string command = BuildAndPrint(preferNativeDecoder: true, maybeExtraHardwareFrames: 128); + + command.ShouldContain("hwupload=extra_hw_frames=128"); + } + + private string BuildAndPrint(bool preferNativeDecoder, Option maybeExtraHardwareFrames = default) { (VideoInputFile videoInputFile, AudioInputFile audioInputFile, FFmpegState ffmpegState, FrameState desiredState) = BuildQsvH264Pipeline(preferNativeDecoder, ScanKind.Progressive, false); + ffmpegState = ffmpegState with { MaybeQsvExtraHardwareFrames = maybeExtraHardwareFrames }; + var builder = new QsvPipelineBuilder( new DefaultFFmpegCapabilities(), new DefaultHardwareCapabilities(), diff --git a/ErsatzTV.FFmpeg/FFmpegState.cs b/ErsatzTV.FFmpeg/FFmpegState.cs index 028f94055..0206f24a7 100644 --- a/ErsatzTV.FFmpeg/FFmpegState.cs +++ b/ErsatzTV.FFmpeg/FFmpegState.cs @@ -30,7 +30,17 @@ public record FFmpegState( bool IsTroubleshooting, bool QsvPreferNativeDecoder = false) { - public int QsvExtraHardwareFrames => MaybeQsvExtraHardwareFrames.IfNone(64); + // the QSV upload pool needs headroom for the frames in flight through the filter graph. + // extra_hw_frames=0 leaves none, so any input that is not throttled exhausts it: the graph + // fails with -12 (Cannot allocate memory), h264_qsv reports "Could not open encoder before + // EOF", and the output file gets no packets at all. Input throttling was the only thing + // hiding it — a work-ahead start (no -readrate) and #350's cold-start burst both remove that + // throttle, so the channel simply dies. A stored 0 is therefore treated as "no pool + // configured" rather than honored literally (ersatztv#529) + public const int MinimumQsvExtraHardwareFrames = 64; + + public int QsvExtraHardwareFrames => + Math.Max(MaybeQsvExtraHardwareFrames.IfNone(MinimumQsvExtraHardwareFrames), MinimumQsvExtraHardwareFrames); public static FFmpegState Concat(bool saveReport, string channelName) => new( diff --git a/docs/decisions.md b/docs/decisions.md index c057cb3ad..8c8c3235d 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -3207,3 +3207,45 @@ single trailing `SaveChangesAsync`). **Accepted residual:** the SPA can render a not-yet-migrated external-URL logo as an `` preview that looks working while the server-side bug won't resolve until the row is re-saved/migrated — a narrow transitional-state cosmetic mismatch, since the startup migration eagerly converts old rows. + +## 2026-07-21 — QSV hardware-frame headroom is a floor, not an operator preference (#529) + +- **`extra_hw_frames=0` is not a valid pool size; it is a dead channel waiting for an unthrottled + read.** `FFmpegState.QsvExtraHardwareFrames` honored a stored `0` literally, so + `hwupload=extra_hw_frames=0` reached FFmpeg with no headroom for frames in flight through the + filter graph. Measured against the deployed FFmpeg 8.1.2 on one real logged command (software + mpeg4 decode → `hwupload` → `vpp_qsv` → `h264_qsv`): the graph fails with `-12 (Cannot allocate + memory)`, `h264_qsv` reports "Could not open encoder before EOF", and **zero segments are + written**. It now clamps to `MinimumQsvExtraHardwareFrames` (64), which is also the value + `IfNone` already used for an unset profile and the seeded profile default. +- **Input throttling was the only thing hiding it, which is why this looked like a #350 + regression.** Truth table, same command, same binary, only the marked tokens differing: + + | readrate | extra_hw_frames | result | + |---|---|---| + | `1.05`, no burst | 0 | 14 segments, exit 0 | + | `1.05` + burst 2 / 4 / 8 | 0 | **ENOMEM, 0 segments** | + | no readrate at all | 0 | **ENOMEM, 0 segments** | + | `1.05` + burst 8 | 64 | 14 segments, exit 0 | + | no readrate at all | 64 | 14 segments, exit 0 | + + So the defect predates #350's burst: any work-ahead start (which takes no `-readrate`) on a + pipeline that uploads to QSV was already failing on a profile with `0`. The burst did not + introduce it — it removed the throttle on *every* realtime session, converting an intermittent + failure into a near-deterministic one, which is how it finally got noticed. +- **`-readrate` was doing load-bearing work nobody had written down.** Its stated job is live-TV + pacing; it was *also* incidentally bounding how fast decoded frames enter the filter graph. This + is why #350's FFmpeg-level benchmark and #516's argument-generation tests were both green and + neither could see it: the burst is bounded in **seconds of input**, which is not a bound on + **memory or hardware surfaces**. Corrects the #350 entry above, which records the burst as + bounded and safe and does not mention hardware frame pools. +- **A floor, not a clamp-to-default.** Values above 64 are still honored; values below it are + raised. We have positive evidence only for 0 (fails) and 64 (works), so the floor sits at the + product's own default rather than at some smaller value we have not tested, and the failure mode + it prevents is a channel that serves nothing at all. An operator who wants more headroom can + still raise it; there is no legitimate reason to ask for less than the default minimum. +- **Fixed at `FFmpegState.QsvExtraHardwareFrames`, the single point every QSV upload site reads.** + `HardwareUploadFilter`, `HardwareUploadQsvFilter`, `WatermarkHardwareUploadFilter`, + `ScaleQsvFilter`, `DeinterlaceQsvFilter` and `SubtitleScaleQsvFilter` all derive from it (the + last three via values `QsvPipelineBuilder` passes down), so one guard covers them all rather + than six call sites that can drift apart. From b27c95094396ac2deae847d524f1879f7e87e20b Mon Sep 17 00:00:00 2001 From: Timothy Date: Tue, 21 Jul 2026 16:06:35 +0200 Subject: [PATCH 2/3] =?UTF-8?q?fix(529):=20address=20review=20=E2=80=94=20?= =?UTF-8?q?decision=20schema,=20#350=20forward=20pointer,=20warning=20log,?= =?UTF-8?q?=20save-time=20normalization=20[decisions-edit]?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../Commands/CreateFFmpegProfileHandler.cs | 7 +- .../Commands/UpdateFFmpegProfileHandler.cs | 7 +- .../Pipeline/QsvPipelineBuilderTests.cs | 68 ++++++++++++------- .../Pipeline/QsvPipelineBuilder.cs | 12 ++++ docs/decisions.md | 46 ++++++++++--- docs/decisions/README.md | 1 + web/src/screens/FFmpegProfilesScreen.tsx | 14 +++- 7 files changed, 118 insertions(+), 37 deletions(-) diff --git a/ErsatzTV.Application/FFmpegProfiles/Commands/CreateFFmpegProfileHandler.cs b/ErsatzTV.Application/FFmpegProfiles/Commands/CreateFFmpegProfileHandler.cs index 78773685e..eeb49803e 100644 --- a/ErsatzTV.Application/FFmpegProfiles/Commands/CreateFFmpegProfileHandler.cs +++ b/ErsatzTV.Application/FFmpegProfiles/Commands/CreateFFmpegProfileHandler.cs @@ -2,6 +2,7 @@ using ErsatzTV.Core; using ErsatzTV.Core.Domain; using ErsatzTV.Core.Errors; using ErsatzTV.Core.Interfaces.Search; +using ErsatzTV.FFmpeg; using ErsatzTV.Infrastructure.Data; using ErsatzTV.Infrastructure.Extensions; using Microsoft.EntityFrameworkCore; @@ -67,7 +68,11 @@ public class CreateFFmpegProfileHandler : HardwareAcceleration = hwAccel, VaapiDriver = request.VaapiDriver, VaapiDevice = request.VaapiDevice, - QsvExtraHardwareFrames = request.QsvExtraHardwareFrames, + // store what the pipeline will actually use, never a pool size FFmpegState would + // floor away at render time (ersatztv#529) + QsvExtraHardwareFrames = request.QsvExtraHardwareFrames is { } frames + ? Math.Max(frames, FFmpegState.MinimumQsvExtraHardwareFrames) + : null, ResolutionId = resolutionId, ScalingBehavior = request.ScalingBehavior, diff --git a/ErsatzTV.Application/FFmpegProfiles/Commands/UpdateFFmpegProfileHandler.cs b/ErsatzTV.Application/FFmpegProfiles/Commands/UpdateFFmpegProfileHandler.cs index ba056ca97..56417b47f 100644 --- a/ErsatzTV.Application/FFmpegProfiles/Commands/UpdateFFmpegProfileHandler.cs +++ b/ErsatzTV.Application/FFmpegProfiles/Commands/UpdateFFmpegProfileHandler.cs @@ -3,6 +3,7 @@ using ErsatzTV.Core.Domain; using ErsatzTV.Core.Errors; using ErsatzTV.Core.FFmpeg; using ErsatzTV.Core.Interfaces.Search; +using ErsatzTV.FFmpeg; using ErsatzTV.FFmpeg.Preset; using ErsatzTV.Infrastructure.Data; using ErsatzTV.Infrastructure.Extensions; @@ -54,7 +55,11 @@ public class UpdateFFmpegProfileHandler(IDbContextFactory dbContextFa p.VaapiDisplay = update.VaapiDisplay; p.VaapiDriver = update.VaapiDriver; p.VaapiDevice = update.VaapiDevice; - p.QsvExtraHardwareFrames = update.QsvExtraHardwareFrames; + // store what the pipeline will actually use, so a profile doesn't keep displaying a pool + // size that FFmpegState floors away at render time (ersatztv#529) + p.QsvExtraHardwareFrames = update.QsvExtraHardwareFrames is { } frames + ? Math.Max(frames, FFmpegState.MinimumQsvExtraHardwareFrames) + : null; p.ResolutionId = update.ResolutionId; p.ScalingBehavior = update.ScalingBehavior; p.PadMode = update.PadMode; diff --git a/ErsatzTV.FFmpeg.Tests/Pipeline/QsvPipelineBuilderTests.cs b/ErsatzTV.FFmpeg.Tests/Pipeline/QsvPipelineBuilderTests.cs index f9967ea0a..d3af9ee99 100644 --- a/ErsatzTV.FFmpeg.Tests/Pipeline/QsvPipelineBuilderTests.cs +++ b/ErsatzTV.FFmpeg.Tests/Pipeline/QsvPipelineBuilderTests.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Text.RegularExpressions; using ErsatzTV.FFmpeg.Capabilities; using ErsatzTV.FFmpeg.Format; using ErsatzTV.FFmpeg.OutputFormat; @@ -55,26 +56,7 @@ public class QsvPipelineBuilderTests [Test] public void Qsv_PreferNativeDecoder_Interlaced_Should_Hwupload_Before_Deinterlace_Qsv() { - (VideoInputFile videoInputFile, AudioInputFile audioInputFile, FFmpegState ffmpegState, FrameState desiredState) = - BuildQsvH264Pipeline(preferNativeDecoder: true, scanKind: ScanKind.Interlaced, deinterlace: true); - - var builder = new QsvPipelineBuilder( - new DefaultFFmpegCapabilities(), - new DefaultHardwareCapabilities(), - HardwareAccelerationMode.Qsv, - videoInputFile, - audioInputFile, - None, - None, - None, - Option.None, - "", - "", - _logger); - - FFmpegPipeline result = builder.Build(ffmpegState, desiredState); - - string command = PrintCommand(videoInputFile, audioInputFile, None, None, None, result); + string command = BuildInterlacedAndPrint(); // VA-API decode, software frames command.ShouldContain("-hwaccel vaapi"); @@ -94,6 +76,7 @@ public class QsvPipelineBuilderTests // leaves the QSV pool no headroom. Measured on the deployed FFmpeg 8.1.2: with 0 the filter graph // fails with -12 and writes zero segments as soon as the input is not throttled (a work-ahead // start, or #350's cold-start burst); with 64 the same command writes segments either way. + [TestCase(-1)] [TestCase(0)] [TestCase(1)] [TestCase(63)] @@ -101,8 +84,34 @@ public class QsvPipelineBuilderTests { string command = BuildAndPrint(preferNativeDecoder: true, maybeExtraHardwareFrames: configured); - command.ShouldContain($"hwupload=extra_hw_frames={FFmpegState.MinimumQsvExtraHardwareFrames}"); - command.ShouldNotContain("hwupload=extra_hw_frames=0,"); + // pinned to the literal value measured against the deployed FFmpeg, not to the constant, so + // that lowering the floor in code cannot quietly satisfy this test + command.ShouldContain("hwupload=extra_hw_frames=64"); + + // every upload site in the whole command, not just the one this pipeline happens to emit + ShouldNeverUploadBelowMinimum(command); + } + + [Test] + public void Qsv_Interlaced_Should_Never_Deinterlace_Upload_Below_Minimum_Extra_Hardware_Frames() + { + // the deinterlace upload is a separate formatter fed from QsvPipelineBuilder, so it needs + // its own guard (ersatztv#529) + string command = BuildInterlacedAndPrint(maybeExtraHardwareFrames: 0); + + command.ShouldContain("hwupload=extra_hw_frames=64,deinterlace_qsv"); + ShouldNeverUploadBelowMinimum(command); + } + + private static void ShouldNeverUploadBelowMinimum(string command) + { + MatchCollection matches = Regex.Matches(command, @"extra_hw_frames=(-?\d+)"); + matches.Count.ShouldBeGreaterThan(0, "expected the pipeline to upload to QSV at all"); + foreach (Match match in matches) + { + int.Parse(match.Groups[1].Value) + .ShouldBeGreaterThanOrEqualTo(FFmpegState.MinimumQsvExtraHardwareFrames, command); + } } [Test] @@ -113,10 +122,21 @@ public class QsvPipelineBuilderTests command.ShouldContain("hwupload=extra_hw_frames=128"); } - private string BuildAndPrint(bool preferNativeDecoder, Option maybeExtraHardwareFrames = default) + private string BuildInterlacedAndPrint(Option maybeExtraHardwareFrames = default) => + BuildAndPrint( + preferNativeDecoder: true, + maybeExtraHardwareFrames, + ScanKind.Interlaced, + deinterlace: true); + + private string BuildAndPrint( + bool preferNativeDecoder, + Option maybeExtraHardwareFrames = default, + ScanKind scanKind = ScanKind.Progressive, + bool deinterlace = false) { (VideoInputFile videoInputFile, AudioInputFile audioInputFile, FFmpegState ffmpegState, FrameState desiredState) = - BuildQsvH264Pipeline(preferNativeDecoder, ScanKind.Progressive, false); + BuildQsvH264Pipeline(preferNativeDecoder, scanKind, deinterlace); ffmpegState = ffmpegState with { MaybeQsvExtraHardwareFrames = maybeExtraHardwareFrames }; diff --git a/ErsatzTV.FFmpeg/Pipeline/QsvPipelineBuilder.cs b/ErsatzTV.FFmpeg/Pipeline/QsvPipelineBuilder.cs index 6f066c585..88230bfb4 100644 --- a/ErsatzTV.FFmpeg/Pipeline/QsvPipelineBuilder.cs +++ b/ErsatzTV.FFmpeg/Pipeline/QsvPipelineBuilder.cs @@ -62,6 +62,18 @@ public class QsvPipelineBuilder : SoftwarePipelineBuilder PipelineContext context, ICollection pipelineSteps) { + // surface the floor rather than applying it silently: a profile that deliberately asked for + // a smaller pool now gets a larger one, which costs additional surfaces (ersatztv#529) + foreach (int configured in ffmpegState.MaybeQsvExtraHardwareFrames + .Filter(f => f < FFmpegState.MinimumQsvExtraHardwareFrames)) + { + _logger.LogWarning( + "QSV extra hardware frames is configured as {Configured}, which leaves the upload pool too " + + "little headroom and fails transcoding on any unthrottled read; using {Applied} instead", + configured, + FFmpegState.MinimumQsvExtraHardwareFrames); + } + FFmpegCapability decodeCapability = _hardwareCapabilities.CanDecode( videoStream.Codec, videoStream.Profile, diff --git a/docs/decisions.md b/docs/decisions.md index 8c8c3235d..e2af037f3 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -2844,6 +2844,16 @@ again — the duplication is what let the defect exist in triplicate. Covered by **Signals:** HLS cold start, readrate, work_ahead_limit, HlsSessionWorker, FFmpegKnownOption capability gate · paths: `HlsSessionWorker`, `SetRealtimeInput`, `FFmpegPlaybackSettingsCalculator`, `FFmpegKnownOption`/`HasOption` · issues: #350 **Mechanics:** `SetRealtimeInput` readrate-burst option; `FFmpegKnownOption.HasOption` version-capability gate +> **Correction (2026-07-21, #529):** the claim below that the burst is "bounded" is true only in +> *seconds of input* — it is **not** a bound on memory or hardware surfaces. `-readrate` was also +> incidentally bounding how fast decoded frames enter the filter graph, and removing that on a QSV +> pipeline whose profile stores `qsvExtraHardwareFrames: 0` exhausts the upload pool: the graph fails +> with `-12 (Cannot allocate memory)`, `h264_qsv` never opens, and zero segments are written. The +> burst is **not** the root cause (a work-ahead start takes no `-readrate` at all and was already +> failing the same way in production), but it removed the throttle on every realtime session and so +> made the failure near-deterministic. See `ffmpeg.qsv-extra-hw-frames-floor`; the decision recorded +> here still stands, with that floor in place. + - **`-readrate` throttles from the first read, so it sets a floor on time-to-first-segment.** The realtime playback path pins input reading to 1.05× wall clock so a channel behaves like live TV. Since `OutputFormatHls.SegmentSeconds` is 4 and the segmenter serves the playlist only once the @@ -3209,6 +3219,10 @@ that looks working while the server-side bug won't resolve until the row is re-s narrow transitional-state cosmetic mismatch, since the startup migration eagerly converts old rows. ## 2026-07-21 — QSV hardware-frame headroom is a floor, not an operator preference (#529) +`key: ffmpeg.qsv-extra-hw-frames-floor` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none` +**Rule:** a QSV upload never emits `extra_hw_frames` below `FFmpegState.MinimumQsvExtraHardwareFrames` (64); a stored `0` or negative value is treated as "no pool configured" rather than honored literally, because with no headroom any unthrottled read exhausts the pool and the transcode writes nothing at all. +**Signals:** QSV, extra_hw_frames, hwupload, hardware frame pool, ENOMEM, "Could not open encoder before EOF", readrate as an incidental allocation bound · paths: `FFmpegState`, `QsvPipelineBuilder`, `HardwareUploadFilter`, `ScaleQsvFilter`, `DeinterlaceQsvFilter` · issues: #529, #350, #516, #519 +**Mechanics:** `FFmpegState.MinimumQsvExtraHardwareFrames`; `FFmpegState.QsvExtraHardwareFrames`; `QsvPipelineBuilder` logs once when a configured value is raised - **`extra_hw_frames=0` is not a valid pool size; it is a dead channel waiting for an unthrottled read.** `FFmpegState.QsvExtraHardwareFrames` honored a stored `0` literally, so @@ -3239,13 +3253,27 @@ narrow transitional-state cosmetic mismatch, since the startup migration eagerly neither could see it: the burst is bounded in **seconds of input**, which is not a bound on **memory or hardware surfaces**. Corrects the #350 entry above, which records the burst as bounded and safe and does not mention hardware frame pools. -- **A floor, not a clamp-to-default.** Values above 64 are still honored; values below it are - raised. We have positive evidence only for 0 (fails) and 64 (works), so the floor sits at the - product's own default rather than at some smaller value we have not tested, and the failure mode - it prevents is a channel that serves nothing at all. An operator who wants more headroom can - still raise it; there is no legitimate reason to ask for less than the default minimum. +- **A floor, not a clamp-to-default — and the floor is wider than the evidence.** Values above 64 + are honored unchanged; values below it are raised. We measured only `0` (fails) and `64` (works), + so `1..63` are **untested, not known-bad**: we raise them rather than trust them, because the + failure they risk is a channel that serves nothing at all. That is a deliberate over-reach, and + it is not free — `extra_hw_frames` allocates *additional* surfaces (64 NV12 1080p surfaces ≈ + 190 MiB, ≈760 MiB at 4K), so an operator who deliberately set a small pool on a memory-constrained + iGPU silently gets a larger one. `QsvPipelineBuilder` therefore **logs a warning** naming both the + configured and the applied value, so the override is discoverable rather than silent. If a smaller + pool is ever measured safe, lower the floor rather than removing it. - **Fixed at `FFmpegState.QsvExtraHardwareFrames`, the single point every QSV upload site reads.** - `HardwareUploadFilter`, `HardwareUploadQsvFilter`, `WatermarkHardwareUploadFilter`, - `ScaleQsvFilter`, `DeinterlaceQsvFilter` and `SubtitleScaleQsvFilter` all derive from it (the - last three via values `QsvPipelineBuilder` passes down), so one guard covers them all rather - than six call sites that can drift apart. + `HardwareUploadFilter`, `HardwareUploadQsvFilter` and `WatermarkHardwareUploadFilter` read it + directly; `ScaleQsvFilter` and `DeinterlaceQsvFilter` take values `QsvPipelineBuilder` passes down + from it. One guard covers them all rather than five call sites that can drift apart. (A sixth + formatter, `SubtitleScaleQsvFilter`, also emits `extra_hw_frames` but is currently dead code — no + construction site exists in the solution — so it is *not* covered by this guard and would need the + same value threading if it is ever revived.) + +**Accepted residual:** the floor is applied at render time only, so a stored `0` keeps displaying as +`0` in the SPA and over `GET /api/v1/ffmpeg/profiles/{id}` while FFmpeg receives 64 — the config no +longer literally describes the behavior. Render-time was chosen deliberately: it fixes every existing +deployment with no DB edit and no migration, which matters because this bug is already failing +transcodes in production. New and updated profiles are normalized on save so stored rows converge on +the truth, and the warning log closes the discoverability gap for rows that predate it; a backfill +migration for old rows was judged not worth the dual-provider cost. diff --git a/docs/decisions/README.md b/docs/decisions/README.md index f7cfd4c02..e0fa4be63 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -50,6 +50,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `ffmpeg.external-logo-graphics-engine` | External-URL channel logos pass through to the graphics engine like any other watermark source; `WatermarkSelector` must never gate them on `File.Exists` (always false for a URL) and never route them through the ffmpeg-native overlay shortcut. | 2026-07-20 | [link](../decisions.md#2026-07-20--external-url-channel-logos-pass-through-to-the-graphics-engine-never-fileexists-gated-never-ffmpeg-native-502) | | `ffmpeg.hls-cold-start-burst` | HLS cold-start latency is fixed with a bounded `-readrate_initial_burst` (gated on FFmpeg ≥6.1 capability detection), not by raising `work_ahead_limit`, which would remove the concurrency guarantee it exists for. | 2026-07-20 | [link](../decisions.md#2026-07-20--hls-cold-start-is-fixed-with--readrate_initial_burst-not-by-raising-the-work-ahead-limit-350) | | `ffmpeg.qsv-decode-encode-split` | QSV decode is decoupled from QSV encode via a single `FFmpegProfile.QsvPreferNativeDecoder` bool (default ON, Linux-only), so a QSV encode profile can decode with the more tolerant native VA-API decoder instead of the QSV decoder, mirroring Jellyfin's hybrid decode/encode toggle instead of a general decode-family enum. | 2026-07-20 | [link](../decisions.md#2026-07-20-498--qsv-decode-is-split-from-qsv-encode-via-a-single-qsvprefernativedecoder-bool) | +| `ffmpeg.qsv-extra-hw-frames-floor` | a QSV upload never emits `extra_hw_frames` below `FFmpegState.MinimumQsvExtraHardwareFrames` (64); a stored `0` or negative value is treated as "no pool configured" rather than honored literally, because with no headroom any unthrottled read exhausts the pool and the transcode writes nothing at all. | 2026-07-21 | [link](../decisions.md#2026-07-21--qsv-hardware-frame-headroom-is-a-floor-not-an-operator-preference-529) | | `ffmpeg.remote-image-fetcher-bounded` | remote graphics-engine images are fetched through `IRemoteImageFetcher` with a pooled `HttpClientFactory` client, a body-covering deadline, a wire-transfer size cap, and a decoder-enforced `DecoderOptions.MaxFrames` bound re-verified post-decode — never cached, re-fetched per element init. | 2026-07-20 | [link](../decisions.md#2026-07-20--remote-graphics-engine-images-are-fetched-through-a-bounded-pooled-iremoteimagefetcher-re-fetched-per-element-init-not-cached-511) | | `graphics.channel-logo-caching` | An external `http(s)` channel-logo URL is fetched, decode-budget-validated, and stored in the image cache under a content-hash name at SAVE time — becoming byte-identical to an uploaded logo — so the render path never fetches a logo over HTTP; a bad URL fails the save with a 422 (BaseError → ValidationProblemDetails). | 2026-07-21 | [link](../decisions.md#2026-07-21--external-channel-logo-urls-are-downloaded-and-cached-at-save-time-the-render-path-never-fetches-a-logo-525) | | `iptv.base-url` | An optional advertised base URL (`iptv.base_url`) is resolved centrally via a pure Core helper (`AdvertisedBaseUrl`) inside the two IPTV generation handlers (M3U + XMLTV); unset/malformed values fall back byte-identical to the request-derived host, and it's a new `iptv` settings group distinct from `ETV_BASE_URL` and out of scope for HDHomeRun. | 2026-07-16 | [link](../decisions.md#2026-07-16--optional-advertised-iptv-base-url-iptvbase_url-resolved-centrally-in-the-two-generators-340) | diff --git a/web/src/screens/FFmpegProfilesScreen.tsx b/web/src/screens/FFmpegProfilesScreen.tsx index 59ec46f31..35135947c 100644 --- a/web/src/screens/FFmpegProfilesScreen.tsx +++ b/web/src/screens/FFmpegProfilesScreen.tsx @@ -20,6 +20,10 @@ import { const BASE_PATH = '/app/ffmpeg-profiles'; +// mirrors FFmpegState.MinimumQsvExtraHardwareFrames — the server floors any smaller value, so the +// form must not offer one it would silently override (ersatztv#529) +const MINIMUM_QSV_EXTRA_HARDWARE_FRAMES = 64; + type Draft = CreateFFmpegProfileRequest; /* ---------- enum option lists (mirror ErsatzTV/Pages/FFmpegEditor.razor) ---------- */ @@ -207,7 +211,7 @@ function defaultDraft(resolutionId: number): Draft { normalizeLoudnessMode: 'Off', normalizeVideo: true, padMode: 'Software', - qsvExtraHardwareFrames: 64, + qsvExtraHardwareFrames: MINIMUM_QSV_EXTRA_HARDWARE_FRAMES, qsvPreferNativeDecoder: true, resolutionId, scalingBehavior: 'ScaleAndPad', @@ -291,11 +295,13 @@ function Row({ function NumberField({ disabled, + min, onChange, unit, value }: { disabled?: boolean; + min?: number; onChange: (next: number) => void; unit?: string; value: number; @@ -303,6 +309,7 @@ function NumberField({ return ( onChange(Number(event.target.value))} size="sm" trailing={unit} @@ -769,9 +776,12 @@ function ProfileEditor({ mode }: { mode: { kind: 'add'; from: null | number } | )} {draft.hardwareAcceleration === 'Qsv' ? ( + {/* below this the QSV upload pool has too little headroom and transcoding fails on + any unthrottled read; the server floors it anyway (ersatztv#529) */} set({ qsvExtraHardwareFrames })} - value={draft.qsvExtraHardwareFrames ?? 0} + value={draft.qsvExtraHardwareFrames ?? MINIMUM_QSV_EXTRA_HARDWARE_FRAMES} /> ) : ( From 11b78bfcbda7b9e7fe4082372ac3c9177883f2b5 Mon Sep 17 00:00:00 2001 From: Timothy Date: Tue, 21 Jul 2026 16:21:18 +0200 Subject: [PATCH 3/3] =?UTF-8?q?fix(529):=20round-two=20review=20=E2=80=94?= =?UTF-8?q?=20cover=20the=20handlers=20with=20tests,=20validate=20in=20the?= =?UTF-8?q?=20SPA=20[decisions-edit]?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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
— 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. --- .../Pipeline/QsvPipelineBuilder.cs | 5 +- .../FFmpegProfileHandlerTests.cs | 74 ++++++++++++++++++- docs/decisions.md | 7 +- web/src/screens/FFmpegProfilesScreen.tsx | 10 +++ 4 files changed, 90 insertions(+), 6 deletions(-) diff --git a/ErsatzTV.FFmpeg/Pipeline/QsvPipelineBuilder.cs b/ErsatzTV.FFmpeg/Pipeline/QsvPipelineBuilder.cs index 88230bfb4..1b295e798 100644 --- a/ErsatzTV.FFmpeg/Pipeline/QsvPipelineBuilder.cs +++ b/ErsatzTV.FFmpeg/Pipeline/QsvPipelineBuilder.cs @@ -67,9 +67,12 @@ public class QsvPipelineBuilder : SoftwarePipelineBuilder foreach (int configured in ffmpegState.MaybeQsvExtraHardwareFrames .Filter(f => f < FFmpegState.MinimumQsvExtraHardwareFrames)) { + // this fires per pipeline build, before we know whether this particular pipeline uploads + // at all — a fully-hardware path may consume neither value — so word it conditionally _logger.LogWarning( "QSV extra hardware frames is configured as {Configured}, which leaves the upload pool too " - + "little headroom and fails transcoding on any unthrottled read; using {Applied} instead", + + "little headroom and fails transcoding on any unthrottled read; will use {Applied} " + + "wherever frames are uploaded", configured, FFmpegState.MinimumQsvExtraHardwareFrames); } diff --git a/ErsatzTV.Tests/Application/FFmpegProfiles/FFmpegProfileHandlerTests.cs b/ErsatzTV.Tests/Application/FFmpegProfiles/FFmpegProfileHandlerTests.cs index 0e110a1c3..dbbe3aa2e 100644 --- a/ErsatzTV.Tests/Application/FFmpegProfiles/FFmpegProfileHandlerTests.cs +++ b/ErsatzTV.Tests/Application/FFmpegProfiles/FFmpegProfileHandlerTests.cs @@ -134,6 +134,68 @@ public class FFmpegProfileHandlerTests persisted.QsvPreferNativeDecoder.ShouldBe(false); } + // ersatztv#529: a stored 0 reached ffmpeg as hwupload=extra_hw_frames=0, leaving the QSV pool no + // headroom; FFmpegState floors it at render time, and these pin that the stored row converges too + // so the profile never keeps displaying a value the pipeline would override. + [TestCase(0, 64)] + [TestCase(-8, 64)] + [TestCase(63, 64)] + [TestCase(64, 64)] + [TestCase(128, 128)] + public async Task Create_Should_Floor_QsvExtraHardwareFrames(int configured, int expected) + { + await SeedResolution(1); + var handler = new CreateFFmpegProfileHandler(_db.Factory, _searchTargets); + + Either result = await handler.Handle( + MakeCreate(1, qsvExtraHardwareFrames: configured), + CancellationToken.None); + + CreateFFmpegProfileResult created = RightOf(result); + + await using TvContext context = _db.CreateContext(); + FFmpegProfile persisted = await context.FFmpegProfiles.FindAsync(created.FFmpegProfileId); + persisted.QsvExtraHardwareFrames.ShouldBe(expected); + } + + [TestCase(0, 64)] + [TestCase(-8, 64)] + [TestCase(128, 128)] + public async Task Update_Should_Floor_QsvExtraHardwareFrames(int configured, int expected) + { + await SeedProfile(1); + await SeedResolution(1); + var handler = new UpdateFFmpegProfileHandler(_db.Factory, _searchTargets); + + Either result = await handler.Handle( + MakeUpdate(1, qsvExtraHardwareFrames: configured), + CancellationToken.None); + + RightOf(result); + + await using TvContext context = _db.CreateContext(); + FFmpegProfile persisted = await context.FFmpegProfiles.FindAsync(1); + persisted.QsvExtraHardwareFrames.ShouldBe(expected); + } + + // null means "unconfigured" and FFmpegState already resolves it to the same 64; it must stay + // null rather than being silently materialized into a stored value + [Test] + public async Task Create_Should_Leave_Null_QsvExtraHardwareFrames_Null() + { + await SeedResolution(1); + var handler = new CreateFFmpegProfileHandler(_db.Factory, _searchTargets); + + Either result = + await handler.Handle(MakeCreate(1), CancellationToken.None); + + CreateFFmpegProfileResult created = RightOf(result); + + await using TvContext context = _db.CreateContext(); + FFmpegProfile persisted = await context.FFmpegProfiles.FindAsync(created.FFmpegProfileId); + persisted.QsvExtraHardwareFrames.ShouldBeNull(); + } + private static TR RightOf(Either either) => either.Match(Left: e => throw new AssertionException($"Expected a Right result, got {e}"), Right: r => r); @@ -184,7 +246,10 @@ public class FFmpegProfileHandlerTests await context.SaveChangesAsync(); } - private static CreateFFmpegProfile MakeCreate(int resolutionId, bool qsvPreferNativeDecoder = true) => + private static CreateFFmpegProfile MakeCreate( + int resolutionId, + bool qsvPreferNativeDecoder = true, + int? qsvExtraHardwareFrames = null) => new( "Default", 1, @@ -194,7 +259,7 @@ public class FFmpegProfileHandlerTests "drm", VaapiDriver.Default, "/dev/dri/renderD128", - null, + qsvExtraHardwareFrames, resolutionId, ScalingBehavior.ScaleAndPad, FilterMode.Software, @@ -221,7 +286,8 @@ public class FFmpegProfileHandlerTests private static UpdateFFmpegProfile MakeUpdate( int id, int resolutionId = 1, - bool qsvPreferNativeDecoder = true) => + bool qsvPreferNativeDecoder = true, + int? qsvExtraHardwareFrames = null) => new( id, "Default", @@ -232,7 +298,7 @@ public class FFmpegProfileHandlerTests "drm", VaapiDriver.Default, "/dev/dri/renderD128", - null, + qsvExtraHardwareFrames, resolutionId, ScalingBehavior.ScaleAndPad, FilterMode.Software, diff --git a/docs/decisions.md b/docs/decisions.md index e2af037f3..b31044e22 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -3276,4 +3276,9 @@ longer literally describes the behavior. Render-time was chosen deliberately: it deployment with no DB edit and no migration, which matters because this bug is already failing transcodes in production. New and updated profiles are normalized on save so stored rows converge on the truth, and the warning log closes the discoverability gap for rows that predate it; a backfill -migration for old rows was judged not worth the dual-provider cost. +migration for old rows was judged not worth the dual-provider cost. Two consequences to know about: +the save-time normalization is **unconditional on `hardwareAcceleration`**, so a non-QSV profile's +stored value moves too (harmless — only the QSV path ever reads it — but it is a stored-state change +on a field the user didn't touch); and a machine client that `PUT`s `0` gets a `200` and then reads +back `64`, which is a silent transform of a submitted value that the OpenAPI description does not +advertise. diff --git a/web/src/screens/FFmpegProfilesScreen.tsx b/web/src/screens/FFmpegProfilesScreen.tsx index 35135947c..a84502895 100644 --- a/web/src/screens/FFmpegProfilesScreen.tsx +++ b/web/src/screens/FFmpegProfilesScreen.tsx @@ -190,6 +190,16 @@ function validate(draft: Draft): null | string { } } + // the `min` on the input only bounds the spinner arrows — a typed value still submits, and the + // server would floor it silently with a 200 (ersatztv#529) + if ( + draft.hardwareAcceleration === 'Qsv' && + draft.qsvExtraHardwareFrames != null && + draft.qsvExtraHardwareFrames < MINIMUM_QSV_EXTRA_HARDWARE_FRAMES + ) { + return `QSV extra hardware frames must be at least ${MINIMUM_QSV_EXTRA_HARDWARE_FRAMES}`; + } + return null; }