From 3a50b9afc2d01ea744488afe819084e608148e46 Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Sun, 1 Mar 2026 22:49:31 -0600 Subject: [PATCH] Paused and playback seek behavior now follows one rule: keep cache on in-range seeks, rebuild only on out-of-range seeks. --- src/Qt/VideoCacheThread.cpp | 64 +++++++++++++++++++++---------------- tests/VideoCacheThread.cpp | 61 ++++++++++++++++++++++++++++++++--- 2 files changed, 94 insertions(+), 31 deletions(-) diff --git a/src/Qt/VideoCacheThread.cpp b/src/Qt/VideoCacheThread.cpp index 2fdb6e3d..aa562447 100644 --- a/src/Qt/VideoCacheThread.cpp +++ b/src/Qt/VideoCacheThread.cpp @@ -161,15 +161,23 @@ namespace openshot if (same_frame_refresh) { const bool is_paused = (speed.load() == 0); if (is_paused) { - // Paused same-frame edits (dragging keyframed properties) - // must not reuse any stale composite/cache state. - if (Timeline* timeline = dynamic_cast(reader)) { - timeline->ClearAllCache(); + const bool was_scrubbing = scrub_active.load(); + if (was_scrubbing && cache && cache_contains) { + // Preserve in-range cache for paused scrub preview -> same-frame commit. + should_mark_seek = false; + should_preroll = false; + should_clear_cache = false; + new_cached_count = cache->Count(); + } else { + // Paused same-frame edit refresh: force full cache refresh. + if (Timeline* timeline = dynamic_cast(reader)) { + timeline->ClearAllCache(); + } + new_cached_count = 0; + should_mark_seek = true; + should_preroll = true; + should_clear_cache = false; } - new_cached_count = 0; - should_mark_seek = true; - should_preroll = true; - should_clear_cache = false; } else { // Same-frame refresh during playback should stay lightweight. should_mark_seek = false; @@ -183,31 +191,32 @@ namespace openshot } } } else { - should_mark_seek = true; - if (cache && !cache_contains) { - // Uncached commit seek: avoid blocking this call path with a - // synchronous ClearAllCache(). The cache thread will reconcile - // window contents on the next iteration around the new playhead. + should_mark_seek = true; + // Uncached commit seek: defer cache clear to cache thread loop. new_cached_count = 0; should_preroll = true; should_clear_cache = true; } else if (cache) { + // In-range commit seek preserves cache window/baseline. + should_mark_seek = false; + should_preroll = false; + should_clear_cache = false; new_cached_count = cache->Count(); + } else { + // No cache object to query: use normal seek behavior. + should_mark_seek = true; } } leaving_scrub = true; } else { - // Non-preroll seeks are used for: - // 1) paused scrubbing (needs seek/scrub semantics), and - // 2) live refreshes while playing (must stay lightweight). + // Non-preroll seeks cover paused scrubbing and live playback refresh. const bool is_paused = (speed.load() == 0); if (is_paused && same_frame_refresh) { - // Property updates at the same paused playhead frame should - // refresh that frame only, without full seek/scrub churn. + // Same-frame paused refresh updates only that frame. should_mark_seek = false; should_preroll = false; should_clear_cache = false; @@ -220,21 +229,21 @@ namespace openshot leaving_scrub = true; } else if (is_paused) { - should_mark_seek = true; if (cache && !cache_contains) { + should_mark_seek = true; new_cached_count = 0; should_clear_cache = true; } else if (cache) { - if (cache_contains) { - cache->Remove(new_position); - } + // In-range paused seek preserves cache continuity. + should_mark_seek = false; new_cached_count = cache->Count(); + } else { + should_mark_seek = true; } entering_scrub = true; } else { - // During playback, avoid seek/scrub side effects that can - // churn cache state and cause visible flicker on updates. + // During playback, keep seek/scrub side effects minimal. should_mark_seek = false; should_preroll = false; should_clear_cache = false; @@ -247,10 +256,11 @@ namespace openshot { std::lock_guard guard(seek_state_mutex); - // Invalidate ready-state baseline immediately on seek so isReady() - // cannot pass/fail against stale cached_index from a prior window. + // Reset readiness baseline only when rebuilding cache. const int dir = computeDirection(); - last_cached_index.store(new_position - dir); + if (should_mark_seek || should_preroll || should_clear_cache) { + last_cached_index.store(new_position - dir); + } requested_display_frame.store(new_position); cached_frame_count.store(new_cached_count); preroll_on_next_fill.store(should_preroll); diff --git a/tests/VideoCacheThread.cpp b/tests/VideoCacheThread.cpp index 0a7c1096..a8dc5ef2 100644 --- a/tests/VideoCacheThread.cpp +++ b/tests/VideoCacheThread.cpp @@ -318,7 +318,7 @@ TEST_CASE("prefetchWindow: interrupt on userSeeked flag", "[VideoCacheThread]") CHECK(!wasFull); } -TEST_CASE("Seek preview: evicts playhead frame to force fresh render", "[VideoCacheThread]") { +TEST_CASE("Seek preview: preserves playhead frame when paused and inside cache", "[VideoCacheThread]") { TestableVideoCacheThread thread; CacheMemory cache(/*max_bytes=*/100000000); Timeline timeline(/*width=*/1280, /*height=*/720, /*fps=*/Fraction(24,1), @@ -333,10 +333,10 @@ TEST_CASE("Seek preview: evicts playhead frame to force fresh render", "[VideoCa thread.Seek(/*new_position=*/100, /*start_preroll=*/false); CHECK(thread.isScrubbing()); - CHECK(thread.getUserSeekedFlag()); + CHECK(!thread.getUserSeekedFlag()); CHECK(!thread.getPrerollOnNextFill()); - CHECK(!cache.Contains(100)); - CHECK(cache.Count() >= 1); + CHECK(cache.Contains(100)); + CHECK(cache.Count() >= 2); } TEST_CASE("Seek preview: outside cache marks uncached without preroll", "[VideoCacheThread]") { @@ -377,6 +377,59 @@ TEST_CASE("Seek commit: exits scrub mode and enables preroll when uncached", "[V CHECK(thread.getPrerollOnNextFill()); } +TEST_CASE("Seek commit: paused in-range seek preserves cached window state", "[VideoCacheThread]") { + TestableVideoCacheThread thread; + CacheMemory cache(/*max_bytes=*/100000000); + Timeline timeline(/*width=*/1280, /*height=*/720, /*fps=*/Fraction(24,1), + /*sample_rate=*/48000, /*channels=*/2, ChannelLayout::LAYOUT_STEREO); + timeline.SetCache(&cache); + thread.Reader(&timeline); + + cache.Add(std::make_shared(120, 0, 0)); + cache.Add(std::make_shared(121, 0, 0)); + REQUIRE(cache.Count() >= 2); + + // Simulate existing cache progress so we can verify no baseline reset. + thread.setLastCachedIndex(180); + + thread.Seek(/*new_position=*/120, /*start_preroll=*/true); + + CHECK(!thread.isScrubbing()); + CHECK(!thread.getUserSeekedFlag()); + CHECK(!thread.getPrerollOnNextFill()); + CHECK(thread.getLastCachedIndex() == 180); + CHECK(cache.Contains(120)); +} + +TEST_CASE("Seek commit: paused scrub preview then same-frame commit preserves cache", "[VideoCacheThread]") { + TestableVideoCacheThread thread; + CacheMemory cache(/*max_bytes=*/100000000); + Timeline timeline(/*width=*/1280, /*height=*/720, /*fps=*/Fraction(24,1), + /*sample_rate=*/48000, /*channels=*/2, ChannelLayout::LAYOUT_STEREO); + timeline.SetCache(&cache); + thread.Reader(&timeline); + + cache.Add(std::make_shared(140, 0, 0)); + cache.Add(std::make_shared(141, 0, 0)); + REQUIRE(cache.Count() >= 2); + + thread.setLastCachedIndex(210); + + // Typical paused seek flow: preview move, then commit same frame. + thread.Seek(/*new_position=*/140, /*start_preroll=*/false); + REQUIRE(thread.isScrubbing()); + REQUIRE(thread.getRequestedDisplayFrame() == 140); + + thread.Seek(/*new_position=*/140, /*start_preroll=*/true); + + CHECK(!thread.isScrubbing()); + CHECK(!thread.getUserSeekedFlag()); + CHECK(!thread.getPrerollOnNextFill()); + CHECK(thread.getLastCachedIndex() == 210); + CHECK(cache.Contains(140)); + CHECK(cache.Count() >= 2); +} + TEST_CASE("NotifyPlaybackPosition: ignored while scrubbing, applied after commit", "[VideoCacheThread]") { TestableVideoCacheThread thread; CacheMemory cache(/*max_bytes=*/100000000);