openshot/libopenshot
0
0
Fork 0
mirror of https://github.com/OpenShot/libopenshot.git synced 2026-03-02 08:53:52 -08:00
Code Issues Packages Projects Releases Wiki Activity

Harden playback/cache path for malformed media and concurrent timeline updates

Browse Source 
- Invalidate timeline cache on ApplyJsonDiff() clip insert (remove affected frame range).
  - Add lock in Timeline::ClearAllCache() for safe concurrent access.
  - Make VideoCacheThread cross-thread state safe (atomics + seek-state mutex).
  - Lock CacheMemory::Contains() to avoid races.
  - Handle malformed audio streams in FFmpegReader by disabling invalid audio and continuing video-only.
  - Add FPS/timebase safety fallbacks in FFmpeg frame/PTS math.
  - Guard Frame::GetSamplesPerFrame() against invalid inputs.
  - Add/adjust regression tests for cache invalidation and invalid rate handling.
This commit is contained in:
Jonathan Thomas
2026-02-11 20:11:47 -06:00
parent 57c1fb2ec3
commit d70e80eac4
9 changed files with 238 additions and 83 deletions
Download Patch File Download Diff File Expand all files Collapse all files

118
src/Qt/VideoCacheThread.cpp
View File

@@ -53,25 +53,28 @@ namespace openshot
return false;
}
if (min_frames_ahead < 0) {
const int64_t ready_min = min_frames_ahead.load();
if (ready_min < 0) {
return true;
}
const int64_t cached_index = last_cached_index.load();
const int64_t playhead = requested_display_frame.load();
int dir = computeDirection();
if (dir > 0) {
return (last_cached_index >= requested_display_frame + min_frames_ahead);
return (cached_index >= playhead + ready_min);
}
return (last_cached_index <= requested_display_frame - min_frames_ahead);
return (cached_index <= playhead - ready_min);
}
void VideoCacheThread::setSpeed(int new_speed)
{
// Only update last_speed and last_dir when new_speed != 0
if (new_speed != 0) {
last_speed = new_speed;
last_dir = (new_speed > 0 ? 1 : -1);
last_speed.store(new_speed);
last_dir.store(new_speed > 0 ? 1 : -1);
}
speed = new_speed;
speed.store(new_speed);
}
// Get the size in bytes of a frame (rough estimate)
@@ -106,29 +109,38 @@ namespace openshot
void VideoCacheThread::Seek(int64_t new_position, bool start_preroll)
{
if (start_preroll) {
userSeeked = true;
bool should_mark_seek = false;
bool should_preroll = false;
int64_t new_cached_count = cached_frame_count.load();
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 (
Timeline* timeline = static_cast<Timeline*>(reader);
timeline->ClearAllCache();
cached_frame_count = 0;
preroll_on_next_fill = true;
if (Timeline* timeline = dynamic_cast<Timeline*>(reader)) {
timeline->ClearAllCache();
}
new_cached_count = 0;
should_preroll = true;
}
else if (cache)
{
cached_frame_count = cache->Count();
preroll_on_next_fill = false;
}
else {
preroll_on_next_fill = false;
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);
if (start_preroll) {
preroll_on_next_fill.store(should_preroll);
userSeeked.store(should_mark_seek);
}
}
requested_display_frame = new_position;
}
void VideoCacheThread::Seek(int64_t new_position)
@@ -139,13 +151,17 @@ namespace openshot
int VideoCacheThread::computeDirection() const
{
// If speed ≠ 0, use its sign; if speed==0, keep last_dir
return (speed != 0 ? (speed > 0 ? 1 : -1) : last_dir);
const int current_speed = speed.load();
if (current_speed != 0) {
return (current_speed > 0 ? 1 : -1);
}
return last_dir.load();
}
void VideoCacheThread::handleUserSeek(int64_t playhead, int dir)
{
// Place last_cached_index just “behind” playhead in the given dir
last_cached_index = playhead - dir;
last_cached_index.store(playhead - dir);
}
void VideoCacheThread::handleUserSeekWithPreroll(int64_t playhead,
@@ -162,7 +178,7 @@ namespace openshot
preroll_start = std::min<int64_t>(timeline_end, playhead + preroll_frames);
}
}
last_cached_index = preroll_start - dir;
last_cached_index.store(preroll_start - dir);
}
int64_t VideoCacheThread::computePrerollFrames(const Settings* settings) const
@@ -187,9 +203,10 @@ namespace openshot
{
if (paused && !cache->Contains(playhead)) {
// If paused and playhead not in cache, clear everything
Timeline* timeline = static_cast<Timeline*>(reader);
timeline->ClearAllCache();
cached_frame_count = 0;
if (Timeline* timeline = dynamic_cast<Timeline*>(reader)) {
timeline->ClearAllCache();
}
cached_frame_count.store(0);
return true;
}
return false;
@@ -224,7 +241,7 @@ namespace openshot
ReaderBase* reader)
{
bool window_full = true;
int64_t next_frame = last_cached_index + dir;
int64_t next_frame = last_cached_index.load() + dir;
// Advance from last_cached_index toward window boundary
while ((dir > 0 && next_frame <= window_end) ||
@@ -234,7 +251,7 @@ namespace openshot
break;
}
// If a Seek was requested mid-caching, bail out immediately
if (userSeeked) {
if (userSeeked.load()) {
break;
}
@@ -243,7 +260,7 @@ namespace openshot
try {
auto framePtr = reader->GetFrame(next_frame);
cache->Add(framePtr);
cached_frame_count = cache->Count();
cached_frame_count.store(cache->Count());
}
catch (const OutOfBoundsFrame&) {
break;
@@ -254,7 +271,7 @@ namespace openshot
cache->Touch(next_frame);
}
last_cached_index = next_frame;
last_cached_index.store(next_frame);
next_frame += dir;
}
@@ -272,27 +289,31 @@ namespace openshot
// If caching disabled or no reader, mark cache as ready and sleep briefly
if (!settings->ENABLE_PLAYBACK_CACHING || !cache) {
cached_frame_count = (cache ? cache->Count() : 0);
min_frames_ahead = -1;
cached_frame_count.store(cache ? cache->Count() : 0);
min_frames_ahead.store(-1);
std::this_thread::sleep_for(double_micro_sec(50000));
continue;
}
// init local vars
min_frames_ahead = settings->VIDEO_CACHE_MIN_PREROLL_FRAMES;
min_frames_ahead.store(settings->VIDEO_CACHE_MIN_PREROLL_FRAMES);
Timeline* timeline = static_cast<Timeline*>(reader);
Timeline* timeline = dynamic_cast<Timeline*>(reader);
if (!timeline) {
std::this_thread::sleep_for(double_micro_sec(50000));
continue;
}
int64_t timeline_end = timeline->GetMaxFrame();
int64_t playhead = requested_display_frame;
bool paused = (speed == 0);
int64_t playhead = requested_display_frame.load();
bool paused = (speed.load() == 0);
int64_t preroll_frames = computePrerollFrames(settings);
cached_frame_count = cache->Count();
cached_frame_count.store(cache->Count());
// Compute effective direction (±1)
int dir = computeDirection();
if (speed != 0) {
last_dir = dir;
if (speed.load() != 0) {
last_dir.store(dir);
}
// Compute bytes_per_frame, max_bytes, and capacity once
@@ -313,16 +334,25 @@ namespace openshot
}
// Handle a user-initiated seek
bool use_preroll = preroll_on_next_fill;
if (userSeeked) {
bool did_user_seek = false;
bool use_preroll = false;
{
std::lock_guard<std::mutex> guard(seek_state_mutex);
playhead = requested_display_frame.load();
did_user_seek = userSeeked.load();
use_preroll = preroll_on_next_fill.load();
if (did_user_seek) {
userSeeked.store(false);
preroll_on_next_fill.store(false);
}
}
if (did_user_seek) {
if (use_preroll) {
handleUserSeekWithPreroll(playhead, dir, timeline_end, preroll_frames);
}
else {
handleUserSeek(playhead, dir);
}
userSeeked = false;
preroll_on_next_fill = false;
}
else if (!paused && capacity >= 1) {
// In playback mode, check if last_cached_index drifted outside the new window
@@ -339,8 +369,8 @@ namespace openshot
);
bool outside_window =
(dir > 0 && last_cached_index > window_end) ||
(dir < 0 && last_cached_index < window_begin);
(dir > 0 && last_cached_index.load() > window_end) ||
(dir < 0 && last_cached_index.load() < window_begin);
if (outside_window) {
handleUserSeek(playhead, dir);
}
@@ -362,7 +392,7 @@ namespace openshot
ready_target = 0;
}
int64_t configured_min = settings->VIDEO_CACHE_MIN_PREROLL_FRAMES;
min_frames_ahead = std::min<int64_t>(configured_min, ready_target);
min_frames_ahead.store(std::min<int64_t>(configured_min, ready_target));
// If paused and playhead is no longer in cache, clear everything
bool did_clear = clearCacheIfPaused(playhead, paused, cache);