From e654a16b600c2ba4a9bf1584285827cc828a9d5c Mon Sep 17 00:00:00 2001 From: gramps Date: Thu, 12 Nov 2026 13:57:03 -0800 Subject: [PATCH] fix(history): file each session under the real encoder, not "unknown" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FfmpegEncoder resolved the encoder name into its OWN StreamHealth instance, while the view model read CurrentHealth — a different object. So every recorded session was filed as "unknown", and the rate history's encoder key (evidence about NVENC says nothing about QSV) had nothing to key on. The name now travels on the encoder interface and the pump forwards it: IFfmpegEncoder.VideoEncoderName -> FramePump.EncoderName. Caught while adding it: reading _framePump.EncoderName from RefreshRateProposal throws, because that call runs at MainViewModel ctor line 354 and the pump is built at line 468. Eight tests failed with the same NullReferenceException, which took a while to see because the harness's test output is buffered — the first honest look at per-test logging named it in one line. Guarded; the history falls back to the last session's encoder for that tier, which is exactly the pre-first-session case the fallback already existed for. Tests: FramePumpTests gains the encoder-name assertion (null before start, h264_nvenc after), and the three fake encoders implement the new member. 675 pass, 0 warnings. --- Models/StreamConfig.cs | 5 ----- Services/Encoder/FfmpegEncoder.cs | 5 ++++- Services/Encoder/FramePump.cs | 4 ++++ Services/Encoder/IFfmpegEncoder.cs | 5 +++++ ViewModels/MainViewModel.Rate.cs | 5 ++++- ViewModels/MainViewModel.StepDown.cs | 2 +- ytLive.Tests/BrandFlashOutputTests.cs | 2 ++ ytLive.Tests/EncoderRestartTests.cs | 2 ++ ytLive.Tests/FramePumpTests.cs | 18 ++++++++++++++++++ 9 files changed, 40 insertions(+), 8 deletions(-) diff --git a/Models/StreamConfig.cs b/Models/StreamConfig.cs index 0a8c3c9..e0ddd6d 100644 --- a/Models/StreamConfig.cs +++ b/Models/StreamConfig.cs @@ -29,11 +29,6 @@ public class StreamHealth public TimeSpan StreamDuration { get; set; } public string? LastError { get; set; } - /// The encoder the probe actually chose ("h264_nvenc", "libopenh264", …). Filled once the - /// encoder starts, and the key the rate history is filed under — evidence about NVENC says nothing - /// about QSV on another machine. - public string? VideoEncoder { get; set; } - // YouTube-specific (liveStreams.status) public string? HealthStatus { get; set; } // "good", "ok", "bad", "noData" public string? HealthMessage { get; set; } diff --git a/Services/Encoder/FfmpegEncoder.cs b/Services/Encoder/FfmpegEncoder.cs index 2df2af3..9705057 100644 --- a/Services/Encoder/FfmpegEncoder.cs +++ b/Services/Encoder/FfmpegEncoder.cs @@ -29,6 +29,9 @@ public sealed class FfmpegEncoder : IFfmpegEncoder private IEncoderProcess? _process; private EncoderOptions? _options; private StreamHealth _health = new() { Status = StreamStatus.Offline }; + + /// + public string? VideoEncoderName { get; private set; } private Task? _stderrLoop; private bool _stopRequested; @@ -82,7 +85,7 @@ public sealed class FfmpegEncoder : IFfmpegEncoder var ffmpegPath = await _locator.LocateAsync(cancellationToken).ConfigureAwait(false); var encoder = options.VideoEncoder ?? await ProbeEncoderAsync(ffmpegPath, cancellationToken).ConfigureAwait(false); - _health.VideoEncoder = encoder; + VideoEncoderName = encoder; var args = FfmpegArgs.Build(options, encoder); var startInfo = new ProcessStartInfo diff --git a/Services/Encoder/FramePump.cs b/Services/Encoder/FramePump.cs index d7f7c59..1d997fa 100644 --- a/Services/Encoder/FramePump.cs +++ b/Services/Encoder/FramePump.cs @@ -91,6 +91,10 @@ public sealed class FramePump : IDisposable private readonly object _gate = new(); private IFfmpegEncoder? _encoder; + + /// The encoder this session is running, or null before the first start. The view model + /// files each broadcast's rate history under it. + public string? EncoderName => _encoder?.VideoEncoderName; private CancellationTokenSource? _cts; private Task? _pumpTask; private bool _started; diff --git a/Services/Encoder/IFfmpegEncoder.cs b/Services/Encoder/IFfmpegEncoder.cs index 522251f..625f7b3 100644 --- a/Services/Encoder/IFfmpegEncoder.cs +++ b/Services/Encoder/IFfmpegEncoder.cs @@ -19,6 +19,11 @@ public interface IFfmpegEncoder : IDisposable /// Fires when the subprocess dies unexpectedly (non-zero exit while live). event EventHandler? ProcessFailed; + /// Which encoder the probe chose for this session ("h264_nvenc", "libopenh264", …). Null + /// before the first start. The rate history is keyed by it: evidence about one encoder says nothing + /// about another. + string? VideoEncoderName { get; } + Task StartAsync(EncoderOptions options, CancellationToken cancellationToken = default); Task SubmitFrameAsync(VideoFrame frame, CancellationToken cancellationToken = default); Task StopAsync(CancellationToken cancellationToken = default); diff --git a/ViewModels/MainViewModel.Rate.cs b/ViewModels/MainViewModel.Rate.cs index fbca674..8b6ed32 100644 --- a/ViewModels/MainViewModel.Rate.cs +++ b/ViewModels/MainViewModel.Rate.cs @@ -145,7 +145,10 @@ public partial class MainViewModel // Before this run's first stream the encoder is unknown; the last session on this tier is // the honest guess, and evidence about a different encoder stays out either way. var lastForTier = history.LastOrDefault(entry => entry.Tier == tier.Display); - var encoder = CurrentHealth.VideoEncoder + // The pump is built later in the constructor than this runs, so before the first session + // there is no encoder to ask; the last session on this tier is the honest guess, and + // evidence about a different encoder stays out either way. + var encoder = (_framePump is { } pump ? pump.EncoderName : null) ?? (string.IsNullOrEmpty(lastForTier.Encoder) ? null : lastForTier.Encoder); var suggestion = RateHistory.Suggest(history, tier.Display, encoder, RatedMbps); diff --git a/ViewModels/MainViewModel.StepDown.cs b/ViewModels/MainViewModel.StepDown.cs index 58b4085..c3980a0 100644 --- a/ViewModels/MainViewModel.StepDown.cs +++ b/ViewModels/MainViewModel.StepDown.cs @@ -57,7 +57,7 @@ public partial class MainViewModel { _layoutStore.AppendRateHistory(new SessionRateResult( SelectedQuality.Display, - CurrentHealth.VideoEncoder ?? "unknown", + _framePump.EncoderName ?? "unknown", _sessionRatedMbps, planner.Current.BitrateMbps, planner.Policy.StepsUsed)); diff --git a/ytLive.Tests/BrandFlashOutputTests.cs b/ytLive.Tests/BrandFlashOutputTests.cs index 7ad5722..b7976a8 100644 --- a/ytLive.Tests/BrandFlashOutputTests.cs +++ b/ytLive.Tests/BrandFlashOutputTests.cs @@ -580,6 +580,8 @@ public sealed class BrandFlashOutputTests /// production encoder uses, so the assertion is about the bytes on the wire. private sealed class CapturingEncoder : IFfmpegEncoder { + public string? VideoEncoderName => "h264_nvenc"; + public readonly List Frames = new(); public readonly TaskCompletionSource FirstFrame = new(TaskCreationOptions.RunContinuationsAsynchronously); diff --git a/ytLive.Tests/EncoderRestartTests.cs b/ytLive.Tests/EncoderRestartTests.cs index d1f3d54..d635516 100644 --- a/ytLive.Tests/EncoderRestartTests.cs +++ b/ytLive.Tests/EncoderRestartTests.cs @@ -86,6 +86,8 @@ public sealed class EncoderRestartTests private sealed class CapturingEncoder : IFfmpegEncoder { + public string? VideoEncoderName => "h264_nvenc"; + public readonly TaskCompletionSource FirstFrame = new(TaskCreationOptions.RunContinuationsAsynchronously); public event Action? Started; public Action? Stopped { get; set; } diff --git a/ytLive.Tests/FramePumpTests.cs b/ytLive.Tests/FramePumpTests.cs index 988e386..838b5a5 100644 --- a/ytLive.Tests/FramePumpTests.cs +++ b/ytLive.Tests/FramePumpTests.cs @@ -22,6 +22,8 @@ public class FramePumpTests { private sealed class FakeEncoder : IFfmpegEncoder { + public string? VideoEncoderName => "h264_nvenc"; + public readonly List Frames = new(); public readonly List Backings = new(); public int StartCount; @@ -133,6 +135,22 @@ public class FramePumpTests } } + [Fact] + public async Task The_pump_reports_which_encoder_the_session_runs() + { + // The rate history is keyed by encoder: evidence recorded against NVENC must not be applied to a + // machine that later runs QSV. Before this the name went to a StreamHealth copy nobody read, so + // every filed session said "unknown". + var encoder = new FakeEncoder(); + using var pump = NewPump(encoder); + + Assert.Null(pump.EncoderName); // nothing has started yet + await pump.StartAsync(); + Assert.Equal("h264_nvenc", pump.EncoderName); + + await pump.StopAsync(); + } + /// The ONE integration test for TASK 5: the real service (hermetic /// HTTP) returns the reusable stream's RTMP URL, the provider seam hands it /// to the pump, and the pump starts the encoder with that exact URL — the