From a7dfc596cae0acef75313c3f7628bc86fa10d526 Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Thu, 13 Nov 2025 16:18:18 -0600 Subject: [PATCH] Fixing regression in Clip::init_reader_rotation() function, which could sometimes override scale_x and scale_y, when a rotation keyframe contained more than 1 point. --- src/Clip.cpp | 44 ++++++++++++++++++++++++++++---------------- tests/Clip.cpp | 43 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 70 insertions(+), 17 deletions(-) diff --git a/src/Clip.cpp b/src/Clip.cpp index 6e56dd81..d85ca591 100644 --- a/src/Clip.cpp +++ b/src/Clip.cpp @@ -144,36 +144,48 @@ void Clip::init_reader_settings() { } void Clip::init_reader_rotation() { - // Don't init rotation if clip already has keyframes. - if (rotation.GetCount() > 0) + // Only apply metadata rotation if clip rotation has not been explicitly set. + if (rotation.GetCount() > 0 || !reader) + return; + + const auto rotate_meta = reader->info.metadata.find("rotate"); + if (rotate_meta == reader->info.metadata.end()) return; - // Get rotation from metadata (if any) float rotate_angle = 0.0f; - if (reader && reader->info.metadata.count("rotate") > 0) { - try { - rotate_angle = strtof(reader->info.metadata["rotate"].c_str(), nullptr); - } catch (const std::exception& e) { - // Leave rotate_angle at default 0.0f - } + try { + rotate_angle = strtof(rotate_meta->second.c_str(), nullptr); + } catch (const std::exception& e) { + return; // ignore invalid metadata } + rotation = Keyframe(rotate_angle); - // Compute uniform scale factors for rotated video. - // Assume reader->info.width and reader->info.height are the clip's natural dimensions. + // Do not overwrite user-authored scale curves. + auto has_default_scale = [](const Keyframe& kf) { + return kf.GetCount() == 1 && fabs(kf.GetPoint(0).co.Y - 1.0) < 0.00001; + }; + if (!has_default_scale(scale_x) || !has_default_scale(scale_y)) + return; + + // No need to adjust scaling when the metadata rotation is effectively zero. + if (fabs(rotate_angle) < 0.0001f) + return; + float w = static_cast(reader->info.width); float h = static_cast(reader->info.height); - float rad = rotate_angle * M_PI / 180.0f; + if (w <= 0.0f || h <= 0.0f) + return; + + float rad = rotate_angle * static_cast(M_PI) / 180.0f; - // Calculate the dimensions of the bounding box for the rotated clip. float new_width = fabs(w * cos(rad)) + fabs(h * sin(rad)); float new_height = fabs(w * sin(rad)) + fabs(h * cos(rad)); + if (new_width <= 0.0f || new_height <= 0.0f) + return; - // To have the rotated clip appear the same size as the unrotated clip, - // compute a uniform scale factor S that brings the bounding box back to (w, h). float uniform_scale = std::min(w / new_width, h / new_height); - // Set scale keyframes uniformly. scale_x = Keyframe(uniform_scale); scale_y = Keyframe(uniform_scale); } diff --git a/tests/Clip.cpp b/tests/Clip.cpp index 62d0377e..9fc118c8 100644 --- a/tests/Clip.cpp +++ b/tests/Clip.cpp @@ -182,6 +182,47 @@ TEST_CASE( "properties", "[libopenshot][clip]" ) delete reader; } +TEST_CASE( "Metadata rotation does not override manual scaling", "[libopenshot][clip]" ) +{ + DummyReader reader(Fraction(24, 1), 640, 480, 48000, 2, 5.0f); + Clip clip; + clip.scale_x = Keyframe(0.5); + clip.scale_y = Keyframe(0.5); + + clip.Reader(&reader); + + REQUIRE(clip.rotation.GetCount() == 0); + CHECK(clip.scale_x.GetPoint(0).co.Y == Approx(0.5).margin(0.00001)); + CHECK(clip.scale_y.GetPoint(0).co.Y == Approx(0.5).margin(0.00001)); +} + +TEST_CASE( "Metadata rotation scales only default clips", "[libopenshot][clip]" ) +{ + DummyReader rotated(Fraction(24, 1), 640, 480, 48000, 2, 5.0f); + rotated.info.metadata["rotate"] = "90"; + + Clip auto_clip; + auto_clip.Reader(&rotated); + + REQUIRE(auto_clip.rotation.GetCount() == 1); + CHECK(auto_clip.rotation.GetPoint(0).co.Y == Approx(90.0).margin(0.00001)); + CHECK(auto_clip.scale_x.GetPoint(0).co.Y == Approx(0.75).margin(0.00001)); + CHECK(auto_clip.scale_y.GetPoint(0).co.Y == Approx(0.75).margin(0.00001)); + + DummyReader rotated_custom(Fraction(24, 1), 640, 480, 48000, 2, 5.0f); + rotated_custom.info.metadata["rotate"] = "90"; + + Clip custom_clip; + custom_clip.scale_x = Keyframe(0.5); + custom_clip.scale_y = Keyframe(0.5); + custom_clip.Reader(&rotated_custom); + + REQUIRE(custom_clip.rotation.GetCount() == 1); + CHECK(custom_clip.rotation.GetPoint(0).co.Y == Approx(90.0).margin(0.00001)); + CHECK(custom_clip.scale_x.GetPoint(0).co.Y == Approx(0.5).margin(0.00001)); + CHECK(custom_clip.scale_y.GetPoint(0).co.Y == Approx(0.5).margin(0.00001)); +} + TEST_CASE( "effects", "[libopenshot][clip]" ) { // Load clip with video @@ -1118,4 +1159,4 @@ TEST_CASE("Reverse time curve (sample-exact, no resampling)", "[libopenshot][cli clip.Close(); r.Close(); cache.Clear(); -} \ No newline at end of file +}