From 4de9801dd4e12301976dd2ec106e05a347f1a817 Mon Sep 17 00:00:00 2001 From: Aras Pranckevicius Date: Mon, 2 Jun 2025 12:02:00 +0200 Subject: [PATCH] Fix: VSE source cache eviction logic New VSE cache implementation (!137926) uses the same eviction logic for the source image ("raw") cache as for the final frame cache -- remove the frames furthest away from the playhead. However, source image cache keys are source media frames, not timeline frames, so this was not working correctly when strips are not at timeline start. So, for each cached entry record the strip-start-relative timeline frame that the entry was created at. This takes care of things like retiming, reversed frames etc. And also makes the cached frames visualization more correct in presence of frame reversal etc. Pull Request: https://projects.blender.org/blender/blender/pulls/139667 --- .../intern/cache/source_image_cache.cc | 56 ++++++++++--------- 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/source/blender/sequencer/intern/cache/source_image_cache.cc b/source/blender/sequencer/intern/cache/source_image_cache.cc index 40d55e4d262..99ca5f945d4 100644 --- a/source/blender/sequencer/intern/cache/source_image_cache.cc +++ b/source/blender/sequencer/intern/cache/source_image_cache.cc @@ -27,9 +27,18 @@ namespace blender::seq { static Mutex source_image_cache_mutex; struct SourceImageCache { + struct FrameEntry { + ImBuf *image = nullptr; + /* Frame in timeline, relative to strip start. Used to determine which + * entries to evict (furthest from the playhead). Due to reversed + * frames, playback rate, retiming the relationship between source frame + * index and timeline frame is not a simple one. */ + float strip_frame = 0; + }; + struct StripEntry { /* Map key is {source media frame index (i.e. movie frame), view ID}. */ - Map, ImBuf *> frames; + Map, FrameEntry> frames; }; Map map_; @@ -42,8 +51,8 @@ struct SourceImageCache { void clear() { for (const auto &item : map_.items()) { - for (ImBuf *image : item.value.frames.values()) { - IMB_freeImBuf(image); + for (const auto &frame : item.value.frames.values()) { + IMB_freeImBuf(frame.image); } } map_.clear(); @@ -55,8 +64,8 @@ struct SourceImageCache { if (entry == nullptr) { return; } - for (ImBuf *image : entry->frames.values()) { - IMB_freeImBuf(image); + for (const auto &frame : entry->frames.values()) { + IMB_freeImBuf(frame.image); } map_.remove_contained(strip); } @@ -107,7 +116,10 @@ ImBuf *source_image_cache_get(const RenderData *context, const Strip *strip, flo return nullptr; } /* Search entries for the frame we want. */ - res = val->frames.lookup_default({frame_index, view_id}, nullptr); + SourceImageCache::FrameEntry *frame = val->frames.lookup_ptr({frame_index, view_id}); + if (frame != nullptr) { + res = frame->image; + } } if (res) { @@ -148,11 +160,12 @@ void source_image_cache_put(const RenderData *context, } BLI_assert_msg(val != nullptr, "Source image cache value should never be null here"); - ImBuf *&item = val->frames.lookup_or_add_default({frame_index, view_id}); - if (item != nullptr) { - IMB_freeImBuf(item); + SourceImageCache::FrameEntry &frame = val->frames.lookup_or_add_default({frame_index, view_id}); + if (frame.image != nullptr) { + IMB_freeImBuf(frame.image); } - item = image; + frame.strip_frame = timeline_frame - strip->start; + frame.image = image; } void source_image_cache_invalidate_strip(Scene *scene, const Strip *strip) @@ -196,17 +209,10 @@ void source_image_cache_iterate(Scene *scene, return; } - const float scene_fps = float(scene->r.frs_sec) / float(scene->r.frs_sec_base); - - for (const auto &[key, value] : cache->map_.items()) { - for (std::pair frame_view : value.frames.keys()) { - /* We have frame index of source media, try to guesstimate the timeline frame. - * Note that this will be not correct when retiming, strobing etc. are used. - * However, factor in playback rate difference. */ - float frame_fl = frame_view.first / time_media_playback_rate_factor_get(key, scene_fps); - float timeline_frame = frame_fl + time_start_frame_get(key); - - callback_iter(userdata, key, int(timeline_frame)); + for (const auto &[strip, frames] : cache->map_.items()) { + for (const auto &[frame_key, frame] : frames.frames.items()) { + const float timeline_frame = strip->start + frame.strip_frame; + callback_iter(userdata, strip, int(timeline_frame)); } } } @@ -220,8 +226,8 @@ size_t source_image_cache_calc_memory_size(const Scene *scene) } size_t size = 0; for (const SourceImageCache::StripEntry &entry : cache->map_.values()) { - for (const ImBuf *image : entry.frames.values()) { - size += IMB_get_size_in_memory(image); + for (const SourceImageCache::FrameEntry &frame : entry.frames.values()) { + size += IMB_get_size_in_memory(frame.image); } } return size; @@ -257,7 +263,7 @@ bool source_image_cache_evict(Scene *scene) int best_score = 0; for (const auto &strip : cache->map_.items()) { for (const auto &entry : strip.value.frames.items()) { - const int item_frame = entry.key.first; + const int item_frame = int(strip.key->start + entry.value.strip_frame); /* Score for removal is distance to current frame; 2x that if behind current frame. */ int score = 0; if (item_frame < cur_frame) { @@ -276,7 +282,7 @@ bool source_image_cache_evict(Scene *scene) /* Remove if we found one. */ if (best_strip != nullptr) { - IMB_freeImBuf(best_strip->frames.lookup(best_key)); + IMB_freeImBuf(best_strip->frames.lookup(best_key).image); best_strip->frames.remove(best_key); return true; }