You've already forked libopenshot
mirror of
https://github.com/OpenShot/libopenshot.git
synced 2026-06-08 22:17:28 -07:00
- Timeline::GetFrame no longer clamps past-end requests to MaxFrame; it returns requested past-end black frames but does not cache them, so scrubbing past end no longer pollutes final_cache.
- VideoCacheThread now preserves raw playhead intent while clamping only for cache/readiness internals, and playback click behavior was corrected so non-preroll playback seeks don’t clear cache unexpectedly.
This commit is contained in:
Jonathan Thomas
+10
-13
@@ -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<std::mutex> 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<std::mutex> 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) {
|
||||
|
||||
+8
-6
@@ -945,12 +945,12 @@ std::shared_ptr<Frame> 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;
|
||||
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<Frame> Timeline::GetFrame(int64_t requested_frame)
|
||||
|
||||
// Check cache 2nd time
|
||||
std::shared_ptr<Frame> 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<Frame> 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);
|
||||
}
|
||||
|
||||
+6
-5
@@ -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<Frame> 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<Frame> 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);
|
||||
}
|
||||
|
||||
+117
-2
@@ -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<Frame>(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<Frame>(150, 0, 0));
|
||||
cache.Add(std::make_shared<Frame>(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<Frame>(220, 0, 0));
|
||||
cache.Add(std::make_shared<Frame>(230, 0, 0));
|
||||
cache.Add(std::make_shared<Frame>(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<Frame>(220, 0, 0));
|
||||
cache.Add(std::make_shared<Frame>(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);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user