diff --git a/src/ui/Logic/VideoPlayers/Ffmpeg/FfmpegPlayer.cs b/src/ui/Logic/VideoPlayers/Ffmpeg/FfmpegPlayer.cs index 9d9e4deda3..bd6b424812 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,43 @@ 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 = string.Empty; + } - return Task.Run(() => + previousSession?.Dispose(); + + var notifyFrameCleared = false; + lock (_loadLock) { - if (_disposed) + // 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; + return Task.CompletedTask; } + ClearCurrentFrameState(); + notifyFrameCleared = true; + _fileName = fileName; + } + + if (notifyFrameCleared) + { + FrameReady?.Invoke(); + } + + return Task.Run(() => + { Session session; try { @@ -148,42 +177,81 @@ 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(); + // 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(); - // 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; + int generation; + lock (_loadLock) + { + generation = ++_loadGeneration; + session = _session; + _session = null; + _fileName = string.Empty; + } + session?.Dispose(); + 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 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()