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.
This commit is contained in:
@@ -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<int> 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(),
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -3207,3 +3207,45 @@ single trailing `SaveChangesAsync`).
|
||||
**Accepted residual:** the SPA can render a not-yet-migrated external-URL logo as an `<img>` 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.
|
||||
|
||||
Reference in New Issue
Block a user