From 894fb7ec42eaf3f4712de6f762c15c97990f9d34 Mon Sep 17 00:00:00 2001 From: BlackSpirits Date: Sun, 13 Sep 2026 21:47:36 +0200 Subject: [PATCH 1/2] FFmpeg: prevent stale asynchronous loads from publishing --- .../Logic/VideoPlayers/Ffmpeg/FfmpegPlayer.cs | 73 +++++++++++++------ 1 file changed, 52 insertions(+), 21 deletions(-) diff --git a/src/ui/Logic/VideoPlayers/Ffmpeg/FfmpegPlayer.cs b/src/ui/Logic/VideoPlayers/Ffmpeg/FfmpegPlayer.cs index 9d9e4deda3..395c50266a 100644 --- a/src/ui/Logic/VideoPlayers/Ffmpeg/FfmpegPlayer.cs +++ b/src/ui/Logic/VideoPlayers/Ffmpeg/FfmpegPlayer.cs @@ -45,6 +45,8 @@ public sealed unsafe class FfmpegPlayer : IVideoPlayer, IDisposable private string _fileName = string.Empty; private bool _disposed; + private readonly Lock _loadLock = new(); + private int _loadGeneration; private Session? _session; private double _volume = 100; private double _speed = 1.0; @@ -130,16 +132,24 @@ public bool CopyCurrentFrame(IntPtr destination, int destinationStride, int widt public Task LoadFile(string fileName, double startPositionSeconds = 0) { - CloseFile(); - _fileName = fileName; + Session? previousSession; + int generation; + lock (_loadLock) + { + // Reserve this open before doing any slow native work. CloseFile or a newer LoadFile + // increments the generation, so this Task can never publish an obsolete session after + // the caller has already closed/replaced it. + generation = ++_loadGeneration; + previousSession = _session; + _session = null; + _fileName = fileName; + } + + previousSession?.Dispose(); + ClearCurrentFrame(); return Task.Run(() => { - if (_disposed) - { - return; - } - Session session; try { @@ -148,33 +158,54 @@ public Task LoadFile(string fileName, double startPositionSeconds = 0) catch (Exception exception) { Se.LogError(exception, $"ffmpeg player failed to open: {fileName}"); - _fileName = string.Empty; + lock (_loadLock) + { + if (generation == _loadGeneration) + { + _fileName = string.Empty; + } + } + return; } - if (_disposed) + lock (_loadLock) { - session.Dispose(); - return; - } + if (_disposed || generation != _loadGeneration) + { + session.Dispose(); + return; + } - _session = session; - session.Volume = _volume; - session.Speed = _speed; - session.Start(); + session.Volume = _volume; + session.Speed = _speed; + session.Start(); - // Always seek once: this is what decodes and shows the first picture (at the wanted - // position) while the player stays paused. - session.Seek(Math.Max(0, startPositionSeconds)); + // Always seek once: this is what decodes and shows the first picture (at the wanted + // position) while the player stays paused. + session.Seek(Math.Max(0, startPositionSeconds)); + _session = session; + } }); } public void CloseFile() { - var session = Interlocked.Exchange(ref _session, null); - _fileName = string.Empty; + Session? session; + lock (_loadLock) + { + _loadGeneration++; + session = _session; + _session = null; + _fileName = string.Empty; + } + session?.Dispose(); + ClearCurrentFrame(); + } + private void ClearCurrentFrame() + { lock (_currentFrameLock) { // The frame belonged to the session's pool, which is gone now. From 6e2675f7ce7cd0f4b866fb110dcc1fafc163a7eb Mon Sep 17 00:00:00 2001 From: BlackSpirits Date: Sun, 13 Sep 2026 21:50:18 +0200 Subject: [PATCH 2/2] FFmpeg: keep stale-load frame cleanup generation-safe --- .../Logic/VideoPlayers/Ffmpeg/FfmpegPlayer.cs | 51 ++++++++++++++++--- 1 file changed, 44 insertions(+), 7 deletions(-) diff --git a/src/ui/Logic/VideoPlayers/Ffmpeg/FfmpegPlayer.cs b/src/ui/Logic/VideoPlayers/Ffmpeg/FfmpegPlayer.cs index 395c50266a..bd6b424812 100644 --- a/src/ui/Logic/VideoPlayers/Ffmpeg/FfmpegPlayer.cs +++ b/src/ui/Logic/VideoPlayers/Ffmpeg/FfmpegPlayer.cs @@ -142,11 +142,30 @@ public Task LoadFile(string fileName, double startPositionSeconds = 0) generation = ++_loadGeneration; previousSession = _session; _session = null; - _fileName = fileName; + _fileName = string.Empty; } previousSession?.Dispose(); - ClearCurrentFrame(); + + var notifyFrameCleared = false; + lock (_loadLock) + { + // A newer open/close may have won while the old Session was shutting down. In that + // case this request must not clear the newer request's frame or file name. + if (_disposed || generation != _loadGeneration) + { + return Task.CompletedTask; + } + + ClearCurrentFrameState(); + notifyFrameCleared = true; + _fileName = fileName; + } + + if (notifyFrameCleared) + { + FrameReady?.Invoke(); + } return Task.Run(() => { @@ -177,6 +196,8 @@ public Task LoadFile(string fileName, double startPositionSeconds = 0) return; } + // Start/Seek are cheap state/thread setup. Keep publication atomic with close or a + // replacement load so a Session cannot be disposed between validation and start. session.Volume = _volume; session.Speed = _speed; session.Start(); @@ -192,29 +213,45 @@ public Task LoadFile(string fileName, double startPositionSeconds = 0) public void CloseFile() { Session? session; + int generation; lock (_loadLock) { - _loadGeneration++; + generation = ++_loadGeneration; session = _session; _session = null; _fileName = string.Empty; } session?.Dispose(); - ClearCurrentFrame(); + + var notifyFrameCleared = false; + lock (_loadLock) + { + // Do not erase a frame that belongs to a newer LoadFile which started while the old + // Session was shutting down. + if (generation == _loadGeneration) + { + ClearCurrentFrameState(); + notifyFrameCleared = true; + } + } + + if (notifyFrameCleared) + { + FrameReady?.Invoke(); + } } - private void ClearCurrentFrame() + private void ClearCurrentFrameState() { lock (_currentFrameLock) { - // The frame belonged to the session's pool, which is gone now. + // The frame belonged to the Session that was just detached. _currentFrame?.Dispose(); _currentFrame = null; } Interlocked.Increment(ref _frameVersion); - FrameReady?.Invoke(); } public void Play()