You've already forked libopenshot
mirror of
https://github.com/OpenShot/libopenshot.git
synced 2026-03-02 08:53:52 -08:00
- Playback updates no longer act like seeks.
- Scrubbing inside cached area keeps cache. - Scrubbing outside cached area clears cache immediately. - Preroll now starts on seek commit (release), not during drag. - Temporary debug logs were removed. - Tests were added for these behaviors.
This commit is contained in:
@@ -74,9 +74,13 @@ namespace openshot
|
||||
// Pausing Code (which re-syncs audio/video times)
|
||||
// - If speed is zero or speed changes
|
||||
// - If pre-roll is not ready (This should allow scrubbing of the timeline without waiting on pre-roll)
|
||||
if ((speed == 0 && video_position == last_video_position) ||
|
||||
(speed != 0 && last_speed != speed) ||
|
||||
(speed != 0 && !is_dirty && !videoCache->isReady()))
|
||||
bool wait_paused_hold = (speed == 0 && video_position == last_video_position);
|
||||
bool wait_speed_change = (speed != 0 && last_speed != speed);
|
||||
bool cache_ready = videoCache->isReady();
|
||||
bool wait_preroll = (speed != 0 && !is_dirty && !cache_ready);
|
||||
bool should_wait = (wait_paused_hold || wait_speed_change || wait_preroll);
|
||||
|
||||
if (should_wait)
|
||||
{
|
||||
// Sleep for a fraction of frame duration
|
||||
std::this_thread::sleep_for(frame_duration / 4);
|
||||
@@ -97,7 +101,16 @@ namespace openshot
|
||||
|
||||
// Set the video frame on the video thread and render frame
|
||||
videoPlayback->frame = frame;
|
||||
videoPlayback->rendered.reset();
|
||||
videoPlayback->render.signal();
|
||||
// Keep decode/position advancement aligned with actual preview updates.
|
||||
// This avoids occasional "silent advance then jump" behavior when
|
||||
// preroll transitions and rendering are briefly out-of-sync.
|
||||
const int render_wait_ms = std::max(
|
||||
1,
|
||||
static_cast<int>(frame_duration.count() / 1000.0 * 2.0)
|
||||
);
|
||||
videoPlayback->rendered.wait(render_wait_ms);
|
||||
|
||||
// Keep track of the last displayed frame
|
||||
last_video_position = video_position;
|
||||
@@ -116,6 +129,12 @@ namespace openshot
|
||||
// Protect against invalid or too-long sleep times
|
||||
std::this_thread::sleep_for(max_sleep);
|
||||
}
|
||||
} else {
|
||||
// If we're behind schedule (e.g. preroll/render stall), do not
|
||||
// burst through delayed frames. Resync timing baseline so
|
||||
// playback continues smoothly at normal cadence.
|
||||
start_time = std::chrono::time_point_cast<micro_sec>(current_time);
|
||||
playback_frames = 0;
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -150,8 +169,8 @@ namespace openshot
|
||||
// Increment playback frames (always in the positive direction)
|
||||
playback_frames += std::abs(speed);
|
||||
|
||||
// Update cache on which frame was retrieved
|
||||
videoCache->Seek(video_position);
|
||||
// Update playhead hint for cache window tracking without triggering seek behavior.
|
||||
videoCache->NotifyPlaybackPosition(video_position);
|
||||
|
||||
// return frame from reader
|
||||
return reader->GetFrame(video_position);
|
||||
@@ -170,7 +189,9 @@ namespace openshot
|
||||
{
|
||||
video_position = new_position;
|
||||
last_video_position = 0;
|
||||
is_dirty = true;
|
||||
// If actively playing, require cache readiness before advancing after seek.
|
||||
// If paused, keep dirty so the requested frame is rendered immediately.
|
||||
is_dirty = (speed == 0);
|
||||
}
|
||||
|
||||
// Start video/audio playback
|
||||
|
||||
@@ -30,6 +30,8 @@ namespace openshot
|
||||
, last_dir(1) // assume forward (+1) on first launch
|
||||
, userSeeked(false)
|
||||
, preroll_on_next_fill(false)
|
||||
, clear_cache_on_next_fill(false)
|
||||
, scrub_active(false)
|
||||
, requested_display_frame(1)
|
||||
, current_display_frame(1)
|
||||
, cached_frame_count(0)
|
||||
@@ -131,40 +133,90 @@ namespace openshot
|
||||
bool should_mark_seek = false;
|
||||
bool should_preroll = false;
|
||||
int64_t new_cached_count = cached_frame_count.load();
|
||||
bool entering_scrub = false;
|
||||
bool leaving_scrub = false;
|
||||
bool cache_contains = false;
|
||||
bool should_clear_cache = false;
|
||||
CacheBase* cache = reader ? reader->GetCache() : nullptr;
|
||||
if (cache) {
|
||||
cache_contains = cache->Contains(new_position);
|
||||
}
|
||||
|
||||
if (start_preroll) {
|
||||
should_mark_seek = true;
|
||||
CacheBase* cache = reader ? reader->GetCache() : nullptr;
|
||||
|
||||
if (cache && !cache->Contains(new_position))
|
||||
{
|
||||
// If user initiated seek, and current frame not found (
|
||||
if (Timeline* timeline = dynamic_cast<Timeline*>(reader)) {
|
||||
timeline->ClearAllCache();
|
||||
}
|
||||
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.
|
||||
new_cached_count = 0;
|
||||
should_preroll = true;
|
||||
should_clear_cache = true;
|
||||
}
|
||||
else if (cache)
|
||||
{
|
||||
new_cached_count = cache->Count();
|
||||
}
|
||||
leaving_scrub = true;
|
||||
}
|
||||
else {
|
||||
// Scrub preview: keep preroll disabled and interrupt current fill.
|
||||
// Do not synchronously clear cache here, as that can block seeks.
|
||||
should_mark_seek = true;
|
||||
if (cache && !cache_contains) {
|
||||
new_cached_count = 0;
|
||||
should_clear_cache = true;
|
||||
}
|
||||
else if (cache) {
|
||||
new_cached_count = cache->Count();
|
||||
}
|
||||
entering_scrub = true;
|
||||
}
|
||||
|
||||
{
|
||||
std::lock_guard<std::mutex> guard(seek_state_mutex);
|
||||
// Invalidate ready-state baseline immediately on seek so isReady()
|
||||
// cannot pass/fail against stale cached_index from a prior window.
|
||||
const int dir = computeDirection();
|
||||
last_cached_index.store(new_position - dir);
|
||||
requested_display_frame.store(new_position);
|
||||
cached_frame_count.store(new_cached_count);
|
||||
if (start_preroll) {
|
||||
preroll_on_next_fill.store(should_preroll);
|
||||
userSeeked.store(should_mark_seek);
|
||||
preroll_on_next_fill.store(should_preroll);
|
||||
// Clear behavior follows the latest seek intent.
|
||||
clear_cache_on_next_fill.store(should_clear_cache);
|
||||
userSeeked.store(should_mark_seek);
|
||||
if (entering_scrub) {
|
||||
scrub_active.store(true);
|
||||
}
|
||||
if (leaving_scrub) {
|
||||
scrub_active.store(false);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void VideoCacheThread::Seek(int64_t new_position)
|
||||
{
|
||||
Seek(new_position, false);
|
||||
NotifyPlaybackPosition(new_position);
|
||||
}
|
||||
|
||||
void VideoCacheThread::NotifyPlaybackPosition(int64_t new_position)
|
||||
{
|
||||
if (new_position <= 0) {
|
||||
return;
|
||||
}
|
||||
if (scrub_active.load()) {
|
||||
return;
|
||||
}
|
||||
|
||||
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(new_position);
|
||||
cached_frame_count.store(new_cached_count);
|
||||
}
|
||||
}
|
||||
|
||||
int VideoCacheThread::computeDirection() const
|
||||
@@ -257,10 +309,12 @@ namespace openshot
|
||||
int64_t window_begin,
|
||||
int64_t window_end,
|
||||
int dir,
|
||||
ReaderBase* reader)
|
||||
ReaderBase* reader,
|
||||
int64_t max_frames_to_fetch)
|
||||
{
|
||||
bool window_full = true;
|
||||
int64_t next_frame = last_cached_index.load() + dir;
|
||||
int64_t fetched_this_pass = 0;
|
||||
|
||||
// Advance from last_cached_index toward window boundary
|
||||
while ((dir > 0 && next_frame <= window_end) ||
|
||||
@@ -280,6 +334,7 @@ namespace openshot
|
||||
auto framePtr = reader->GetFrame(next_frame);
|
||||
cache->Add(framePtr);
|
||||
cached_frame_count.store(cache->Count());
|
||||
++fetched_this_pass;
|
||||
}
|
||||
catch (const OutOfBoundsFrame&) {
|
||||
break;
|
||||
@@ -292,6 +347,12 @@ namespace openshot
|
||||
|
||||
last_cached_index.store(next_frame);
|
||||
next_frame += dir;
|
||||
|
||||
// In active playback, avoid long uninterrupted prefetch bursts
|
||||
// that can delay player thread frame retrieval.
|
||||
if (max_frames_to_fetch > 0 && fetched_this_pass >= max_frames_to_fetch) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
return window_full;
|
||||
@@ -305,6 +366,21 @@ namespace openshot
|
||||
while (!threadShouldExit()) {
|
||||
Settings* settings = Settings::Instance();
|
||||
CacheBase* cache = reader ? reader->GetCache() : nullptr;
|
||||
Timeline* timeline = dynamic_cast<Timeline*>(reader);
|
||||
|
||||
// Process deferred clears even when caching is currently disabled
|
||||
// (e.g. active scrub mode), so stale ranges are removed promptly.
|
||||
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();
|
||||
timeline->ClearAllCache();
|
||||
cached_frame_count.store(0);
|
||||
// Reset ready baseline immediately after clear. Otherwise a
|
||||
// stale last_cached_index from the old cache window can make
|
||||
// isReady() report true before new preroll is actually filled.
|
||||
last_cached_index.store(clear_playhead - dir_on_clear);
|
||||
}
|
||||
|
||||
// If caching disabled or no reader, mark cache as ready and sleep briefly
|
||||
if (!settings->ENABLE_PLAYBACK_CACHING || !cache) {
|
||||
@@ -317,7 +393,6 @@ namespace openshot
|
||||
// init local vars
|
||||
min_frames_ahead.store(settings->VIDEO_CACHE_MIN_PREROLL_FRAMES);
|
||||
|
||||
Timeline* timeline = dynamic_cast<Timeline*>(reader);
|
||||
if (!timeline) {
|
||||
std::this_thread::sleep_for(double_micro_sec(50000));
|
||||
continue;
|
||||
@@ -366,7 +441,10 @@ namespace openshot
|
||||
}
|
||||
}
|
||||
if (did_user_seek) {
|
||||
if (use_preroll) {
|
||||
// During active playback, prioritize immediate forward readiness
|
||||
// from the playhead. Use directional preroll offset only while
|
||||
// paused/scrubbing contexts.
|
||||
if (use_preroll && paused) {
|
||||
handleUserSeekWithPreroll(playhead, dir, timeline_end, preroll_frames);
|
||||
}
|
||||
else {
|
||||
@@ -395,6 +473,22 @@ namespace openshot
|
||||
}
|
||||
}
|
||||
|
||||
// If a clear was requested by a seek that arrived after the loop
|
||||
// began, apply it now before any additional prefetch work. This
|
||||
// avoids "build then suddenly clear" behavior during playback.
|
||||
bool should_clear_mid_loop = clear_cache_on_next_fill.exchange(false);
|
||||
if (should_clear_mid_loop && timeline) {
|
||||
timeline->ClearAllCache();
|
||||
cached_frame_count.store(0);
|
||||
last_cached_index.store(playhead - dir);
|
||||
}
|
||||
|
||||
// While user is dragging/scrubbing, skip cache prefetch work.
|
||||
if (scrub_active.load()) {
|
||||
std::this_thread::sleep_for(double_micro_sec(10000));
|
||||
continue;
|
||||
}
|
||||
|
||||
// If capacity is insufficient, sleep and retry
|
||||
if (capacity < 1) {
|
||||
std::this_thread::sleep_for(double_micro_sec(50000));
|
||||
@@ -411,7 +505,8 @@ namespace openshot
|
||||
ready_target = 0;
|
||||
}
|
||||
int64_t configured_min = settings->VIDEO_CACHE_MIN_PREROLL_FRAMES;
|
||||
min_frames_ahead.store(std::min<int64_t>(configured_min, ready_target));
|
||||
const int64_t required_ahead = std::min<int64_t>(configured_min, ready_target);
|
||||
min_frames_ahead.store(required_ahead);
|
||||
|
||||
// If paused and playhead is no longer in cache, clear everything
|
||||
bool did_clear = clearCacheIfPaused(playhead, paused, cache);
|
||||
@@ -429,7 +524,21 @@ namespace openshot
|
||||
window_end);
|
||||
|
||||
// Attempt to fill any missing frames in that window
|
||||
bool window_full = prefetchWindow(cache, window_begin, window_end, dir, reader);
|
||||
int64_t max_frames_to_fetch = -1;
|
||||
if (!paused) {
|
||||
// Keep cache thread responsive during playback seeks so player
|
||||
// can start as soon as pre-roll is met instead of waiting for a
|
||||
// full cache window pass.
|
||||
max_frames_to_fetch = 8;
|
||||
}
|
||||
bool window_full = prefetchWindow(
|
||||
cache,
|
||||
window_begin,
|
||||
window_end,
|
||||
dir,
|
||||
reader,
|
||||
max_frames_to_fetch
|
||||
);
|
||||
|
||||
// If paused and window was already full, keep playhead fresh
|
||||
if (paused && window_full) {
|
||||
|
||||
@@ -61,7 +61,7 @@ namespace openshot
|
||||
/// @return The current speed (1=normal, 2=fast, –1=rewind, etc.)
|
||||
int getSpeed() const { return speed.load(); }
|
||||
|
||||
/// Seek to a specific frame (no preroll).
|
||||
/// Backward-compatible alias for playback position updates (no seek side effects).
|
||||
void Seek(int64_t new_position);
|
||||
|
||||
/**
|
||||
@@ -71,6 +71,9 @@ namespace openshot
|
||||
*/
|
||||
void Seek(int64_t new_position, bool start_preroll);
|
||||
|
||||
/// Update playback position without triggering seek behavior or cache invalidation.
|
||||
void NotifyPlaybackPosition(int64_t new_position);
|
||||
|
||||
/// Start the cache thread at high priority. Returns true if it’s actually running.
|
||||
bool StartThread();
|
||||
|
||||
@@ -171,7 +174,8 @@ namespace openshot
|
||||
int64_t window_begin,
|
||||
int64_t window_end,
|
||||
int dir,
|
||||
ReaderBase* reader);
|
||||
ReaderBase* reader,
|
||||
int64_t max_frames_to_fetch = -1);
|
||||
|
||||
//---------- Internal state ----------
|
||||
|
||||
@@ -182,6 +186,8 @@ namespace openshot
|
||||
std::atomic<int> last_dir; ///< Last direction sign (+1 forward, –1 backward).
|
||||
std::atomic<bool> userSeeked; ///< True if Seek(..., true) was called (forces a cache reset).
|
||||
std::atomic<bool> preroll_on_next_fill; ///< True if next cache rebuild should include preroll offset.
|
||||
std::atomic<bool> clear_cache_on_next_fill; ///< True if next cache loop should clear existing cache ranges.
|
||||
std::atomic<bool> scrub_active; ///< True while user is dragging/scrubbing the playhead.
|
||||
|
||||
std::atomic<int64_t> requested_display_frame; ///< Frame index the user requested.
|
||||
int64_t current_display_frame; ///< Currently displayed frame (unused here, reserved).
|
||||
|
||||
@@ -164,11 +164,16 @@ namespace openshot
|
||||
}
|
||||
|
||||
void QtPlayer::Seek(int64_t new_frame)
|
||||
{
|
||||
Seek(new_frame, true);
|
||||
}
|
||||
|
||||
void QtPlayer::Seek(int64_t new_frame, bool start_preroll)
|
||||
{
|
||||
// Check for seek
|
||||
if (reader && threads_started && new_frame > 0) {
|
||||
// Notify cache thread that seek has occurred
|
||||
p->videoCache->Seek(new_frame, true);
|
||||
p->videoCache->Seek(new_frame, start_preroll);
|
||||
|
||||
// Notify audio thread that seek has occurred
|
||||
p->audioPlayback->Seek(new_frame);
|
||||
|
||||
@@ -75,6 +75,9 @@ namespace openshot
|
||||
/// Seek to a specific frame in the player
|
||||
void Seek(int64_t new_frame);
|
||||
|
||||
/// Seek to a specific frame, optionally triggering preroll cache rebuild.
|
||||
void Seek(int64_t new_frame, bool start_preroll);
|
||||
|
||||
/// Set the source URL/path of this player (which will create an internal Reader)
|
||||
void SetSource(const std::string &source);
|
||||
|
||||
|
||||
@@ -42,6 +42,10 @@ public:
|
||||
void setMinFramesAhead(int64_t v) { min_frames_ahead.store(v); }
|
||||
void setLastDir(int d) { last_dir.store(d); }
|
||||
void forceUserSeekFlag() { userSeeked.store(true); }
|
||||
bool isScrubbing() const { return scrub_active.load(); }
|
||||
bool getUserSeekedFlag() const { return userSeeked.load(); }
|
||||
bool getPrerollOnNextFill() const { return preroll_on_next_fill.load(); }
|
||||
int64_t getRequestedDisplayFrame() const { return requested_display_frame.load(); }
|
||||
};
|
||||
|
||||
// ----------------------------------------------------------------------------
|
||||
@@ -313,3 +317,84 @@ TEST_CASE("prefetchWindow: interrupt on userSeeked flag", "[VideoCacheThread]")
|
||||
CHECK(thread.getLastCachedIndex() == 23);
|
||||
CHECK(!wasFull);
|
||||
}
|
||||
|
||||
TEST_CASE("Seek preview: inside cache is cheap and does not invalidate", "[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>(100, 0, 0));
|
||||
cache.Add(std::make_shared<Frame>(101, 0, 0));
|
||||
REQUIRE(cache.Count() >= 2);
|
||||
|
||||
thread.Seek(/*new_position=*/100, /*start_preroll=*/false);
|
||||
|
||||
CHECK(thread.isScrubbing());
|
||||
CHECK(thread.getUserSeekedFlag());
|
||||
CHECK(!thread.getPrerollOnNextFill());
|
||||
CHECK(cache.Contains(100));
|
||||
CHECK(cache.Count() >= 2);
|
||||
}
|
||||
|
||||
TEST_CASE("Seek preview: outside cache marks uncached without preroll", "[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>(10, 0, 0));
|
||||
cache.Add(std::make_shared<Frame>(11, 0, 0));
|
||||
REQUIRE(cache.Count() >= 2);
|
||||
|
||||
thread.Seek(/*new_position=*/300, /*start_preroll=*/false);
|
||||
|
||||
CHECK(thread.isScrubbing());
|
||||
CHECK(thread.getUserSeekedFlag());
|
||||
CHECK(!thread.getPrerollOnNextFill());
|
||||
CHECK(cache.Count() >= 2);
|
||||
}
|
||||
|
||||
TEST_CASE("Seek commit: exits scrub mode and enables preroll when uncached", "[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);
|
||||
|
||||
thread.Seek(/*new_position=*/200, /*start_preroll=*/false);
|
||||
CHECK(thread.isScrubbing());
|
||||
|
||||
thread.Seek(/*new_position=*/200, /*start_preroll=*/true);
|
||||
|
||||
CHECK(!thread.isScrubbing());
|
||||
CHECK(thread.getUserSeekedFlag());
|
||||
CHECK(thread.getPrerollOnNextFill());
|
||||
}
|
||||
|
||||
TEST_CASE("NotifyPlaybackPosition: ignored while scrubbing, applied after commit", "[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);
|
||||
|
||||
thread.Seek(/*new_position=*/120, /*start_preroll=*/false);
|
||||
REQUIRE(thread.isScrubbing());
|
||||
CHECK(thread.getRequestedDisplayFrame() == 120);
|
||||
|
||||
thread.NotifyPlaybackPosition(/*new_position=*/25);
|
||||
CHECK(thread.getRequestedDisplayFrame() == 120);
|
||||
|
||||
thread.Seek(/*new_position=*/120, /*start_preroll=*/true);
|
||||
REQUIRE(!thread.isScrubbing());
|
||||
|
||||
thread.NotifyPlaybackPosition(/*new_position=*/25);
|
||||
CHECK(thread.getRequestedDisplayFrame() == 25);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user