Live scrubbing: video follows the seek bar while dragging (#49)

Author: danielchalmers

SentryReplay.Data/Playback/ICameraPlayer.cs

@@ -16,5 +16,12 @@ public interface ICameraPlayer : IDisposable
     Task PauseAsync();
     Task StopAsync();
     Task CloseAsync();
-    Task SeekAsync(TimeSpan position);
+
+    /// <summary>
+    /// Seeks to <paramref name="position"/>. Accurate seeks (the default) decode forward to land
+    /// exactly on the target frame; fast seeks jump to the nearest keyframe, which is far cheaper
+    /// but can land slightly before the target -- intended for live scrubbing while the seek bar
+    /// thumb is being dragged, where responsiveness matters more than frame precision.
+    /// </summary>
+    Task SeekAsync(TimeSpan position, bool accurate = true);
 }

SentryReplay.Tests/Fixtures/FakeCameraPlayer.cs

@@ -14,6 +14,9 @@ internal sealed class FakeCameraPlayer : ICameraPlayer
     public List<string> OpenedPaths { get; } = [];
     public List<TimeSpan> SeekPositions { get; } = [];
 
+    /// <summary>Ordered log of the <c>accurate</c> flag passed to each <see cref="SeekAsync"/> call.</summary>
+    public List<bool> SeekAccurateFlags { get; } = [];
+
     /// <summary>
     /// Ordered log of play/pause/seek calls so tests can assert on call ordering
     /// (e.g. that a post-recovery resume plays before it seeks).
@@ -98,10 +101,11 @@ public Task CloseAsync()
         return Task.CompletedTask;
     }
 
-    public Task SeekAsync(TimeSpan position)
+    public Task SeekAsync(TimeSpan position, bool accurate = true)
     {
         SeekPositions.Add(position);
-        CallLog.Add($"seek:{position.TotalSeconds}");
+        SeekAccurateFlags.Add(accurate);
+        CallLog.Add(accurate ? $"seek:{position.TotalSeconds}" : $"scrub:{position.TotalSeconds}");
         Position = position;
         PositionChanged?.Invoke(this, new CameraPositionChangedEventArgs(position));
         return Task.CompletedTask;

SentryReplay.Tests/MainWindowViewModelTests.cs

@@ -649,6 +649,123 @@ public async Task WhileScrubbing_ControllerPositionDoesNotMoveTheSlider()
         vm.SeekPosition.ShouldBe(0.25, 0.0001);
     }
 
