docs(498): fold Fable review fixes into plan + spec
- Blocker 1: VA-API decode to SOFTWARE frames (drop -hwaccel_output_format, new DecoderVaapiToSoftware) so the proven hwupload/vpp_qsv branch bridges to the QSV encoder — the naive hardware-surface path emits a bare vpp_qsv on VA-API frames and fails on ~all content. - Blocker 2: bool? domain property + != false coercion (DeinterlaceVideo pattern) so create-with-false actually persists false. - High 3: REST DTOs bool?=null + ?? true for /api/v1 additive-compat. - Medium 4: correct Task 4 test scaffolding (DefaultHardwareCapabilities). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -12,6 +12,8 @@
|
||||
|
||||
- **Field name (verbatim):** `QsvPreferNativeDecoder` (C#), `qsvPreferNativeDecoder` (TS/JSON).
|
||||
- **Default: ON.** New column has store default `true`; existing rows backfill to `true` (set via `HasDefaultValue(true)` in EF config so `add-migration.sh` emits `defaultValue: true` and keeps snapshots consistent). Domain `New()` sets `= true`. SPA `defaultDraft` sets `true`.
|
||||
- **The domain property is `bool?` (nullable), NOT `bool`** — this is the repo's `DeinterlaceVideo` pattern (`FFmpegProfile.cs:38`, `FFmpegProfileConfiguration.cs:17`). Reason (EF Core gotcha): with a non-nullable `bool` + a store default, EF treats the CLR-default `false` as "not set" and omits it from INSERT, so a user creating a profile with the box **unchecked** would silently persist `true` — defeating the OFF escape hatch. `bool?` makes an explicit `false` distinct from the unset sentinel. Coerce **null-means-ON** everywhere the effective value is read: use `!= false` (null→true, true→true, false→false), NEVER `== true` (which would read null as OFF).
|
||||
- **REST request DTOs use `bool? QsvPreferNativeDecoder = null`** (additive-compat): `/api/v1` is additive-only with real machine clients; an old client omitting the key must NOT flip the feature off. `ToCommand()` passes `QsvPreferNativeDecoder ?? true` so omission preserves default-ON, explicit `false` is honored. The MediatR command + ViewModel stay plain `bool` (resolved values).
|
||||
- **QSV-only:** the flag has effect only when acceleration is QSV; ignored everywhere else. UI row renders only when `hardwareAcceleration === 'Qsv'`.
|
||||
- **UI copy (verbatim):** label `Prefer native decoder`; help `Decode with the more error-tolerant VA-API decoder instead of QSV, while still encoding with QSV. Recommended on Intel — handles imperfect streams and is required for Dolby Vision.`
|
||||
- **`FFmpegState` change is a trailing defaulted parameter** `bool QsvPreferNativeDecoder = false` — appended after `IsTroubleshooting` so all existing `new FFmpegState(...)` callers (tests, `Concat`, error-loop sites) keep compiling with `false`.
|
||||
@@ -32,12 +34,12 @@
|
||||
- Create (generated): `ErsatzTV.Infrastructure.Sqlite/Migrations/*_Add_FFmpegProfile_QsvPreferNativeDecoder.cs`, `ErsatzTV.Infrastructure.MySql/Migrations/*_Add_FFmpegProfile_QsvPreferNativeDecoder.cs`, + both `TvContextModelSnapshot.cs`
|
||||
|
||||
**Interfaces:**
|
||||
- Produces: `FFmpegProfile.QsvPreferNativeDecoder` (bool) — consumed by Tasks 4, 6, 7.
|
||||
- Produces: `FFmpegProfile.QsvPreferNativeDecoder` (`bool?`, null-means-ON) — consumed by Tasks 4, 6, 7.
|
||||
|
||||
- [ ] **Step 1: Add the domain property.** In `ErsatzTV.Core/Domain/FFmpegProfile.cs`, after line 16 (`public int? QsvExtraHardwareFrames { get; set; }`) add:
|
||||
- [ ] **Step 1: Add the domain property.** In `ErsatzTV.Core/Domain/FFmpegProfile.cs`, after line 16 (`public int? QsvExtraHardwareFrames { get; set; }`) add (nullable — see Global Constraints):
|
||||
|
||||
```csharp
|
||||
public bool QsvPreferNativeDecoder { get; set; }
|
||||
public bool? QsvPreferNativeDecoder { get; set; }
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Default it on for new profiles.** In `FFmpegProfile.New()` (after `QsvExtraHardwareFrames = 64,` at L65) add:
|
||||
@@ -126,25 +128,28 @@ git -c core.hooksPath=/dev/null commit -m "feat(498): thread QsvPreferNativeDeco
|
||||
- Consumes: nothing new.
|
||||
- Produces: `new QsvHardwareAccelerationOption(Option<string> device, FFmpegCapability decodeCapability, bool preferNativeDecoder = false)` — a 3rd optional ctor arg. Consumed by Task 4.
|
||||
|
||||
- [ ] **Step 1: Write the failing test.** Append to `QsvHardwareAccelerationOptionTests.cs` (before the closing brace):
|
||||
- [ ] **Step 1: Write the failing test.** Append to `QsvHardwareAccelerationOptionTests.cs` (before the closing brace). NOTE: the native path emits `-hwaccel vaapi` **without** `-hwaccel_output_format vaapi` — omitting the output format makes ffmpeg download the VA-API-decoded frames to system memory, which routes them through the QSV builder's proven `format=nv12,hwupload,vpp_qsv` filter branch (see Task 4 design note). Emitting `-hwaccel_output_format vaapi` would keep frames as VA-API surfaces and break the QSV filter graph.
|
||||
|
||||
```csharp
|
||||
[Test]
|
||||
public void GlobalOptions_WithHardwareDecode_AndPreferNative_ShouldUseVaapiDecode()
|
||||
public void GlobalOptions_WithHardwareDecode_AndPreferNative_ShouldUseVaapiDecodeToSoftware()
|
||||
{
|
||||
var option = new QsvHardwareAccelerationOption("/dev/dri/renderD128", FFmpegCapability.Hardware, preferNativeDecoder: true);
|
||||
|
||||
option.GlobalOptions.ShouldBe(
|
||||
[
|
||||
"-hwaccel", "vaapi",
|
||||
"-hwaccel_output_format", "vaapi",
|
||||
"-init_hw_device", "vaapi=va:/dev/dri/renderD128",
|
||||
"-init_hw_device", "qsv=hw@va",
|
||||
"-filter_hw_device", "hw"
|
||||
]);
|
||||
// must NOT keep frames on the GPU as VA-API surfaces
|
||||
option.GlobalOptions.ShouldNotContain("-hwaccel_output_format");
|
||||
}
|
||||
```
|
||||
|
||||
Hardware-verify note (not blocking the arg test): if on the Intel host ffmpeg fails to bind `-hwaccel vaapi` to the `va` device, add `-hwaccel_device va` after `-hwaccel vaapi` in the native branch. The device is already created by `-init_hw_device vaapi=va:<dev>`.
|
||||
|
||||
- [ ] **Step 2: Run the test to verify it fails.**
|
||||
|
||||
Run: `dotnet test ErsatzTV.FFmpeg.Tests/ErsatzTV.FFmpeg.Tests.csproj --filter FullyQualifiedName~QsvHardwareAccelerationOptionTests.GlobalOptions_WithHardwareDecode_AndPreferNative_ShouldUseVaapiDecode`
|
||||
@@ -178,11 +183,13 @@ public class QsvHardwareAccelerationOption(
|
||||
|
||||
if (decodeCapability is FFmpegCapability.Hardware)
|
||||
{
|
||||
// decode with the error-tolerant native VA-API decoder, but keep the QSV
|
||||
// encoder; otherwise decode (and encode) via QSV
|
||||
// native path: decode with the error-tolerant VA-API decoder and let ffmpeg
|
||||
// download frames to system memory (no -hwaccel_output_format), so the QSV
|
||||
// filter graph's software->hwupload branch bridges them to the QSV encoder.
|
||||
// default path: decode (and keep frames) on QSV.
|
||||
result.AddRange(
|
||||
preferNativeDecoder
|
||||
? ["-hwaccel", "vaapi", "-hwaccel_output_format", "vaapi"]
|
||||
? ["-hwaccel", "vaapi"]
|
||||
: ["-hwaccel", "qsv", "-hwaccel_output_format", "qsv"]);
|
||||
}
|
||||
|
||||
@@ -247,77 +254,127 @@ git -c core.hooksPath=/dev/null commit -m "feat(498): QsvHardwareAccelerationOpt
|
||||
|
||||
---
|
||||
|
||||
### Task 4: `QsvPipelineBuilder` — select VA-API decoder + pass the flag (arg-level test)
|
||||
### Task 4: `QsvPipelineBuilder` — VA-API-decode-to-software + pass the flag (arg-level test)
|
||||
|
||||
**Files:**
|
||||
- Create: `ErsatzTV.FFmpeg/Decoder/DecoderVaapiToSoftware.cs`
|
||||
- Modify: `ErsatzTV.FFmpeg/Pipeline/QsvPipelineBuilder.cs` (`SetAccelState` L105/L108-116; `SetDecoder` L125-135)
|
||||
- Test: `ErsatzTV.FFmpeg.Tests/Pipeline/QsvPipelineBuilderTests.cs` (create if absent)
|
||||
- Test: `ErsatzTV.FFmpeg.Tests/Pipeline/QsvPipelineBuilderTests.cs` (create)
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `FFmpegState.QsvPreferNativeDecoder` (Task 2); `QsvHardwareAccelerationOption(..., preferNativeDecoder)` (Task 3); existing `DecoderVaapi` (`ErsatzTV.FFmpeg/Decoder/Vaapi/DecoderVaapi.cs`, `Name => "implicit_vaapi"`).
|
||||
- Produces: a QSV pipeline whose decode side is VA-API when the flag is set.
|
||||
- Consumes: `FFmpegState.QsvPreferNativeDecoder` (`bool?`, Task 2 — coerce with `!= false`); `QsvHardwareAccelerationOption(..., preferNativeDecoder)` (Task 3).
|
||||
- Produces: `DecoderVaapiToSoftware` (software-output implicit VA-API decoder); a QSV pipeline whose decode side is VA-API-to-software when the flag is set.
|
||||
|
||||
**Design note (read before implementing):** When `QsvPreferNativeDecoder` is true and decode is hardware-capable, we (a) pass the flag into `QsvHardwareAccelerationOption` so the global options emit `-hwaccel vaapi -hwaccel_output_format vaapi`, and (b) set `DecoderHardwareAccelerationMode = HardwareAccelerationMode.Vaapi` so `SetDecoder` returns the generic `DecoderVaapi`. The encoder mode stays `Qsv`. The QSV builder already inserts a `HardwareDownloadFilter` after filtering (`QsvPipelineBuilder.cs:204`) and re-uploads for the QSV encoder via `SetPixelFormat`, so the decoded VA-API surfaces are bridged to the QSV encoder through the existing software round-trip — **provided no QSV-only hardware filter runs on the VA-API frames in between.** The arg test below pins the decode + device + encoder shape; full hardware-filter-graph correctness (scale/tonemap on VA-API-then-QSV) is the **empirical spot** validated on Intel hardware post-deploy (Global Constraints → Verification bar). If the arg test reveals a QSV hardware filter emitted onto VA-API frames, force software scaling for the native-decode path (set `PadMode`/filter mode to software in this branch) — the plain VA-API-decode + software-filter + QSV-encode path is the safe baseline Jellyfin also falls back to.
|
||||
**Design note (READ — Fable review corrected the original approach):** The naive approach (set `DecoderHardwareAccelerationMode = Vaapi` with `-hwaccel_output_format vaapi`, reuse `DecoderVaapi`) is **BROKEN**: `DecoderVaapi` outputs *hardware* VA-API surfaces, and `QsvPipelineBuilder.SetScale` unconditionally chooses `ScaleQsvFilter` when the encoder is QSV — which, on hardware frames, emits a **bare `vpp_qsv`** (even for same-resolution content, via the "still need pixel format" branch at `ScaleQsvFilter.cs:41-47`) directly onto VA-API frames. ffmpeg does NOT auto-`hwmap` between hw contexts → the graph fails to configure on ~all content. `DeinterlaceQsvFilter` fails the same way on interlaced content. The `HardwareDownloadFilter` at `QsvPipelineBuilder.cs:204` runs *after* scale/deinterlace, too late to save it.
|
||||
|
||||
- [ ] **Step 1: Write the failing arg-level test.** Create `ErsatzTV.FFmpeg.Tests/Pipeline/QsvPipelineBuilderTests.cs`. Model it on `PipelineBuilderBaseTests` (same usings, `DefaultFFmpegCapabilities`, `PrintCommand` pattern), but instantiate `QsvPipelineBuilder` with an H264 input and an `FFmpegState` whose `EncoderHardwareAccelerationMode = Qsv`, `DecoderHardwareAccelerationMode = Qsv`, `VaapiDevice = "/dev/dri/renderD128"`, and `QsvPreferNativeDecoder = true`. Assert the generated command contains VA-API decode + the derived-device chain + a QSV encoder:
|
||||
**Correct approach — produce SOFTWARE frames after VA-API decode.** Emit `-hwaccel vaapi` WITHOUT `-hwaccel_output_format` (Task 3) so ffmpeg downloads decoded frames to system memory, and use a decoder whose `OutputFrameDataLocation` is `Software`. Then every QSV filter takes its proven software branch: `ScaleQsvFilter` → `format=nv12,hwupload=extra_hw_frames=N,vpp_qsv` (`ScaleQsvFilter.cs:83-86`); `DeinterlaceQsvFilter` → `format=nv12,hwupload=…,deinterlace_qsv` (`DeinterlaceQsvFilter.cs:17-18`); `SetTonemap` picks the software `TonemapFilter` because the decoder mode is not `Qsv` (`QsvPipelineBuilder.cs:705`). This is **exactly the graph the production error-loop paths already run** (`FFmpegLibraryProcessService.cs:782/923` — software decode + QSV encode), just with VA-API-accelerated tolerant decode instead of a CPU decoder. Trade-off (state honestly in the PR): a PCIe download/upload round-trip, NOT Jellyfin's zero-copy `hwmap=derive_device=qsv` (that filter does not exist in this codebase and is out of scope). We keep the QSV *encoder* and the tolerant *decoder*, which is the whole point of #498.
|
||||
|
||||
- [ ] **Step 1: Create the software-output VA-API decoder.** Create `ErsatzTV.FFmpeg/Decoder/DecoderVaapiToSoftware.cs`:
|
||||
|
||||
```csharp
|
||||
using ErsatzTV.FFmpeg.Format;
|
||||
|
||||
namespace ErsatzTV.FFmpeg.Decoder;
|
||||
|
||||
// VA-API-accelerated decode that downloads frames to system memory (no
|
||||
// -hwaccel_output_format). Pairs with `-hwaccel vaapi` from
|
||||
// QsvHardwareAccelerationOption on the "prefer native decoder" QSV path: the
|
||||
// error-tolerant VA-API decoder feeds software frames into the QSV builder's
|
||||
// format=nv12,hwupload,vpp_qsv branch, which re-uploads for the QSV encoder.
|
||||
public class DecoderVaapiToSoftware : DecoderBase
|
||||
{
|
||||
protected override FrameDataLocation OutputFrameDataLocation => FrameDataLocation.Software;
|
||||
|
||||
public override string Name => "implicit_vaapi";
|
||||
|
||||
// no -c:v (implicit decoder; `-hwaccel vaapi` drives VA-API) and no
|
||||
// -hwaccel_output_format (frames download to software)
|
||||
public override string[] InputOptions(InputFile inputFile) => [];
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Write the failing arg-level tests.** Create `ErsatzTV.FFmpeg.Tests/Pipeline/QsvPipelineBuilderTests.cs`. Model it on `PipelineBuilderBaseTests` (copy the usings, the nested `DefaultFFmpegCapabilities` class, and the 6-arg `PrintCommand` helper from `PipelineBuilderBaseTests.cs:534-565` verbatim). Add helper `BuildQsvH264Pipeline(bool preferNativeDecoder)` that builds a single 8-bit H264 `VideoInputFile` (1920x1080, `PixelFormatYuv420P`, `ScanKind.Progressive`), an `AudioInputFile` (Aac, 2ch), a `FrameState desiredState` targeting `VideoFormat.H264` / `FFmpegFilterMode.Software`, and an `FFmpegState` with `DecoderHardwareAccelerationMode = HardwareAccelerationMode.Qsv`, `EncoderHardwareAccelerationMode = HardwareAccelerationMode.Qsv`, `VaapiDevice = "/dev/dri/renderD128"`, `QsvPreferNativeDecoder: preferNativeDecoder` (pass as the trailing arg). Build with:
|
||||
|
||||
```csharp
|
||||
var builder = new QsvPipelineBuilder(
|
||||
new DefaultFFmpegCapabilities(), // IFFmpegCapabilities (1st arg)
|
||||
new DefaultHardwareCapabilities(), // IHardwareCapabilities (2nd arg) — CanDecode/CanEncode => Hardware for 8-bit H264
|
||||
videoInputFile,
|
||||
audioInputFile,
|
||||
None, // watermark
|
||||
None, // subtitle
|
||||
None, // concat
|
||||
Option<GraphicsEngineInput>.None,
|
||||
"", // reports folder
|
||||
"", // fonts folder
|
||||
_logger);
|
||||
FFmpegPipeline result = builder.Build(ffmpegState, desiredState);
|
||||
```
|
||||
|
||||
(`DefaultHardwareCapabilities` is `ErsatzTV.FFmpeg/Capabilities/DefaultHardwareCapabilities.cs`; add `using ErsatzTV.FFmpeg.Capabilities;`. The 6-arg `PrintCommand(videoInputFile, audioInputFile, None, None, None, result)` returns the joined command string.) Assertions:
|
||||
|
||||
```csharp
|
||||
[Test]
|
||||
public void Qsv_PreferNativeDecoder_Should_Decode_With_Vaapi_And_Encode_With_Qsv()
|
||||
public void Qsv_PreferNativeDecoder_Should_Decode_Via_Vaapi_To_Software_Then_Qsv_Encode()
|
||||
{
|
||||
FFmpegPipeline result = BuildQsvH264Pipeline(preferNativeDecoder: true);
|
||||
string command = PrintCommand(result);
|
||||
string command = BuildAndPrint(preferNativeDecoder: true);
|
||||
|
||||
// VA-API decode, frames downloaded to software (NO hwaccel_output_format)
|
||||
command.ShouldContain("-hwaccel vaapi");
|
||||
command.ShouldContain("-hwaccel_output_format vaapi");
|
||||
command.ShouldNotContain("-hwaccel_output_format");
|
||||
command.ShouldNotContain("-hwaccel qsv");
|
||||
command.ShouldNotContain("-c:v h264_qsv -"); // no QSV *decoder* input option
|
||||
// derived-device chain retained for the QSV encoder
|
||||
command.ShouldContain("-init_hw_device vaapi=va:/dev/dri/renderD128");
|
||||
command.ShouldContain("-init_hw_device qsv=hw@va");
|
||||
command.ShouldContain("-c:v h264_qsv");
|
||||
command.ShouldNotContain("-hwaccel qsv");
|
||||
// software frames re-uploaded before QSV filters/encoder (proves NO bare vpp_qsv on VA-API frames)
|
||||
command.ShouldContain("hwupload=extra_hw_frames");
|
||||
// QSV encoder still used
|
||||
command.ShouldContain("h264_qsv");
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Qsv_Default_Should_Decode_And_Encode_With_Qsv()
|
||||
{
|
||||
FFmpegPipeline result = BuildQsvH264Pipeline(preferNativeDecoder: false);
|
||||
string command = PrintCommand(result);
|
||||
string command = BuildAndPrint(preferNativeDecoder: false);
|
||||
|
||||
command.ShouldContain("-hwaccel qsv");
|
||||
command.ShouldContain("-c:v h264_qsv");
|
||||
command.ShouldContain("-hwaccel_output_format qsv");
|
||||
command.ShouldContain("h264_qsv");
|
||||
command.ShouldNotContain("-hwaccel vaapi");
|
||||
}
|
||||
```
|
||||
|
||||
Implement `BuildQsvH264Pipeline(bool preferNativeDecoder)` and `PrintCommand(FFmpegPipeline)` as private helpers in the fixture: build a single H264 `VideoInputFile` (1920x1080, `PixelFormatYuv420P`), a `FrameState desiredState` targeting `VideoFormat.H264` / `FFmpegFilterMode.Software`, and an `FFmpegState` as described (use `HardwareAccelerationMode.Qsv` for both decode+encode, `VaapiDevice = "/dev/dri/renderD128"`, `QsvPreferNativeDecoder: preferNativeDecoder`). Copy the exact `PrintCommand` helper + `DefaultFFmpegCapabilities` from `PipelineBuilderBaseTests.cs:534-565`. Construct `QsvPipelineBuilder` with `new DefaultFFmpegCapabilities()` as the hardware-capabilities arg (its `CanDecode`/`CanEncode` return `Hardware`, so the QSV decode/encode branches are exercised) and the same remaining ctor args the other builders use (audio input, `None` watermark/subtitle/concat/graphics, `""` reports/fonts folders, `_logger`).
|
||||
(where `BuildAndPrint` = `PrintCommand(...)` over `BuildQsvH264Pipeline(...)`.) Optionally add a third test with an interlaced input (`ScanKind.Interlaced` + a context that deinterlaces) asserting the native command contains `hwupload=extra_hw_frames` followed by `deinterlace_qsv` (never a bare `deinterlace_qsv` on VA-API frames) — add it if the `PipelineContext.ShouldDeinterlace` setup is tractable; otherwise note it as a manual hardware check.
|
||||
|
||||
- [ ] **Step 2: Run to verify it fails.**
|
||||
- [ ] **Step 3: Run to verify it fails.**
|
||||
|
||||
Run: `dotnet test ErsatzTV.FFmpeg.Tests/ErsatzTV.FFmpeg.Tests.csproj --filter FullyQualifiedName~QsvPipelineBuilderTests`
|
||||
Expected: FAIL — the `preferNativeDecoder=true` case still emits `-hwaccel qsv` and a `*_qsv` decoder.
|
||||
Expected: FAIL — the native case still emits `-hwaccel qsv`.
|
||||
|
||||
- [ ] **Step 3: Pass the flag into the accel option.** In `QsvPipelineBuilder.SetAccelState`, change line 105 from:
|
||||
- [ ] **Step 4: Pass the flag into the accel option.** In `QsvPipelineBuilder.SetAccelState`, change line 105 from:
|
||||
|
||||
```csharp
|
||||
pipelineSteps.Add(new QsvHardwareAccelerationOption(ffmpegState.VaapiDevice, decodeCapability));
|
||||
```
|
||||
|
||||
to:
|
||||
to (coerce the nullable flag with `!= false` — null means ON):
|
||||
|
||||
```csharp
|
||||
pipelineSteps.Add(new QsvHardwareAccelerationOption(
|
||||
ffmpegState.VaapiDevice,
|
||||
decodeCapability,
|
||||
ffmpegState.QsvPreferNativeDecoder));
|
||||
ffmpegState.QsvPreferNativeDecoder != false));
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Set the decoder mode to VA-API for the native path.** In the same method, change the `return ffmpegState with { … }` block (L108-116) so the decoder mode is VA-API when preferring native decode and decode is hardware-capable:
|
||||
- [ ] **Step 5: Set the decoder mode to VA-API for the native path.** In the same method, change the `return ffmpegState with { … }` block (L108-116) so the decoder mode is `Vaapi` when preferring native decode and decode is hardware-capable:
|
||||
|
||||
```csharp
|
||||
// disable hw accel if decoder/encoder isn't supported
|
||||
return ffmpegState with
|
||||
{
|
||||
DecoderHardwareAccelerationMode = decodeCapability == FFmpegCapability.Hardware
|
||||
? ffmpegState.QsvPreferNativeDecoder
|
||||
? ffmpegState.QsvPreferNativeDecoder != false
|
||||
? HardwareAccelerationMode.Vaapi
|
||||
: HardwareAccelerationMode.Qsv
|
||||
: HardwareAccelerationMode.None,
|
||||
@@ -327,27 +384,29 @@ to:
|
||||
};
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Return the VA-API decoder for that mode.** In `QsvPipelineBuilder.SetDecoder`, add a VA-API case to the switch (before the `_ =>` fallback at L134):
|
||||
- [ ] **Step 6: Return the software-output VA-API decoder for that mode.** In `QsvPipelineBuilder.SetDecoder`, add a case to the switch (before the `_ =>` fallback at L134):
|
||||
|
||||
```csharp
|
||||
(HardwareAccelerationMode.Vaapi, _) => new DecoderVaapi(),
|
||||
(HardwareAccelerationMode.Vaapi, _) => new DecoderVaapiToSoftware(),
|
||||
```
|
||||
|
||||
- [ ] **Step 6: Handle the pixel-format branch.** The `PixelFormat = ffmpegState.DecoderHardwareAccelerationMode == HardwareAccelerationMode.Qsv ? … : …` at L169-171 already takes the non-QSV branch for VA-API decode (source pixel format), which is correct — no change needed. Verify the file still builds:
|
||||
The pixel-format branch at L169-171 (`== Qsv ? … : source`) correctly takes the source-pixel-format branch for the VA-API-decode case — no change needed. `SetTonemap` (L705, `== Qsv`) correctly falls to the software `TonemapFilter` — no change needed.
|
||||
|
||||
- [ ] **Step 7: Build.**
|
||||
|
||||
Run: `dotnet build ErsatzTV.FFmpeg/ErsatzTV.FFmpeg.csproj`
|
||||
Expected: `Build succeeded`.
|
||||
|
||||
- [ ] **Step 7: Run the new arg tests + the full FFmpeg test suite (regression).**
|
||||
- [ ] **Step 8: Run the new arg tests + the full FFmpeg test suite (regression).**
|
||||
|
||||
Run: `dotnet test ErsatzTV.FFmpeg.Tests/ErsatzTV.FFmpeg.Tests.csproj`
|
||||
Expected: PASS, including both new `QsvPipelineBuilderTests` and all existing pipeline tests. If a QSV-only hardware filter appears on the VA-API path (test surfaces `scale_qsv`/`vpp_qsv`/`_qsv` filter before download), apply the software-scaling fallback described in the Design note, then re-run.
|
||||
Expected: PASS — both new tests plus all existing pipeline tests. The native test proves frames go VA-API→software→`hwupload`→QSV (no bare `vpp_qsv` on VA-API frames); the default test proves the QSV-decode path is unchanged.
|
||||
|
||||
- [ ] **Step 8: Commit.**
|
||||
- [ ] **Step 9: Commit.**
|
||||
|
||||
```bash
|
||||
git add ErsatzTV.FFmpeg/Pipeline/QsvPipelineBuilder.cs ErsatzTV.FFmpeg.Tests/Pipeline/QsvPipelineBuilderTests.cs
|
||||
git -c core.hooksPath=/dev/null commit -m "feat(498): QSV pipeline decodes via VA-API when preferring native decoder"
|
||||
git add ErsatzTV.FFmpeg/Decoder/DecoderVaapiToSoftware.cs ErsatzTV.FFmpeg/Pipeline/QsvPipelineBuilder.cs ErsatzTV.FFmpeg.Tests/Pipeline/QsvPipelineBuilderTests.cs
|
||||
git -c core.hooksPath=/dev/null commit -m "feat(498): QSV pipeline decodes via VA-API to software when preferring native decoder"
|
||||
```
|
||||
|
||||
---
|
||||
@@ -372,10 +431,10 @@ to:
|
||||
```csharp
|
||||
GetTonemapAlgorithm(playbackSettings),
|
||||
channel.Number == FileSystemLayout.TranscodeTroubleshootingChannel,
|
||||
channel.FFmpegProfile.QsvPreferNativeDecoder);
|
||||
channel.FFmpegProfile.QsvPreferNativeDecoder != false);
|
||||
```
|
||||
|
||||
Leave the two error-loop sites (L782-785, L923-926) untouched — they default `QsvPreferNativeDecoder` to false, which is correct (their decoder is already software/None).
|
||||
(`FFmpegState.QsvPreferNativeDecoder` is non-nullable `bool`; the domain property is `bool?`, so coerce with `!= false` — null-means-ON.) Leave the two error-loop sites (L782-785, L923-926) untouched — they default `QsvPreferNativeDecoder` to false, which is correct (their decoder is already software/None).
|
||||
|
||||
- [ ] **Step 2: Build the Core project.**
|
||||
|
||||
@@ -434,18 +493,18 @@ git -c core.hooksPath=/dev/null commit -m "feat(498): pass QsvPreferNativeDecode
|
||||
bool QsvPreferNativeDecoder);
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Map it in `Mapper`.** In `ProjectToViewModel`, change L40 ` profile.DeinterlaceVideo == true);` to:
|
||||
- [ ] **Step 4: Map it in `Mapper`.** The ViewModel/response-model fields are plain `bool`; the domain is `bool?` → coerce with `!= false` (null-means-ON, matching the `DeinterlaceVideo == true` pattern but ON-by-default). In `ProjectToViewModel`, change L40 ` profile.DeinterlaceVideo == true);` to:
|
||||
|
||||
```csharp
|
||||
profile.DeinterlaceVideo == true,
|
||||
profile.QsvPreferNativeDecoder);
|
||||
profile.QsvPreferNativeDecoder != false);
|
||||
```
|
||||
|
||||
In `ProjectToFullResponseModel`, change L83 ` ffmpegProfile.DeinterlaceVideo == true);` to:
|
||||
|
||||
```csharp
|
||||
ffmpegProfile.DeinterlaceVideo == true,
|
||||
ffmpegProfile.QsvPreferNativeDecoder);
|
||||
ffmpegProfile.QsvPreferNativeDecoder != false);
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Persist it in the Create handler.** In `CreateFFmpegProfileHandler.Validate`, in the `new FFmpegProfile { … }` initializer change L108 ` DeinterlaceVideo = request.DeinterlaceVideo` to:
|
||||
@@ -461,19 +520,19 @@ In `ProjectToFullResponseModel`, change L83 ` ffmpegProfile.Deinterla
|
||||
p.QsvPreferNativeDecoder = update.QsvPreferNativeDecoder;
|
||||
```
|
||||
|
||||
- [ ] **Step 7: Append to the REST request DTOs + their `ToCommand`.** In `CreateFFmpegProfileRequest.cs`, add the record param (before the `)` that opens the body) and the trailing `ToCommand()` arg:
|
||||
- [ ] **Step 7: Append to the REST request DTOs + their `ToCommand` (nullable for additive-compat).** The request param is `bool? … = null` (an old client omitting the key must NOT flip the feature off), and `ToCommand` resolves omission to default-ON with `?? true`. In `CreateFFmpegProfileRequest.cs`:
|
||||
|
||||
record param — change ` bool DeinterlaceVideo)` (the one followed by `{`) to:
|
||||
```csharp
|
||||
bool DeinterlaceVideo,
|
||||
bool QsvPreferNativeDecoder)
|
||||
bool? QsvPreferNativeDecoder = null)
|
||||
```
|
||||
`ToCommand()` — change the final ` DeinterlaceVideo);` to:
|
||||
```csharp
|
||||
DeinterlaceVideo,
|
||||
QsvPreferNativeDecoder);
|
||||
QsvPreferNativeDecoder ?? true);
|
||||
```
|
||||
Apply the identical two edits to `UpdateFFmpegProfileRequest.cs` (its `ToCommand(int id)` ends the same way).
|
||||
Apply the identical two edits to `UpdateFFmpegProfileRequest.cs` (its `ToCommand(int id)` ends the same way). NOTE: the MediatR command records (`CreateFFmpegProfile`/`UpdateFFmpegProfile`, Step 2) keep the param as plain `bool` — the `?? true` resolves it here.
|
||||
|
||||
- [ ] **Step 8: Verify the API query handlers.** Open `ErsatzTV.Application/FFmpegProfiles/Queries/GetFFmpegProfileByIdForApiHandler.cs` and `GetAllFFmpegProfilesForApiHandler.cs`. If they call `Mapper.ProjectToFullResponseModel`, Steps 3-4 already cover them. If either builds `new FFmpegFullProfileResponseModel(...)` inline, append `profile.QsvPreferNativeDecoder` as the final positional arg there too.
|
||||
|
||||
@@ -646,6 +705,7 @@ Run: `git push -u origin feat/498-qsv-native-decode`
|
||||
|
||||
## Notes for the executor
|
||||
|
||||
- The **highest-risk task is Task 4** (VA-API frames flowing through the QSV builder's filter graph). Its arg tests pin the command shape; the filter-graph correctness on real hardware is the deferred empirical check. If Task 4's tests surface a `*_qsv` hardware filter emitted onto VA-API frames, apply the software-scaling fallback in the native-decode branch (safe Jellyfin-equivalent baseline) rather than inventing a `hwmap` filter that doesn't yet exist in the codebase.
|
||||
- **Task 4 is the substantive task.** The correct approach (VA-API decode → *software* frames → existing `hwupload`/`vpp_qsv` branch → QSV encode) is statically sound and reuses the production software-decode+QSV-encode graph; the arg test proves `hwupload` bridges the frames (no bare `vpp_qsv` on VA-API frames). Do NOT emit `-hwaccel_output_format vaapi` and do NOT reuse `DecoderVaapi` (both keep frames on the GPU and break the QSV filter graph — see the Task 4 design note). What's deferred to the Intel host is only driver-level empirics (iHD tolerance on imperfect NAL units, upload bandwidth), not graph construction.
|
||||
- **The `bool?` nullable + `!= false` coercion is load-bearing (Blocker 2).** A non-nullable `bool` + `HasDefaultValue(true)` would make EF silently store `true` when a user creates a profile with the box unchecked. Keep the domain property `bool?`, read effective value with `!= false` everywhere (Mapper, Task 5), and keep the REST DTO params `bool? = null` with `?? true` (Task 6 Step 7). The Task 6 Step 10 round-trip test (request `false` → stored `false`) is the regression guard for this.
|
||||
- Every positional-record edit (Task 6) will fail the build loudly at the mismatched call site if a slot is missed — trust the compiler, fix, rebuild.
|
||||
- Do NOT set `ETV_UPDATE_GOLDENS`.
|
||||
|
||||
@@ -155,6 +155,14 @@ FFmpegProfilesScreen (checkbox, default true)
|
||||
- OpenAPI regen updates `docs/endpoint-index.md` + `v1.json` automatically.
|
||||
- No route change → `docs/blazor-route-parity.md` untouched.
|
||||
|
||||
## Post-review revisions (Fable, 2026-07-20)
|
||||
|
||||
An independent Fable review of the implementation plan corrected two design details; the plan is authoritative, but recording them here so this spec doesn't mislead:
|
||||
|
||||
1. **Decode produces SOFTWARE frames, not GPU VA-API surfaces.** The QSV builder's `SetScale`/`SetDeinterlace` emit a *bare* `vpp_qsv`/`deinterlace_qsv` on hardware frames, which cannot consume VA-API surfaces (ffmpeg won't auto-`hwmap`). So the native path emits `-hwaccel vaapi` **without** `-hwaccel_output_format vaapi` (frames download to system memory) and uses a new `DecoderVaapiToSoftware` (software output). Frames then flow through the existing, proven `format=nv12,hwupload=…,vpp_qsv`/`deinterlace_qsv` branch — the same graph the production error-loop paths (software-decode + QSV-encode) already run. This is a PCIe round-trip, NOT Jellyfin's zero-copy `hwmap=derive_device=qsv` (that filter doesn't exist here; out of scope). We still get the tolerant decoder + QSV encoder, which is the point.
|
||||
|
||||
2. **The domain property is `bool?`, and REST DTOs are `bool? = null`.** A non-nullable `bool` + `HasDefaultValue(true)` makes EF silently persist `true` when a user creates a profile with the box unchecked (CLR-default `false` reads as "unset"). Use the repo's `DeinterlaceVideo` pattern: `bool?` property, `HasDefaultValue(true)`, effective value read with `!= false` (null-means-ON). REST request DTOs use `bool? = null` + `?? true` so an old client omitting the field can't flip the feature off (`/api/v1` additive-compat).
|
||||
|
||||
## Out of scope (explicitly not doing)
|
||||
|
||||
- A general `DecodeHardwareAcceleration` enum column / second dropdown (issue's Option C) —
|
||||
|
||||
Reference in New Issue
Block a user