diff --git a/src/Qt/VideoCacheThread.cpp b/src/Qt/VideoCacheThread.cpp index 597bc8cb..ad80e33a 100644 --- a/src/Qt/VideoCacheThread.cpp +++ b/src/Qt/VideoCacheThread.cpp @@ -63,7 +63,7 @@ namespace openshot } const int64_t cached_index = last_cached_index.load(); - const int64_t playhead = requested_display_frame.load(); + int64_t playhead = requested_display_frame.load(); int dir = computeDirection(); // Near timeline boundaries, don't require more pre-roll than can exist. @@ -77,6 +77,7 @@ namespace openshot if (max_frame < 1) { return false; } + playhead = clampToTimelineRange(playhead, max_frame); int64_t required_ahead = ready_min; int64_t available_ahead = (dir > 0) @@ -146,7 +147,6 @@ namespace openshot const int64_t timeline_end = resolveTimelineEnd(); const int64_t clamped_new_position = clampToTimelineRange(new_position, timeline_end); const int64_t current_requested = requested_display_frame.load(); - const int64_t clamped_requested = clampToTimelineRange(current_requested, timeline_end); bool should_mark_seek = false; bool should_preroll = false; @@ -265,7 +265,7 @@ namespace openshot if (should_mark_seek || should_preroll || should_clear_cache) { last_cached_index.store(clamped_new_position - dir); } - requested_display_frame.store(clamped_new_position); + requested_display_frame.store(new_position); cached_frame_count.store(new_cached_count); preroll_on_next_fill.store(should_preroll); // Clear behavior follows the latest seek intent. @@ -294,16 +294,13 @@ namespace openshot return; } - const int64_t timeline_end = resolveTimelineEnd(); - const int64_t clamped_new_position = clampToTimelineRange(new_position, timeline_end); - int64_t new_cached_count = cached_frame_count.load(); if (CacheBase* cache = reader ? reader->GetCache() : nullptr) { new_cached_count = cache->Count(); } { std::lock_guard guard(seek_state_mutex); - requested_display_frame.store(clamped_new_position); + requested_display_frame.store(new_position); cached_frame_count.store(new_cached_count); } } @@ -490,7 +487,8 @@ namespace openshot bool should_clear_cache = clear_cache_on_next_fill.exchange(false); if (should_clear_cache && timeline) { const int dir_on_clear = computeDirection(); - const int64_t clear_playhead = requested_display_frame.load(); + const int64_t clear_playhead = clampToTimelineRange( + requested_display_frame.load(), resolveTimelineEnd()); timeline->ClearAllCache(); cached_frame_count.store(0); // Reset ready baseline immediately after clear. Otherwise a @@ -515,9 +513,8 @@ namespace openshot continue; } int64_t timeline_end = resolveTimelineEnd(); - int64_t playhead = requested_display_frame.load(); - playhead = clampToTimelineRange(playhead, timeline_end); - requested_display_frame.store(playhead); + int64_t raw_playhead = requested_display_frame.load(); + int64_t playhead = clampToTimelineRange(raw_playhead, timeline_end); bool paused = (speed.load() == 0); int64_t preroll_frames = computePrerollFrames(settings); @@ -572,8 +569,8 @@ namespace openshot bool use_preroll = false; { std::lock_guard guard(seek_state_mutex); - playhead = clampToTimelineRange(requested_display_frame.load(), timeline_end); - requested_display_frame.store(playhead); + raw_playhead = requested_display_frame.load(); + playhead = clampToTimelineRange(raw_playhead, timeline_end); did_user_seek = userSeeked.load(); use_preroll = preroll_on_next_fill.load(); if (did_user_seek) { diff --git a/src/Timeline.cpp b/src/Timeline.cpp index 70ec04da..e604bc2d 100644 --- a/src/Timeline.cpp +++ b/src/Timeline.cpp @@ -945,12 +945,12 @@ std::shared_ptr Timeline::GetFrame(int64_t requested_frame) if (requested_frame < 1) requested_frame = 1; const int64_t max_frame = GetMaxFrame(); - if (max_frame > 0 && requested_frame > max_frame) - requested_frame = max_frame; + const bool past_timeline_end = (max_frame > 0 && requested_frame > max_frame); // Check cache std::shared_ptr frame; - frame = final_cache->GetFrame(requested_frame); + if (!past_timeline_end) + frame = final_cache->GetFrame(requested_frame); if (frame) { // Debug output ZmqLogger::Instance()->AppendDebugMethod( @@ -967,7 +967,8 @@ std::shared_ptr Timeline::GetFrame(int64_t requested_frame) // Check cache 2nd time std::shared_ptr frame; - frame = final_cache->GetFrame(requested_frame); + if (!past_timeline_end) + frame = final_cache->GetFrame(requested_frame); if (frame) { // Debug output ZmqLogger::Instance()->AppendDebugMethod( @@ -1118,8 +1119,9 @@ std::shared_ptr Timeline::GetFrame(int64_t requested_frame) // Set frame # on mapped frame new_frame->SetFrameNumber(requested_frame); - // Add final frame to cache - final_cache->Add(new_frame); + // Add final frame to cache (only for valid timeline range) + if (!past_timeline_end) + final_cache->Add(new_frame); if (force_safe_composite) { last_rendered_cache_epoch.store(current_epoch, std::memory_order_relaxed); } diff --git a/tests/Timeline.cpp b/tests/Timeline.cpp index 74d82d2f..d5cc8567 100644 --- a/tests/Timeline.cpp +++ b/tests/Timeline.cpp @@ -1378,7 +1378,7 @@ TEST_CASE( "ApplyJSONDiff Update Reader Info", "[libopenshot][timeline]" ) } -TEST_CASE("GetFrame clamps requests past timeline end", "[libopenshot][timeline][cache]") { +TEST_CASE("GetFrame past-end requests are not cached", "[libopenshot][timeline][cache]") { TimelineSolidColorReader reader( 64, 64, 30, 1, @@ -1396,14 +1396,15 @@ TEST_CASE("GetFrame clamps requests past timeline end", "[libopenshot][timeline] const int64_t end = t.GetMaxFrame(); REQUIRE(end > 1); REQUIRE(t.GetCache() != nullptr); + const int64_t count_before = t.GetCache()->Count(); std::shared_ptr first = t.GetFrame(end + 25); REQUIRE(first != nullptr); - CHECK(first->number == end); - const int64_t count_after_first = t.GetCache()->Count(); + CHECK(first->number == end + 25); + CHECK(t.GetCache()->Count() == count_before); std::shared_ptr second = t.GetFrame(end + 120); REQUIRE(second != nullptr); - CHECK(second->number == end); - CHECK(t.GetCache()->Count() == count_after_first); + CHECK(second->number == end + 120); + CHECK(t.GetCache()->Count() == count_before); } diff --git a/tests/VideoCacheThread.cpp b/tests/VideoCacheThread.cpp index f0c2d80b..60cab97d 100644 --- a/tests/VideoCacheThread.cpp +++ b/tests/VideoCacheThread.cpp @@ -45,6 +45,7 @@ public: bool isScrubbing() const { return scrub_active.load(); } bool getUserSeekedFlag() const { return userSeeked.load(); } bool getPrerollOnNextFill() const { return preroll_on_next_fill.load(); } + bool getClearCacheOnNextFill() const { return clear_cache_on_next_fill.load(); } int64_t getRequestedDisplayFrame() const { return requested_display_frame.load(); } }; @@ -163,6 +164,26 @@ TEST_CASE("isReady: clamps preroll requirement at timeline boundaries", "[VideoC CHECK(thread.isReady()); } +TEST_CASE("isReady: treats out-of-range playhead as timeline edge for readiness", "[VideoCacheThread]") { + TestableVideoCacheThread thread; + + Timeline timeline(/*width=*/1280, /*height=*/720, /*fps=*/Fraction(30,1), + /*sample_rate=*/48000, /*channels=*/2, ChannelLayout::LAYOUT_STEREO); + thread.Reader(&timeline); + + const int64_t end = timeline.info.video_length; + REQUIRE(end > 10); + + thread.setMinFramesAhead(30); + thread.setSpeed(1); + thread.setPlayhead(end + 100); + + thread.setLastCachedIndex(end - 1); + CHECK(!thread.isReady()); + thread.setLastCachedIndex(end); + CHECK(thread.isReady()); +} + TEST_CASE("clearCacheIfPaused: clears only when paused and not in cache", "[VideoCacheThread]") { TestableVideoCacheThread thread; CacheMemory cache(/*max_bytes=*/100000000); @@ -217,6 +238,27 @@ TEST_CASE("clearCacheIfPaused: clears when paused past timeline end and playhead CHECK(cache.Count() == 0); } +TEST_CASE("clearCacheIfPaused: does not clear when paused past timeline end and end frame is cached", "[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); + + const int64_t end = timeline.info.video_length; + REQUIRE(end > 1); + + cache.Add(std::make_shared(end, 0, 0)); + const int64_t initial_count = cache.Count(); + REQUIRE(initial_count > 0); + + const bool didClear = thread.clearCacheIfPaused(/*playhead=*/end + 12, /*paused=*/true, &cache); + CHECK(!didClear); + CHECK(cache.Count() == initial_count); +} + TEST_CASE("handleUserSeek: sets last_cached_index to playhead - dir", "[VideoCacheThread]") { TestableVideoCacheThread thread; @@ -470,7 +512,7 @@ TEST_CASE("Seek preview: paused out-of-range seek clamps to end and preserves ca CHECK(thread.isScrubbing()); CHECK(!thread.getUserSeekedFlag()); - CHECK(thread.getRequestedDisplayFrame() == end); + CHECK(thread.getRequestedDisplayFrame() == end + 24); CHECK(thread.getLastCachedIndex() == end - 1); CHECK(cache.Contains(end)); } @@ -495,11 +537,60 @@ TEST_CASE("Seek commit: paused out-of-range seek past end enables cache rebuild CHECK(!thread.isScrubbing()); CHECK(thread.getUserSeekedFlag()); CHECK(thread.getPrerollOnNextFill()); - CHECK(thread.getRequestedDisplayFrame() == end); + CHECK(thread.getRequestedDisplayFrame() == end + 24); CHECK(thread.getLastCachedIndex() == end - 1); CHECK(!cache.Contains(end)); } +TEST_CASE("Seek commit: playback jump to cached frame 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(150, 0, 0)); + cache.Add(std::make_shared(220, 0, 0)); + thread.setPlayhead(220); + thread.setSpeed(1); + thread.setLastCachedIndex(230); + + thread.Seek(/*new_position=*/150, /*start_preroll=*/true); + + CHECK(!thread.isScrubbing()); + CHECK(!thread.getUserSeekedFlag()); + CHECK(!thread.getPrerollOnNextFill()); + CHECK(!thread.getClearCacheOnNextFill()); + CHECK(thread.getRequestedDisplayFrame() == 150); + CHECK(thread.getLastCachedIndex() == 230); +} + +TEST_CASE("Seek commit: playback click inside active cached window 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(220, 0, 0)); + cache.Add(std::make_shared(230, 0, 0)); + cache.Add(std::make_shared(260, 0, 0)); + thread.setPlayhead(220); + thread.setSpeed(1); + thread.setLastCachedIndex(260); + + thread.Seek(/*new_position=*/230, /*start_preroll=*/true); + + CHECK(!thread.isScrubbing()); + CHECK(!thread.getUserSeekedFlag()); + CHECK(!thread.getPrerollOnNextFill()); + CHECK(!thread.getClearCacheOnNextFill()); + CHECK(thread.getRequestedDisplayFrame() == 230); + CHECK(thread.getLastCachedIndex() == 260); +} + TEST_CASE("NotifyPlaybackPosition: ignored while scrubbing, applied after commit", "[VideoCacheThread]") { TestableVideoCacheThread thread; CacheMemory cache(/*max_bytes=*/100000000); @@ -521,3 +612,27 @@ TEST_CASE("NotifyPlaybackPosition: ignored while scrubbing, applied after commit thread.NotifyPlaybackPosition(/*new_position=*/25); CHECK(thread.getRequestedDisplayFrame() == 25); } + +TEST_CASE("Seek non-preroll: playback uncached target does not force cache rebuild", "[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(220, 0, 0)); + cache.Add(std::make_shared(221, 0, 0)); + thread.setPlayhead(220); + thread.setSpeed(1); + thread.setLastCachedIndex(230); + + thread.Seek(/*new_position=*/120, /*start_preroll=*/false); + + CHECK(!thread.isScrubbing()); + CHECK(!thread.getUserSeekedFlag()); + CHECK(!thread.getPrerollOnNextFill()); + CHECK(!thread.getClearCacheOnNextFill()); + CHECK(thread.getRequestedDisplayFrame() == 120); + CHECK(thread.getLastCachedIndex() == 230); +}