+    // Synchronous (no async/await in the test body itself -- see CreateViewModelWithOpenedClip's
+    // comment on why thread affinity matters here). FakeCameraPlayer's SeekAsync/OpenAsync and an
+    // uncontended SemaphoreSlim all complete synchronously, so blocking on the resulting tasks
+    // never suspends the thread and keeps every controller property change on it.
+    [Fact]
+    public void DragSequence_IssuesFastSeeks_ReleaseIssuesAccurateSeekAtReleasePosition()
+    {
+        using var clipFiles = TestClipFiles.Create(chunkCount: 1); // 60s clip (see TestClipFiles)
+        var (vm, _, front) = CreateViewModelWithOpenedClip(UniquelyNamed(clipFiles));
+
+        vm.BeginSeek();
+
+        // Simulate a drag: each slider value change while dragging should scrub-seek (fast/keyframe).
+        vm.SeekPosition = 0.2; // 12s of 60s
+        vm.OnSeekSliderValueChanged();
+
+        vm.SeekPosition = 0.5; // 30s
+        vm.OnSeekSliderValueChanged();
+
+        front.SeekPositions.ShouldContain(TimeSpan.FromSeconds(12));
+        front.SeekPositions.ShouldContain(TimeSpan.FromSeconds(30));
+
+        // Every seek issued so far while dragging must have been fast (non-accurate).
+        front.SeekAccurateFlags.ShouldAllBe(accurate => accurate == false);
+
+        // Release at 0.75 (45s): EndSeekAsync must issue exactly one ACCURATE seek at the release position.
+        vm.SeekPosition = 0.75;
+
+        // Blocking rather than awaiting is deliberate here, not an oversight: this whole test must stay
+        // pinned to one thread (see CreateViewModelWithOpenedClip's comment), and EndSeekAsync only ever
+        // awaits FakeCameraPlayer calls and an uncontended SemaphoreSlim, both of which complete
+        // synchronously -- so this never actually blocks.
+#pragma warning disable xUnit1031
+        vm.EndSeekAsync().GetAwaiter().GetResult();
+#pragma warning restore xUnit1031
+
+        front.SeekPositions[^1].ShouldBe(TimeSpan.FromSeconds(45));
+        front.SeekAccurateFlags[^1].ShouldBeTrue();
+    }
+
+    [Fact]
+    public void PositionSync_WhenNotDragging_DoesNotTriggerScrubSeeks()
+    {
+        using var clipFiles = TestClipFiles.Create(chunkCount: 1);
+        var (vm, controller, front) = CreateViewModelWithOpenedClip(UniquelyNamed(clipFiles));
+
+        front.SeekPositions.Clear();
+
+        // Playback position advances on its own (not a drag): SeekPosition updates via the
+        // controller -> UpdateSeekPositionFromController path, which does not go through
+        // OnSeekSliderValueChanged, so no scrub seek should ever be issued.
+        controller.Position = TimeSpan.FromSeconds(10);
+        vm.OnSeekSliderValueChanged(); // the view raises ValueChanged for programmatic changes too
+
+        front.SeekPositions.ShouldBeEmpty();
+    }
+
+    // A generous 20s deadline (vs. the 5s used elsewhere in these test files): this helper waits on
+    // a real clip-open flowing through Task.Run/the media source builder, which can slow down a lot
+    // under the CPU/disk contention of the full suite's many parallel test classes; 20s comfortably
+    // absorbs that while still catching a genuine hang.
+    private static async Task WaitUntilAsync(Func<bool> condition)
+    {
+        var deadline = DateTime.UtcNow.AddSeconds(20);
+        while (!condition())
+        {
+            if (DateTime.UtcNow > deadline)
+            {
+                throw new TimeoutException("Condition was not met within the timeout.");
+            }
+
+            await Task.Delay(10);
+        }
+    }
+
+    // FfconcatMediaSourceBuilder writes each camera's playlist to a shared temp path keyed only by
+    // the clip's Name (not its folder), and every TestClipFiles.Create() call across the whole
+    // suite uses the same literal name ("Test Clip") -- so under full-suite parallel execution,
+    // concurrently-running tests can race on the very same playlist file. Giving this test's clip
+    // its own unique name sidesteps that shared-fixture collision without touching the fixture (or
+    // the production path-naming) itself, which is shared by dozens of other tests.
+    private static CamClip UniquelyNamed(TestClipFiles clipFiles)
+    {
+        var clip = clipFiles.Clip;
+        return new CamClip(clip.FullPath, $"{clip.Name} {Guid.NewGuid():N}", clip.Timestamp, clip.Chunks, clip.Event);
+    }
+
+    // Opens the clip on the controller to completion BEFORE the view-model subscribes to it, then
+    // attaches the view-model. Doing it in this order (rather than via CreateViewModelWithController,
+    // which subscribes up front) avoids the deadlock described on that helper: the real open flow's
+    // ObservableProperty writes happen on background-thread continuations, and if the view-model were
+    // already subscribed, its PropertyChanged handler would call Dispatcher.Invoke from that background
+    // thread with no pumped message loop to service it, hanging the open forever. Once the clip is
+    // fully open and idle, driving the view-model's own seek APIs from the test thread afterward is safe.
+    // Synchronous by design (no async/await): the view-model's constructor captures
+    // Dispatcher.CurrentDispatcher for whatever thread calls it, and RunOnUiThread only stays
+    // deadlock-free while every later controller property change happens on that exact same
+    // thread (see the comment on CreateViewModelWithController above). An async method resumes
+    // its continuation after an await on a thread-pool thread with no guarantee it matches the
+    // thread that ran the code before the await -- which silently breaks that invariant. Blocking
+    // on the wait here (instead of awaiting it) keeps everything, including the VM construction
+    // and every later test action, pinned to the single calling thread.
+    private static (MainWindowViewModel Vm, VideoPlayerController Controller, FakeCameraPlayer Front) CreateViewModelWithOpenedClip(
+        CamClip clip)
+    {
+        var front = new FakeCameraPlayer();
+        var built = new VideoPlayerController(front, new FakeCameraPlayer(), new FakeCameraPlayer(), new FakeCameraPlayer());
+
+        built.LoadClips([clip]);
+        built.Playlist.MoveTo(0);
+        WaitUntilAsync(() => front.PlayCount > 0 && built.IsMediaOpen && !built.IsLoading).GetAwaiter().GetResult();
+
+        var vm = new MainWindowViewModel(() => built, backgroundYield: () => Task.CompletedTask);
+        vm.InitializePlayer();
+        return (vm, built, front);
+    }
+
     [Fact]
     public void ControllerLoadingAndPlaying_MirrorToViewModel()
     {

SentryReplay.Tests/SeekScrubCoalescerTests.cs

@@ -0,0 +1,141 @@
+namespace SentryReplay.Tests;
+
+public sealed class SeekScrubCoalescerTests
+{
+    [Fact]
+    public void FirstValue_IssuesImmediately()
+    {
+        var issued = new List<TimeSpan>();
+        var coalescer = new SeekScrubCoalescer(position =>
+        {
+            issued.Add(position);
+            return Task.CompletedTask;
+        });
+
+        coalescer.OnDragValueChanged(TimeSpan.FromSeconds(1));
+
+        issued.ShouldBe([TimeSpan.FromSeconds(1)]);
+    }
+
+    [Fact]
+    public async Task RapidBurst_WhileSeekInFlight_DropsIntermediatesAndIssuesOnlyLastValueOnCompletion()
+    {
+        var issued = new List<TimeSpan>();
+        var gate = new TaskCompletionSource();
+        var coalescer = new SeekScrubCoalescer(async position =>
+        {
+            issued.Add(position);
+
+            // The first seek blocks so the burst below all lands while it's in flight.
+            if (issued.Count == 1)
+            {
+                await gate.Task;
+            }
+        });
+
+        // First value issues immediately and blocks on the gate.
+        coalescer.OnDragValueChanged(TimeSpan.FromSeconds(1));
+        issued.ShouldBe([TimeSpan.FromSeconds(1)]);
+
+        // A rapid burst of further values arrives while the first seek is still in flight; all but
+        // the last are dropped entirely (never issued, not even queued).
+        coalescer.OnDragValueChanged(TimeSpan.FromSeconds(1.3));
+        coalescer.OnDragValueChanged(TimeSpan.FromSeconds(1.6));
+        coalescer.OnDragValueChanged(TimeSpan.FromSeconds(1.9));
+        coalescer.OnDragValueChanged(TimeSpan.FromSeconds(2.5));
+
+        issued.ShouldBe([TimeSpan.FromSeconds(1)]); // still only the first -- nothing issued yet
+
+        // Let the first seek complete; the coalescer should now issue exactly the last queued value.
+        gate.SetResult();
+        await WaitUntilAsync(() => issued.Count == 2);
+
+        issued.ShouldBe([TimeSpan.FromSeconds(1), TimeSpan.FromSeconds(2.5)]);
+    }
+
+    [Fact]
+    public void ValueWithinMinimumStep_OfLastIssuedValue_IsSkipped()
+    {
+        var issued = new List<TimeSpan>();
+        var coalescer = new SeekScrubCoalescer(
+            position =>
+            {
+                issued.Add(position);
+                return Task.CompletedTask;
+            },
+            minimumStep: TimeSpan.FromMilliseconds(250));
+
+        coalescer.OnDragValueChanged(TimeSpan.FromSeconds(10));
+        issued.ShouldBe([TimeSpan.FromSeconds(10)]);
+
+        // Well within 250ms of the last-issued value -- skipped, no new seek.
+        coalescer.OnDragValueChanged(TimeSpan.FromMilliseconds(10100));
+        issued.ShouldBe([TimeSpan.FromSeconds(10)]);
+
+        // Clears the 250ms threshold -- issued.
+        coalescer.OnDragValueChanged(TimeSpan.FromMilliseconds(10300));
+        issued.ShouldBe([TimeSpan.FromSeconds(10), TimeSpan.FromMilliseconds(10300)]);
+    }
+
+    [Fact]
+    public async Task PendingValueWithinMinimumStep_OfLastIssuedValue_IsNotReissuedOnCompletion()
+    {
+        var issued = new List<TimeSpan>();
+        var gate = new TaskCompletionSource();
+        var coalescer = new SeekScrubCoalescer(async position =>
+        {
+            issued.Add(position);
+            if (issued.Count == 1)
+            {
+                await gate.Task;
+            }
+        });
+
+        coalescer.OnDragValueChanged(TimeSpan.FromSeconds(5));
+
+        // Queued while in flight, but only 100ms away from the last-issued value.
+        coalescer.OnDragValueChanged(TimeSpan.FromMilliseconds(5100));
+
+        gate.SetResult();
+
+        // Give the continuation a chance to run; it must NOT issue a second seek.
+        await Task.Delay(50);
+
+        issued.ShouldBe([TimeSpan.FromSeconds(5)]);
+        coalescer.IsSeekInFlight.ShouldBeFalse();
+    }
+
+    [Fact]
+    public void Reset_ForgetsLastIssuedValue_SoNextValueAlwaysIssues()
+    {
+        var issued = new List<TimeSpan>();
+        var coalescer = new SeekScrubCoalescer(position =>
+        {
+            issued.Add(position);
+            return Task.CompletedTask;
+        });
+
+        coalescer.OnDragValueChanged(TimeSpan.FromSeconds(10));
+        coalescer.Reset();
+
+        // Even though this is well within 250ms of the previous value, Reset cleared history for
+        // the new drag, so it must issue.
+        coalescer.OnDragValueChanged(TimeSpan.FromMilliseconds(10050));
+
+        issued.ShouldBe([TimeSpan.FromSeconds(10), TimeSpan.FromMilliseconds(10050)]);
+    }
+
+    private static async Task WaitUntilAsync(Func<bool> condition)
+    {
+        var deadline = DateTime.UtcNow.AddSeconds(5);
+        while (!condition())
+        {
+            if (DateTime.UtcNow > deadline)
+            {
+                throw new TimeoutException("Condition was not met within the timeout.");
+            }
+
+            await Task.Delay(10);
+        }
+    }
+}

SentryReplay.Tests/VideoPlayerControllerTests.cs

@@ -99,6 +99,43 @@ public async Task PauseSeekAndStop_ControlOpenPlayers()
         controller.IsMediaOpen.ShouldBeFalse();
     }
 
