diff --git a/src/Clip.cpp b/src/Clip.cpp index 917366f4..8f863c7b 100644 --- a/src/Clip.cpp +++ b/src/Clip.cpp @@ -519,11 +519,17 @@ std::shared_ptr Clip::GetFrame(std::shared_ptr backgroun frame->GetAudioChannelsCount()); } - // Apply background canvas (i.e. flatten this image onto previous layer image) - apply_background(frame, background_frame); + const bool from_timeline = (options != nullptr); + if (from_timeline) { + // Timeline path composites into destination only, preserving cached clip pixels. + compose_onto_background(frame, background_frame); + return frame; + } - // Return processed 'frame' - return frame; + // Direct clip callers expect the returned frame image to include compositing. + auto output = std::make_shared(*frame.get()); + apply_background(output, background_frame); + return output; } else // Throw error if reader not initialized @@ -1277,6 +1283,16 @@ void Clip::apply_background(std::shared_ptr frame, std::shared_ frame->AddImage(background_canvas); } +void Clip::compose_onto_background(std::shared_ptr frame, std::shared_ptr background_frame) { + // Composite onto background without mutating source clip frame image. + auto canvas = background_frame->GetImage(); + QPainter painter(canvas.get()); + painter.setCompositionMode(static_cast(composite)); + painter.drawImage(0, 0, *frame->GetImage()); + painter.end(); + background_frame->AddImage(canvas); +} + // Apply effects to the source frame (if any) void Clip::apply_effects(std::shared_ptr frame, int64_t timeline_frame_number, TimelineInfoStruct* options, bool before_keyframes) { diff --git a/src/Clip.h b/src/Clip.h index cfb37768..80749001 100644 --- a/src/Clip.h +++ b/src/Clip.h @@ -130,6 +130,9 @@ namespace openshot { /// Apply background image to the current clip image (i.e. flatten this image onto previous layer) void apply_background(std::shared_ptr frame, std::shared_ptr background_frame); + /// Composite clip image onto a destination frame without mutating clip frame image. + void compose_onto_background(std::shared_ptr frame, std::shared_ptr background_frame); + /// Apply effects to the source frame (if any) void apply_effects(std::shared_ptr frame, int64_t timeline_frame_number, TimelineInfoStruct* options, bool before_keyframes); diff --git a/src/Qt/PlayerPrivate.cpp b/src/Qt/PlayerPrivate.cpp index da672bb5..38d84a12 100644 --- a/src/Qt/PlayerPrivate.cpp +++ b/src/Qt/PlayerPrivate.cpp @@ -189,6 +189,9 @@ namespace openshot { video_position = new_position; last_video_position = 0; + // Drop local frame reference so same-frame refreshes cannot reuse stale + // content after timeline/clip property updates. + frame.reset(); // 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); diff --git a/src/Qt/VideoCacheThread.cpp b/src/Qt/VideoCacheThread.cpp index ab0ef95e..ddc737b8 100644 --- a/src/Qt/VideoCacheThread.cpp +++ b/src/Qt/VideoCacheThread.cpp @@ -94,6 +94,8 @@ namespace openshot if (new_speed != 0) { last_speed.store(new_speed); last_dir.store(new_speed > 0 ? 1 : -1); + // Leaving paused/scrub context: resume normal cache behavior. + scrub_active.store(false); } speed.store(new_speed); } @@ -168,6 +170,9 @@ namespace openshot should_clear_cache = true; } else if (cache) { + if (cache_contains) { + cache->Remove(new_position); + } new_cached_count = cache->Count(); } entering_scrub = true; diff --git a/src/Timeline.cpp b/src/Timeline.cpp index f8648423..d5821ca9 100644 --- a/src/Timeline.cpp +++ b/src/Timeline.cpp @@ -1485,6 +1485,12 @@ void Timeline::apply_json_to_clips(Json::Value change) { final_cache->Remove(old_starting_frame - 8, old_ending_frame + 8); final_cache->Remove(new_starting_frame - 8, new_ending_frame + 8); + // Clear transformed/composited clip cache (keyed by clip frame number), + // since property updates (e.g. alpha) can change all rendered frames. + if (existing_clip->GetCache()) { + existing_clip->GetCache()->Clear(); + } + // Remove cache on clip's Reader (if found) if (existing_clip->Reader() && existing_clip->Reader()->GetCache()) { existing_clip->Reader()->GetCache()->Remove(old_starting_frame - 8, old_ending_frame + 8); diff --git a/tests/Clip.cpp b/tests/Clip.cpp index 53500d62..e06c34ef 100644 --- a/tests/Clip.cpp +++ b/tests/Clip.cpp @@ -760,6 +760,43 @@ TEST_CASE( "composite_over_opaque_background_blend", "[libopenshot][clip][pr]" ) CHECK(center.blue() == Approx(127).margin(12)); } +TEST_CASE( "cached_frame_not_mutated_by_background_compositing", "[libopenshot][clip][cache]" ) +{ + // Source clip: solid red + openshot::CacheMemory cache; + auto src = std::make_shared(1, 64, 64, "#000000", 0, 2); + src->AddColor(QColor(Qt::red)); + cache.Add(src); + + openshot::DummyReader dummy(openshot::Fraction(30,1), 64, 64, 44100, 2, 1.0, &cache); + dummy.Open(); + + openshot::Clip clip; + clip.Reader(&dummy); + clip.Open(); + clip.display = openshot::FRAME_DISPLAY_NONE; + clip.alpha.AddPoint(1, 0.5); // semi-transparent source to reveal background + + // First composite over blue background (expect purple-ish) + auto bg_blue = std::make_shared(1, 64, 64, "#000000", 0, 2); + bg_blue->AddColor(QColor(Qt::blue)); + auto out_blue = clip.GetFrame(bg_blue, 1); + QColor c1 = out_blue->GetImage()->pixelColor(32, 32); + CHECK(c1.red() == Approx(127).margin(14)); + CHECK(c1.green() == Approx(0).margin(8)); + CHECK(c1.blue() == Approx(127).margin(14)); + + // Second composite of same clip frame over green background should be yellow-ish. + // If cached frame was mutated by the first call, this will incorrectly remain purple-ish. + auto bg_green = std::make_shared(1, 64, 64, "#000000", 0, 2); + bg_green->AddColor(QColor(Qt::green)); + auto out_green = clip.GetFrame(bg_green, 1); + QColor c2 = out_green->GetImage()->pixelColor(32, 32); + CHECK(c2.red() == Approx(127).margin(14)); + CHECK(c2.green() == Approx(127).margin(14)); + CHECK(c2.blue() == Approx(0).margin(8)); +} + TEST_CASE("all_composite_modes_simple_colors", "[libopenshot][clip][composite]") { // Source clip: solid red diff --git a/tests/Timeline.cpp b/tests/Timeline.cpp index b8c14ba0..e4978648 100644 --- a/tests/Timeline.cpp +++ b/tests/Timeline.cpp @@ -80,6 +80,55 @@ public: void SetJsonValue(const Json::Value root) override { ReaderBase::SetJsonValue(root); } }; +class TimelineSolidColorReader : public ReaderBase { +private: + bool is_open = false; + CacheMemory cache; + QColor color; + +public: + TimelineSolidColorReader(int width, + int height, + int fps_num, + int fps_den, + int64_t length_frames, + const QColor& fill_color) + : color(fill_color) { + info.has_video = true; + info.has_audio = false; + info.width = width; + info.height = height; + info.fps = Fraction(fps_num, fps_den); + info.video_length = length_frames; + info.duration = static_cast(length_frames / info.fps.ToDouble()); + info.sample_rate = 48000; + info.channels = 2; + info.audio_stream_index = -1; + } + + openshot::CacheBase* GetCache() override { return &cache; } + bool IsOpen() override { return is_open; } + std::string Name() override { return "TimelineSolidColorReader"; } + void Open() override { is_open = true; } + void Close() override { is_open = false; } + + std::shared_ptr GetFrame(int64_t number) override { + auto frame = std::make_shared(number, info.width, info.height, "#00000000"); + frame->GetImage()->fill(color); + return frame; + } + + std::string Json() const override { return JsonValue().toStyledString(); } + Json::Value JsonValue() const override { + Json::Value root = ReaderBase::JsonValue(); + root["type"] = "TimelineSolidColorReader"; + root["path"] = ""; + return root; + } + void SetJson(const std::string value) override { (void) value; } + void SetJsonValue(const Json::Value root) override { ReaderBase::SetJsonValue(root); } +}; + TEST_CASE( "constructor", "[libopenshot][timeline]" ) { Fraction fps(30000,1000); @@ -1173,6 +1222,88 @@ TEST_CASE( "ApplyJSONDiff insert invalidates overlapping timeline cache", "[libo CHECK(!t.GetCache()->Contains(10)); } +TEST_CASE( "ApplyJSONDiff alpha updates refresh fixed-frame preview content", "[libopenshot][timeline]" ) +{ + // Deterministic solid-color readers avoid any fixture/image ambiguity. + TimelineSolidColorReader base_reader( + /*width=*/64, /*height=*/64, /*fps_num=*/30, /*fps_den=*/1, /*length_frames=*/300, + QColor(10, 200, 20, 255) + ); + TimelineSolidColorReader overlay_reader( + /*width=*/64, /*height=*/64, /*fps_num=*/30, /*fps_den=*/1, /*length_frames=*/300, + QColor(220, 30, 180, 255) + ); + + Clip base_clip(&base_reader); + base_clip.Id("BASE_ALPHA_TEST"); + base_clip.Layer(0); + base_clip.Position(0.0); + base_clip.End(5.0); + + Clip overlay_clip(&overlay_reader); + overlay_clip.Id("OVERLAY_ALPHA_TEST"); + overlay_clip.Layer(1); + overlay_clip.Position(0.0); + overlay_clip.End(5.0); + + Timeline t(64, 64, Fraction(30, 1), 44100, 2, LAYOUT_STEREO); + t.AddClip(&base_clip); + t.AddClip(&overlay_clip); + t.Open(); + + const int64_t frame_number = 1; + + auto apply_alpha = [&](double alpha_value) { + Keyframe alpha_kf(alpha_value); + Json::Value root(Json::arrayValue); + Json::Value change(Json::objectValue); + change["type"] = "update"; + change["partial"] = true; + + Json::Value key(Json::arrayValue); + key.append("clips"); + Json::Value key_id(Json::objectValue); + key_id["id"] = overlay_clip.Id(); + key.append(key_id); + change["key"] = key; + + Json::Value value(Json::objectValue); + value["alpha"] = alpha_kf.JsonValue(); + change["value"] = value; + + root.append(change); + t.ApplyJsonDiff(root.toStyledString()); + + Clip* updated = t.GetClip(overlay_clip.Id()); + REQUIRE(updated != nullptr); + CHECK(updated->alpha.GetValue(frame_number) == Approx(alpha_value).margin(0.0001)); + }; + + // Establish reference colors for alpha=1.0 (top) and alpha=0.0 (bottom). + // Prime cache at fixed frame. + std::shared_ptr initial = t.GetFrame(frame_number); + REQUIRE(initial != nullptr); + REQUIRE(t.GetCache() != nullptr); + REQUIRE(overlay_clip.GetCache() != nullptr); + REQUIRE(t.GetCache()->Contains(frame_number)); + REQUIRE(overlay_clip.GetCache()->Count() > 0); + + // Repeated alpha updates at the same frame must invalidate both timeline and + // clip caches, preventing stale preview frames from being reused. + const std::vector alpha_steps = {0.9, 0.8, 0.7, 0.6, 0.5}; + for (double alpha_value : alpha_steps) { + apply_alpha(alpha_value); + CHECK(!t.GetCache()->Contains(frame_number)); + CHECK(overlay_clip.GetCache()->Count() == 0); + + // Re-request frame to repopulate caches before next update. + std::shared_ptr refreshed = t.GetFrame(frame_number); + REQUIRE(refreshed != nullptr); + CHECK(t.GetCache()->Contains(frame_number)); + CHECK(overlay_clip.GetCache()->Count() > 0); + } +} + TEST_CASE( "ApplyJSONDiff Update Reader Info", "[libopenshot][timeline]" ) { // Create a timeline diff --git a/tests/VideoCacheThread.cpp b/tests/VideoCacheThread.cpp index 21e36aa4..0a7c1096 100644 --- a/tests/VideoCacheThread.cpp +++ b/tests/VideoCacheThread.cpp @@ -318,7 +318,7 @@ TEST_CASE("prefetchWindow: interrupt on userSeeked flag", "[VideoCacheThread]") CHECK(!wasFull); } -TEST_CASE("Seek preview: inside cache is cheap and does not invalidate", "[VideoCacheThread]") { +TEST_CASE("Seek preview: evicts playhead frame to force fresh render", "[VideoCacheThread]") { TestableVideoCacheThread thread; CacheMemory cache(/*max_bytes=*/100000000); Timeline timeline(/*width=*/1280, /*height=*/720, /*fps=*/Fraction(24,1), @@ -335,8 +335,8 @@ TEST_CASE("Seek preview: inside cache is cheap and does not invalidate", "[Video CHECK(thread.isScrubbing()); CHECK(thread.getUserSeekedFlag()); CHECK(!thread.getPrerollOnNextFill()); - CHECK(cache.Contains(100)); - CHECK(cache.Count() >= 2); + CHECK(!cache.Contains(100)); + CHECK(cache.Count() >= 1); } TEST_CASE("Seek preview: outside cache marks uncached without preroll", "[VideoCacheThread]") {