+    [Fact]
+    public async Task ScrubSeekAsync_IssuesFastSeeksToOpenPlayers()
+    {
+        using var clipFiles = TestClipFiles.Create(chunkCount: 1);
+        var front = new FakeCameraPlayer();
+        var back = new FakeCameraPlayer();
+        using var controller = CreateController(front, back);
+
+        controller.LoadClips([clipFiles.Clip]);
+        controller.Playlist.MoveTo(0);
+        await WaitUntilAsync(() => front.PlayCount > 0 && back.PlayCount > 0);
+
+        await controller.ScrubSeekAsync(TimeSpan.FromSeconds(12));
+
+        front.SeekPositions.ShouldContain(TimeSpan.FromSeconds(12));
+        back.SeekPositions.ShouldContain(TimeSpan.FromSeconds(12));
+        front.SeekAccurateFlags[^1].ShouldBeFalse();
+        back.SeekAccurateFlags[^1].ShouldBeFalse();
+        controller.Position.ShouldBe(TimeSpan.FromSeconds(12));
+    }
+
+    [Fact]
+    public async Task SeekAsync_IssuesAccurateSeeksToOpenPlayers()
+    {
+        using var clipFiles = TestClipFiles.Create(chunkCount: 1);
+        var front = new FakeCameraPlayer();
+        using var controller = CreateController(front);
+
+        controller.LoadClips([clipFiles.Clip]);
+        controller.Playlist.MoveTo(0);
+        await WaitUntilAsync(() => front.PlayCount > 0);
+
+        await controller.SeekAsync(TimeSpan.FromSeconds(12));
+
+        front.SeekAccurateFlags[^1].ShouldBeTrue();
+    }
+
     [Fact]
     public async Task StopAsync_ClosesPlayersEvenWhenStopFails()
     {

SentryReplay/MainWindow.xaml.cs

@@ -155,6 +155,11 @@ private async void SeekSlider_PreviewMouseUp(object sender, MouseButtonEventArgs
         await _viewModel.EndSeekAsync();
     }
 
+    // Fires for both thumb-drag and click-then-drag (WPF raises ValueChanged on every Value mutation,
+    // whether from dragging the Thumb or from IsMoveToPointEnabled's click-to-position), and also for
+    // the one-off value jump a plain click makes. PreviewMouseDown has already called BeginSeek by
+    // the time this fires, so even a plain click issues one keyframe scrub seek here — harmless,
+    // since the accurate mouse-up seek runs behind the same serialized lock and lands last.
     private void SeekSlider_ValueChanged(object sender, RoutedPropertyChangedEventArgs<double> e)
     {
         _viewModel.OnSeekSliderValueChanged();