diff --git a/CMakeLists.txt b/CMakeLists.txt index 9a812bb7b..279fce9bd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -24,8 +24,8 @@ For more information, please visit . set(CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/cmake/Modules") ################ PROJECT VERSION #################### -set(PROJECT_VERSION_FULL "1.0.0") -set(PROJECT_SO_VERSION 31) +set(PROJECT_VERSION_FULL "1.0.1") +set(PROJECT_SO_VERSION 32) # Remove the dash and anything following, to get the #.#.# version for project() STRING(REGEX REPLACE "\-.*$" "" VERSION_NUM "${PROJECT_VERSION_FULL}") diff --git a/doc/JSON-KEYFRAME-BENCHMARK.md b/doc/JSON-KEYFRAME-BENCHMARK.md new file mode 100644 index 000000000..90756c21b --- /dev/null +++ b/doc/JSON-KEYFRAME-BENCHMARK.md @@ -0,0 +1,50 @@ + + +# JSON keyframe edit benchmark + +Build `openshot-benchmark`, then run from the repository root: + +```sh +cmake --build build --target openshot-benchmark -j2 +QT_QPA_PLATFORM=offscreen build/tests/openshot-benchmark --test 'Timeline JSON transforms (0 existing keys)' +QT_QPA_PLATFORM=offscreen build/tests/openshot-benchmark --test 'Timeline JSON transforms (1000 existing keys)' +``` + +Each case uses an image clip on layer 5 of a 1280×720, 24 fps timeline. It +starts with the specified number of keys **per axis**, then adds 100 successive +`location_x` / `location_y` samples with Bezier interpolation and handles. Each +JSON diff contains the complete growing curves, matching the editor's transform +update format. Both axes are sent in one diff. The result verifies the final +point counts and values so a discarded update cannot appear faster. + +Payload generation, serialization, initial population, and clip/timeline setup +are outside the timer. The measured operation is `Timeline::ApplyJsonDiff`, +including JSON parsing, native keyframe replacement, and cache invalidation. +There is no concurrent player or frame rendering. This isolates application cost; +it is not a measurement of end-to-end GUI responsiveness or lock contention. + +The benchmark reports the median operations/second of three trials. One operation +is a JSON update here; for the existing rendering trials it is a frame. +Milliseconds per update = `1000 / operations_per_second`. + +## Local comparison + +Baseline: release-20260919 at cfa5dfa3, before native JSON/keyframe optimizations. +Optimized build: local changes described below. Both use the same benchmark, +build configuration (`-O3 -DNDEBUG`), system JsonCpp, and machine. These timings +are local measurements, not cross-machine performance guarantees. + +| Existing keys per axis | Baseline ms/update | Optimized ms/update | +| --- | ---: | ---: | +| 0 (grows to 100) | 0.82 | 0.33 | +| 1,000 (grows to 1,100) | 17.99 | 6.73 | + +The changes transfer already-owned JSON values through the existing by-value +interfaces, avoid copying derived animation data just to load base clip fields, +and retain keyframe storage with a fast append path for sorted input. Unsorted +and duplicate frame coordinates still use the existing `AddPoint` behavior. +Existing public signatures, virtual method layout, and the JSON format are unchanged. +The complete incoming animation still has to be parsed on every update. diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 7452807ac..ce95a60d2 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -237,7 +237,7 @@ add_feature_info("Wayland screen capture" OPENSHOT_WAYLAND_CAPTURE "Use the opti # Find JUCE-based openshot Audio libraries if(NOT TARGET OpenShot::Audio) # Only load if necessary (not for integrated builds) - find_package(OpenShotAudio 1.0.0 REQUIRED) + find_package(OpenShotAudio 1.0.1 REQUIRED) endif() target_link_libraries(openshot PUBLIC OpenShot::Audio) diff --git a/src/Clip.cpp b/src/Clip.cpp index a03df145e..90e8eee81 100644 --- a/src/Clip.cpp +++ b/src/Clip.cpp @@ -10,6 +10,7 @@ // // SPDX-License-Identifier: LGPL-3.0-or-later +#include #include "Clip.h" #include "AudioResampler.h" @@ -77,6 +78,7 @@ void Clip::init_settings() ClipBase::End(0.0); gravity = GRAVITY_CENTER; scale = SCALE_FIT; + location_coordinate_system = "auto"; anchor = ANCHOR_CANVAS; display = FRAME_DISPLAY_NONE; mixing = VOLUME_MIX_NONE; @@ -968,6 +970,7 @@ Json::Value Clip::JsonValue() const { root["parentObjectId"] = parentObjectId; root["gravity"] = gravity; root["scale"] = scale; + root["location_coordinate_system"] = location_coordinate_system; root["anchor"] = anchor; root["display"] = display; root["mixing"] = mixing; @@ -1035,9 +1038,9 @@ void Clip::SetJson(const std::string value) { // Parse JSON string into JSON objects try { - const Json::Value root = openshot::stringToJson(value); + Json::Value root = openshot::stringToJson(value); // Set all values that match - SetJsonValue(root); + SetJsonValue(std::move(root)); } catch (const std::exception& e) { @@ -1047,7 +1050,7 @@ void Clip::SetJson(const std::string value) { } // Load Json::Value into this object -void Clip::SetJsonValue(const Json::Value root) { +void Clip::SetJsonValue(Json::Value root) { auto ensure_default_keyframe = [](Keyframe& kf, double default_value) { if (kf.GetCount() == 0) { kf = Keyframe(default_value); @@ -1055,7 +1058,7 @@ void Clip::SetJsonValue(const Json::Value root) { }; // Set parent data - ClipBase::SetJsonValue(root); + SetBaseJsonValue(root); // Older project files predate reader-applied orientation metadata and stored // phone/camera rotation as ordinary clip rotation/scale keyframes. @@ -1084,6 +1087,12 @@ void Clip::SetJsonValue(const Json::Value root) { gravity = (GravityType) root["gravity"].asInt(); if (!root["scale"].isNull()) scale = (ScaleType) root["scale"].asInt(); + if (!root["location_coordinate_system"].isNull()) { + const auto& coordinates = root["location_coordinate_system"]; + location_coordinate_system = coordinates.isString() ? coordinates.asString() : "auto"; + if (location_coordinate_system != "canvas" && location_coordinate_system != "geometry") + location_coordinate_system = "auto"; + } if (!root["anchor"].isNull()) anchor = (AnchorType) root["anchor"].asInt(); if (!root["display"].isNull()) @@ -1097,59 +1106,59 @@ void Clip::SetJsonValue(const Json::Value root) { if (!root["waveform_mode"].isNull()) waveform_mode = root["waveform_mode"].asInt(); if (!root["scale_x"].isNull()) - scale_x.SetJsonValue(root["scale_x"]); + scale_x.SetJsonValue(std::move(root["scale_x"])); if (!root["scale_y"].isNull()) - scale_y.SetJsonValue(root["scale_y"]); + scale_y.SetJsonValue(std::move(root["scale_y"])); if (!root["location_x"].isNull()) - location_x.SetJsonValue(root["location_x"]); + location_x.SetJsonValue(std::move(root["location_x"])); if (!root["location_y"].isNull()) - location_y.SetJsonValue(root["location_y"]); + location_y.SetJsonValue(std::move(root["location_y"])); if (!root["alpha"].isNull()) - alpha.SetJsonValue(root["alpha"]); + alpha.SetJsonValue(std::move(root["alpha"])); if (!root["corner_radius"].isNull()) - corner_radius.SetJsonValue(root["corner_radius"]); + corner_radius.SetJsonValue(std::move(root["corner_radius"])); if (!root["margin"].isNull()) - margin.SetJsonValue(root["margin"]); + margin.SetJsonValue(std::move(root["margin"])); if (!root["rotation"].isNull()) - rotation.SetJsonValue(root["rotation"]); + rotation.SetJsonValue(std::move(root["rotation"])); if (!root["time"].isNull()) - time.SetJsonValue(root["time"]); + time.SetJsonValue(std::move(root["time"])); if (!root["volume"].isNull()) - volume.SetJsonValue(root["volume"]); + volume.SetJsonValue(std::move(root["volume"])); if (!root["wave_color"].isNull()) - wave_color.SetJsonValue(root["wave_color"]); + wave_color.SetJsonValue(std::move(root["wave_color"])); if (!root["shear_x"].isNull()) - shear_x.SetJsonValue(root["shear_x"]); + shear_x.SetJsonValue(std::move(root["shear_x"])); if (!root["shear_y"].isNull()) - shear_y.SetJsonValue(root["shear_y"]); + shear_y.SetJsonValue(std::move(root["shear_y"])); if (!root["origin_x"].isNull()) - origin_x.SetJsonValue(root["origin_x"]); + origin_x.SetJsonValue(std::move(root["origin_x"])); if (!root["origin_y"].isNull()) - origin_y.SetJsonValue(root["origin_y"]); + origin_y.SetJsonValue(std::move(root["origin_y"])); if (!root["channel_filter"].isNull()) - channel_filter.SetJsonValue(root["channel_filter"]); + channel_filter.SetJsonValue(std::move(root["channel_filter"])); if (!root["channel_mapping"].isNull()) - channel_mapping.SetJsonValue(root["channel_mapping"]); + channel_mapping.SetJsonValue(std::move(root["channel_mapping"])); if (!root["has_audio"].isNull()) - has_audio.SetJsonValue(root["has_audio"]); + has_audio.SetJsonValue(std::move(root["has_audio"])); if (!root["has_video"].isNull()) - has_video.SetJsonValue(root["has_video"]); + has_video.SetJsonValue(std::move(root["has_video"])); if (!root["perspective_c1_x"].isNull()) - perspective_c1_x.SetJsonValue(root["perspective_c1_x"]); + perspective_c1_x.SetJsonValue(std::move(root["perspective_c1_x"])); if (!root["perspective_c1_y"].isNull()) - perspective_c1_y.SetJsonValue(root["perspective_c1_y"]); + perspective_c1_y.SetJsonValue(std::move(root["perspective_c1_y"])); if (!root["perspective_c2_x"].isNull()) - perspective_c2_x.SetJsonValue(root["perspective_c2_x"]); + perspective_c2_x.SetJsonValue(std::move(root["perspective_c2_x"])); if (!root["perspective_c2_y"].isNull()) - perspective_c2_y.SetJsonValue(root["perspective_c2_y"]); + perspective_c2_y.SetJsonValue(std::move(root["perspective_c2_y"])); if (!root["perspective_c3_x"].isNull()) - perspective_c3_x.SetJsonValue(root["perspective_c3_x"]); + perspective_c3_x.SetJsonValue(std::move(root["perspective_c3_x"])); if (!root["perspective_c3_y"].isNull()) - perspective_c3_y.SetJsonValue(root["perspective_c3_y"]); + perspective_c3_y.SetJsonValue(std::move(root["perspective_c3_y"])); if (!root["perspective_c4_x"].isNull()) - perspective_c4_x.SetJsonValue(root["perspective_c4_x"]); + perspective_c4_x.SetJsonValue(std::move(root["perspective_c4_x"])); if (!root["perspective_c4_y"].isNull()) - perspective_c4_y.SetJsonValue(root["perspective_c4_y"]); + perspective_c4_y.SetJsonValue(std::move(root["perspective_c4_y"])); // Core clip transforms should never remain empty after load. Empty JSON // point arrays can be produced by editing flows that remove every keyframe. @@ -1692,7 +1701,10 @@ QTransform Clip::get_transform(std::shared_ptr frame, int width, int heig } return location * (canvas_size - anchored_position); }; - if (scale == SCALE_CROP) { + // Preserve imported location curves in their original units. Converting only + // their keyframes cannot preserve animated scale/margin or reader resizing. + if (location_coordinate_system == "geometry" || + (location_coordinate_system != "canvas" && scale == SCALE_CROP)) { x += location_offset(location_x_value, x - layout_x, layout_width, scaled_source_width); y += location_offset(location_y_value, y - layout_y, layout_height, scaled_source_height); } else { diff --git a/src/Clip.h b/src/Clip.h index 8901141e9..072cb0c4e 100644 --- a/src/Clip.h +++ b/src/Clip.h @@ -184,6 +184,9 @@ namespace openshot { public: openshot::GravityType gravity; ///< The gravity of a clip determines where it snaps to its parent openshot::ScaleType scale; ///< The scale determines how a clip should be resized to fit its parent + /// Location units retained by imported projects: "auto", "canvas", or "geometry". + /// Auto uses geometry-relative units for Crop and canvas-relative units otherwise. + std::string location_coordinate_system; openshot::AnchorType anchor; ///< The anchor determines what parent a clip should snap to openshot::FrameDisplayType display; ///< The format to display the frame number (if any) openshot::VolumeMixType mixing; ///< What strategy should be followed when mixing audio with other clips diff --git a/src/ClipBase.cpp b/src/ClipBase.cpp index 4d1418edf..39eb8a4a3 100644 --- a/src/ClipBase.cpp +++ b/src/ClipBase.cpp @@ -78,6 +78,10 @@ Json::Value ClipBase::JsonValue() const { // Load Json::Value into this object void ClipBase::SetJsonValue(const Json::Value root) { + SetBaseJsonValue(root); +} + +void ClipBase::SetBaseJsonValue(const Json::Value& root) { // Set data from Json (if key is found) if (!root["id"].isNull()) diff --git a/src/ClipBase.h b/src/ClipBase.h index 732160c16..ff1ab755c 100644 --- a/src/ClipBase.h +++ b/src/ClipBase.h @@ -39,6 +39,9 @@ namespace openshot { std::string previous_properties; ///< This string contains the previous JSON properties openshot::TimelineBase* timeline; ///< Pointer to the parent timeline instance (if any) + /// Load common clip fields without copying derived animation data. + void SetBaseJsonValue(const Json::Value& root); + /// Generate JSON for a property Json::Value add_property_json(std::string name, float value, std::string type, std::string memo, const Keyframe* keyframe, float min_value, float max_value, bool readonly, int64_t requested_frame) const; diff --git a/src/Color.cpp b/src/Color.cpp index 7cc15f384..a65e7e1fd 100644 --- a/src/Color.cpp +++ b/src/Color.cpp @@ -10,6 +10,7 @@ // // SPDX-License-Identifier: LGPL-3.0-or-later +#include #include #include "Color.h" @@ -102,9 +103,9 @@ void Color::SetJson(const std::string value) { // Parse JSON string into JSON objects try { - const Json::Value root = openshot::stringToJson(value); + Json::Value root = openshot::stringToJson(value); // Set all values that match - SetJsonValue(root); + SetJsonValue(std::move(root)); } catch (const std::exception& e) { @@ -114,15 +115,15 @@ void Color::SetJson(const std::string value) { } // Load Json::Value into this object -void Color::SetJsonValue(const Json::Value root) { +void Color::SetJsonValue(Json::Value root) { // Set data from Json (if key is found) if (!root["red"].isNull()) - red.SetJsonValue(root["red"]); + red.SetJsonValue(std::move(root["red"])); if (!root["green"].isNull()) - green.SetJsonValue(root["green"]); + green.SetJsonValue(std::move(root["green"])); if (!root["blue"].isNull()) - blue.SetJsonValue(root["blue"]); + blue.SetJsonValue(std::move(root["blue"])); if (!root["alpha"].isNull()) - alpha.SetJsonValue(root["alpha"]); + alpha.SetJsonValue(std::move(root["alpha"])); } diff --git a/src/EffectBase.cpp b/src/EffectBase.cpp index d3fd1c80d..415e1c9af 100644 --- a/src/EffectBase.cpp +++ b/src/EffectBase.cpp @@ -10,6 +10,7 @@ // // SPDX-License-Identifier: LGPL-3.0-or-later +#include #include #include #include @@ -136,7 +137,7 @@ void EffectBase::SetJson(const std::string value) { { Json::Value root = openshot::stringToJson(value); // Set all values that match - SetJsonValue(root); + SetJsonValue(std::move(root)); } catch (const std::exception& e) { @@ -146,7 +147,7 @@ void EffectBase::SetJson(const std::string value) { } // Load Json::Value into this object -void EffectBase::SetJsonValue(const Json::Value root) { +void EffectBase::SetJsonValue(Json::Value root) { const std::string original_id = this->Id(); const std::string original_parent_effect_id = this->info.parent_effect_id; @@ -159,7 +160,7 @@ void EffectBase::SetJsonValue(const Json::Value root) { root["id"].asString() == original_parent_effect_id && root["id"].asString() != original_id; if (applying_parent_payload) { - my_root = root; + my_root = std::move(root); my_root["id"] = original_id; my_root["parent_effect_id"] = original_parent_effect_id; } else if (parentEffect){ @@ -167,7 +168,7 @@ void EffectBase::SetJsonValue(const Json::Value root) { my_root["id"] = this->Id(); my_root["parent_effect_id"] = this->info.parent_effect_id; } else { - my_root = root; + my_root = std::move(root); } // Legacy compatibility: older shared-mask JSON stored source trim @@ -178,7 +179,7 @@ void EffectBase::SetJsonValue(const Json::Value root) { my_root["end"] = my_root["mask_end"]; // Set parent data - ClipBase::SetJsonValue(my_root); + SetBaseJsonValue(my_root); // Set data from Json (if key is found) if (!my_root["order"].isNull()) diff --git a/src/FFmpegColorRange.h b/src/FFmpegColorRange.h new file mode 100644 index 000000000..383f133a0 --- /dev/null +++ b/src/FFmpegColorRange.h @@ -0,0 +1,31 @@ +// SPDX-FileCopyrightText: 2026 OpenShot Studios, LLC +// SPDX-License-Identifier: LGPL-3.0-or-later +#pragma once + +#include "FFmpegUtilities.h" + +// Normalize deprecated JPEG-range YUVJ formats before creating swscale contexts. +// swscale expects non-YUVJ formats plus explicit color-range metadata. +inline AVPixelFormat NormalizeDeprecatedPixFmt(AVPixelFormat pix_fmt, bool& is_full_range) { + switch (pix_fmt) { + case AV_PIX_FMT_YUVJ420P: + is_full_range = true; + return AV_PIX_FMT_YUV420P; + case AV_PIX_FMT_YUVJ422P: + is_full_range = true; + return AV_PIX_FMT_YUV422P; + case AV_PIX_FMT_YUVJ444P: + is_full_range = true; + return AV_PIX_FMT_YUV444P; + case AV_PIX_FMT_YUVJ440P: + is_full_range = true; + return AV_PIX_FMT_YUV440P; +#ifdef AV_PIX_FMT_YUVJ411P + case AV_PIX_FMT_YUVJ411P: + is_full_range = true; + return AV_PIX_FMT_YUV411P; +#endif + default: + return pix_fmt; + } +} diff --git a/src/FFmpegReader.cpp b/src/FFmpegReader.cpp index f6564a55e..ca32da3fd 100644 --- a/src/FFmpegReader.cpp +++ b/src/FFmpegReader.cpp @@ -23,6 +23,8 @@ #include #include "FFmpegUtilities.h" +#include "FFmpegColorRange.h" +#include "PreviewSize.h" #include "effects/CropHelpers.h" #include "FFmpegReader.h" @@ -75,31 +77,6 @@ int hw_de_on = 0; AVHWDeviceType hw_de_av_device_type_global = AV_HWDEVICE_TYPE_NONE; #endif -// Normalize deprecated JPEG-range YUVJ formats before creating swscale contexts. -// swscale expects non-YUVJ formats plus explicit color-range metadata. -static AVPixelFormat NormalizeDeprecatedPixFmt(AVPixelFormat pix_fmt, bool& is_full_range) { - switch (pix_fmt) { - case AV_PIX_FMT_YUVJ420P: - is_full_range = true; - return AV_PIX_FMT_YUV420P; - case AV_PIX_FMT_YUVJ422P: - is_full_range = true; - return AV_PIX_FMT_YUV422P; - case AV_PIX_FMT_YUVJ444P: - is_full_range = true; - return AV_PIX_FMT_YUV444P; - case AV_PIX_FMT_YUVJ440P: - is_full_range = true; - return AV_PIX_FMT_YUV440P; -#ifdef AV_PIX_FMT_YUVJ411P - case AV_PIX_FMT_YUVJ411P: - is_full_range = true; - return AV_PIX_FMT_YUV411P; -#endif - default: - return pix_fmt; - } -} FFmpegReader::FFmpegReader(const std::string &path, bool inspect_reader) : FFmpegReader(path, DurationStrategy::VideoPreferred, inspect_reader) {} @@ -2016,7 +1993,7 @@ void FFmpegReader::ProcessVideoPacket(int64_t requested_frame) { // Determine if image needs to be scaled (for performance reasons) int original_height = src_height; - if (max_width != 0 && max_height != 0 && max_width < width && max_height < height) { + if (max_width > 0 && max_height > 0 && (max_width < width || max_height < height)) { // Override width and height (but maintain aspect ratio) float ratio = float(width) / float(height); int possible_width = round(max_height * ratio); @@ -2031,6 +2008,13 @@ void FFmpegReader::ProcessVideoPacket(int64_t requested_frame) { width = max_width; height = possible_height; } + // Clip aspect ratios, animated scale/crop, and decode-size limits can + // undo timeline alignment. Align the final reduced RGB dimensions here. + if (src_width >= 4 && src_height >= 4) { + const QSize aligned = AlignPreviewSize(QSize(width, height)); + width = aligned.width(); + height = aligned.height(); + } } // Determine required buffer size and allocate buffer diff --git a/src/FFmpegWriter.cpp b/src/FFmpegWriter.cpp index a9adb9e04..012c2d3f2 100644 --- a/src/FFmpegWriter.cpp +++ b/src/FFmpegWriter.cpp @@ -945,7 +945,6 @@ void FFmpegWriter::flush_encoders() { if (pkt->duration <= 0) { pkt->duration = audio_codec_ctx->frame_size > 0 ? audio_codec_ctx->frame_size : audio_input_frame_size; } - const int64_t packet_duration = pkt->duration; av_packet_rescale_ts(pkt, audio_codec_ctx->time_base, audio_st->time_base); pkt->stream_index = audio_st->index; pkt->flags |= AV_PKT_FLAG_KEY; @@ -957,7 +956,7 @@ void FFmpegWriter::flush_encoders() { + av_err2string(error_code) + "]", "error_code", error_code); } - audio_timestamp += packet_duration; + // Flushing emits already-submitted samples; do not advance the input clock. AV_FREE_PACKET(pkt); } av_packet_free(&pkt); @@ -974,8 +973,7 @@ void FFmpegWriter::flush_encoders() { break; } - // Since the PTS can change during encoding, set the value again. This seems like a huge hack, - // but it fixes lots of PTS related issues when I do this. + // Legacy encoding API timestamp fallback. pkt->pts = pkt->dts = audio_timestamp; if (pkt->duration <= 0) { pkt->duration = audio_codec_ctx->frame_size > 0 ? audio_codec_ctx->frame_size : audio_input_frame_size; @@ -1801,7 +1799,7 @@ void FFmpegWriter::write_audio_packets(bool is_final, std::shared_ptrnb_samples = total_frame_samples / channels_in_frame; + audio_converted->nb_samples = total_frame_samples / info.channels; av_samples_alloc(audio_converted->data, audio_converted->linesize, info.channels, audio_converted->nb_samples, output_sample_fmt, 0); Logger::Instance()->AppendDebugMethod( @@ -1850,8 +1848,11 @@ void FFmpegWriter::write_audio_packets(bool is_final, std::shared_ptrnb_samples // number of input samples to convert ); - // Set remaining samples - remaining_frame_samples = total_frame_samples; + // Resampling can buffer samples internally. Only copy samples actually + // returned, not the estimated capacity (which can read past the buffer). + if (nb_samples < 0) + throw ErrorEncodingAudio("Could not resample audio", nb_samples); + remaining_frame_samples = nb_samples * info.channels; // Create a new array (to hold all resampled S16 audio samples) all_resampled_samples = (int16_t *) av_malloc( @@ -2109,11 +2110,14 @@ void FFmpegWriter::write_audio_packets(bool is_final, std::shared_ptrpts = pkt->dts = audio_timestamp; + if (error_code == 0 && got_packet_ptr > 0) { + + // Delayed encoders (AAC) return timestamps for earlier input frames. + // Preserve those timestamps, just as the flush path does. + if (pkt->pts == AV_NOPTS_VALUE) + pkt->pts = audio_timestamp; + if (pkt->dts == AV_NOPTS_VALUE) + pkt->dts = pkt->pts; if (pkt->duration <= 0) { pkt->duration = frame_nb_samples; } @@ -2136,8 +2140,8 @@ void FFmpegWriter::write_audio_packets(bool is_final, std::shared_ptrdata[0])); @@ -2177,8 +2181,20 @@ AVFrame *FFmpegWriter::allocate_avframe(PixelFormat pix_fmt, int width, int heig // Create buffer (if not provided) if (!new_buffer) { + int palette_padding = 0; +#ifdef AV_PIX_FMT_FLAG_PSEUDOPAL + // Old av_frame_ref/av_image_copy still read a synthetic RGB8 palette, + // even though av_image_get_buffer_size excludes it. Keep it readable + // when the encoder takes a reference to this externally owned buffer. + if (av_pix_fmt_desc_get(pix_fmt)->flags & AV_PIX_FMT_FLAG_PSEUDOPAL) + palette_padding = 1024; +#endif // New Buffer - new_buffer = (uint8_t *) av_malloc(*buffer_size * sizeof(uint8_t)); + new_buffer = (uint8_t *) av_malloc(*buffer_size + palette_padding); + if (!new_buffer) + throw OutOfMemory("Could not allocate video frame buffer", path); + if (palette_padding) + memset(new_buffer + *buffer_size, 0, palette_padding); // Attach buffer to AVFrame AV_COPY_PICTURE_DATA(new_av_frame, new_buffer, pix_fmt, width, height); new_av_frame->width = width; @@ -2233,7 +2249,7 @@ void FFmpegWriter::process_video_packet(std::shared_ptr frame) { persistent_dst_frame->height = info.height; persistent_dst_size = av_image_get_buffer_size( - dst_fmt, info.width, info.height, 1 + dst_fmt, info.width, info.height, 32 ); if (persistent_dst_size < 0) throw ErrorEncodingVideo("Invalid destination image size", -1); @@ -2251,7 +2267,7 @@ void FFmpegWriter::process_video_packet(std::shared_ptr frame) { dst_fmt, info.width, info.height, - 1 + 32 ); } @@ -2305,12 +2321,19 @@ void FFmpegWriter::process_video_packet(std::shared_ptr frame) { if (!new_frame) throw OutOfMemory("Could not allocate new_frame via allocate_avframe", path); - // Copy persistent_dst_buffer → new_frame buffer - memcpy( - new_frame->data[0], - persistent_dst_buffer, - static_cast(bytes_final) - ); + // Copy visible pixels from padded scaler planes into the packed encoder frame. +#ifdef AV_PIX_FMT_FLAG_PSEUDOPAL + // Older FFmpeg's av_image_copy also copies a synthetic palette for RGB8 + // (GIF), although av_image_get_buffer_size no longer allocates that palette. + if (av_pix_fmt_desc_get(dst_fmt)->flags & AV_PIX_FMT_FLAG_PSEUDOPAL) { + av_image_copy_plane(new_frame->data[0], new_frame->linesize[0], + persistent_dst_frame->data[0], persistent_dst_frame->linesize[0], + av_image_get_linesize(dst_fmt, info.width, 0), info.height); + } else +#endif + av_image_copy(new_frame->data, new_frame->linesize, + const_cast(persistent_dst_frame->data), + persistent_dst_frame->linesize, dst_fmt, info.width, info.height); // Queue the deep‐copied frame for encoding add_avframe(frame, new_frame); diff --git a/src/KeyFrame.cpp b/src/KeyFrame.cpp index 8c7615816..ebf95cb56 100644 --- a/src/KeyFrame.cpp +++ b/src/KeyFrame.cpp @@ -357,9 +357,9 @@ void Keyframe::SetJson(const std::string value) { // Parse JSON string into JSON objects try { - const Json::Value root = openshot::stringToJson(value); + Json::Value root = openshot::stringToJson(value); // Set all values that match - SetJsonValue(root); + SetJsonValue(std::move(root)); } catch (const std::exception& e) { @@ -369,22 +369,28 @@ void Keyframe::SetJson(const std::string value) { } // Load Json::Value into this object -void Keyframe::SetJsonValue(const Json::Value root) { +void Keyframe::SetJsonValue(Json::Value root) { // Clear existing points Points.clear(); - Points.shrink_to_fit(); if (root.isObject() && !root["Points"].isNull()) { + // Reuse allocation across edits, and append the usual sorted input directly. + const size_t count = root["Points"].size(); + if (count > Points.capacity()) + Points.reserve(std::max(count, Points.capacity() * 2)); // loop through points in JSON Object - for (const auto existing_point : root["Points"]) { + for (auto& existing_point : root["Points"]) { // Create Point Point p; // Load Json into Point - p.SetJsonValue(existing_point); + p.SetJsonValue(std::move(existing_point)); // Add Point to Keyframe - AddPoint(p); + if (Points.empty() || Points.back().co.X < p.co.X) + Points.push_back(p); + else + AddPoint(p); // Preserve ordering and last-wins duplicate handling. } } else if (root.isNumeric()) { // Create Point from Numeric value diff --git a/src/Point.cpp b/src/Point.cpp index 8e7e54729..d7ef2c2b8 100644 --- a/src/Point.cpp +++ b/src/Point.cpp @@ -10,6 +10,7 @@ // // SPDX-License-Identifier: LGPL-3.0-or-later +#include #include "Point.h" #include "Exceptions.h" @@ -89,9 +90,9 @@ void Point::SetJson(const std::string value) { // Parse JSON string into JSON objects try { - const Json::Value root = openshot::stringToJson(value); + Json::Value root = openshot::stringToJson(value); // Set all values that match - SetJsonValue(root); + SetJsonValue(std::move(root)); } catch (const std::exception& e) { @@ -101,14 +102,14 @@ void Point::SetJson(const std::string value) { } // Load Json::Value into this object -void Point::SetJsonValue(const Json::Value root) { +void Point::SetJsonValue(Json::Value root) { if (!root["co"].isNull()) - co.SetJsonValue(root["co"]); // update coordinate + co.SetJsonValue(std::move(root["co"])); // update coordinate if (!root["handle_left"].isNull()) - handle_left.SetJsonValue(root["handle_left"]); // update coordinate + handle_left.SetJsonValue(std::move(root["handle_left"])); // update coordinate if (!root["handle_right"].isNull()) - handle_right.SetJsonValue(root["handle_right"]); // update coordinate + handle_right.SetJsonValue(std::move(root["handle_right"])); // update coordinate if (!root["interpolation"].isNull()) interpolation = (InterpolationType) root["interpolation"].asInt(); if (!root["handle_type"].isNull()) diff --git a/src/PreviewSize.h b/src/PreviewSize.h new file mode 100644 index 000000000..3c5832e9e --- /dev/null +++ b/src/PreviewSize.h @@ -0,0 +1,20 @@ +// SPDX-FileCopyrightText: 2026 OpenShot Studios, LLC +// SPDX-License-Identifier: LGPL-3.0-or-later + +#ifndef OPENSHOT_PREVIEW_SIZE_H +#define OPENSHOT_PREVIEW_SIZE_H + +#include +#include + +namespace openshot { +// Apply only to reduced previews, after fitting the aspect ratio. Four RGBA +// pixels occupy 16 bytes. Align both axes so quarter-turn orientation also +// produces aligned rows. Tiny previews have a minimum size of 4x4. +inline QSize AlignPreviewSize(const QSize& size) { + return QSize(std::max(4, size.width() / 4 * 4), + std::max(4, size.height() / 4 * 4)); +} +} + +#endif diff --git a/src/Qt/PlayerPrivate.cpp b/src/Qt/PlayerPrivate.cpp index bacaef73a..3cc3d722d 100644 --- a/src/Qt/PlayerPrivate.cpp +++ b/src/Qt/PlayerPrivate.cpp @@ -23,8 +23,9 @@ namespace openshot int close_to_sync = 5; // Constructor PlayerPrivate::PlayerPrivate(openshot::RendererBase *rb) - : renderer(rb), Thread("player"), video_position(1), audio_position(0), - speed(1), reader(NULL), last_video_position(1), max_sleep_ms(125000), playback_frames(0), is_dirty(true) + : Thread("player"), playback_frames(0), video_position(1), audio_position(0), + reader(nullptr), speed(1), last_speed(1), renderer(rb), + last_video_position(1), max_sleep_ms(125000), is_dirty(true) { videoCache = new openshot::VideoCacheThread(); audioPlayback = new openshot::AudioPlaybackThread(videoCache); @@ -97,10 +98,16 @@ namespace openshot } // Get the current video frame - frame = getFrame(); + const uint64_t frame_seek_generation = seek_generation.load(); + auto frame_to_render = getFrame(); + // A seek during decoding invalidates the old frame. Do not hand + // that frame to the renderer or overwrite the new playhead. + if (seek_generation.load() != frame_seek_generation) + continue; + const int64_t rendered_position = video_position.load(); // Set the video frame on the video thread and render frame - videoPlayback->frame = frame; + videoPlayback->SetFrame(std::move(frame_to_render)); videoPlayback->rendered.reset(); videoPlayback->render.signal(); // Keep decode/position advancement aligned with actual preview updates. @@ -113,7 +120,11 @@ namespace openshot videoPlayback->rendered.wait(render_wait_ms); // Keep track of the last displayed frame - last_video_position = video_position; + { + std::lock_guard lock(frame_mutex); + if (seek_generation.load() == frame_seek_generation) + last_video_position = rendered_position; + } last_speed = speed; // Calculate the diff between 'now' and the predicted frame end time @@ -142,51 +153,60 @@ namespace openshot // Get the next displayed frame (based on speed and direction) std::shared_ptr PlayerPrivate::getFrame() { - try { - // Getting new frame, so clear this flag - is_dirty = false; - - // Get the next frame (based on speed) - if (video_position + speed >= 1 && video_position + speed <= reader->info.video_length) { - video_position = video_position + speed; - - } else if (video_position + speed < 1) { - // Start of reader (prevent negative frame number and pause playback) - video_position = 1; - speed = 0; - } else if (video_position + speed > reader->info.video_length) { - // End of reader (prevent negative frame number and pause playback) - video_position = reader->info.video_length; - speed = 0; - } - - if (frame && frame->number == video_position && video_position == last_video_position) { - // return cached frame - return frame; - } - else - { - // Increment playback frames (always in the positive direction) - playback_frames += std::abs(speed); - - // Update playhead hint for cache window tracking without triggering seek behavior. - videoCache->NotifyPlaybackPosition(video_position); - - // return frame from reader - return reader->GetFrame(video_position); - } - - } catch (const ReaderClosed & e) { - // ... - } catch (const OutOfBoundsFrame & e) { - // ... - } - return std::shared_ptr(); + int64_t position; + uint64_t generation; + ReaderBase* current_reader; + { + std::lock_guard lock(frame_mutex); + // Getting new frame, so clear this flag + is_dirty = false; + + // Get the next frame (based on speed) + const int current_speed = speed.load(); + const int64_t next_position = video_position.load() + current_speed; + if (next_position >= 1 && next_position <= reader->info.video_length) { + video_position = next_position; + } else if (next_position < 1) { + video_position = 1; + speed = 0; + } else { + video_position = reader->info.video_length; + speed = 0; + } + + position = video_position.load(); + if (frame && frame->number == position && position == last_video_position) + return frame; + // Increment playback frames (always in the positive direction) + playback_frames += std::abs(speed.load()); + // Update cache position before releasing the seek lock. + videoCache->NotifyPlaybackPosition(position); + current_reader = reader; + generation = seek_generation.load(); + } + + // Decoding can take much longer than a frame. Keep Seek responsive while + // it runs, then publish only if no newer seek invalidated the result. + std::shared_ptr decoded; + try { + decoded = current_reader->GetFrame(position); + } catch (const ReaderClosed &) { + } catch (const OutOfBoundsFrame &) { + } + { + std::lock_guard lock(frame_mutex); + if (seek_generation.load() != generation) + return {}; + frame = std::move(decoded); + return frame; + } } // Seek to a new position void PlayerPrivate::Seek(int64_t new_position) { + std::lock_guard lock(frame_mutex); + ++seek_generation; video_position = new_position; last_video_position = 0; // Drop local frame reference so same-frame refreshes cannot reuse stale diff --git a/src/Qt/PlayerPrivate.h b/src/Qt/PlayerPrivate.h index 303ba2c2e..e015bbe20 100644 --- a/src/Qt/PlayerPrivate.h +++ b/src/Qt/PlayerPrivate.h @@ -20,6 +20,8 @@ #include "../Qt/AudioPlaybackThread.h" #include "../Qt/VideoPlaybackThread.h" #include "../Qt/VideoCacheThread.h" +#include +#include namespace openshot { @@ -30,19 +32,21 @@ namespace openshot class PlayerPrivate : juce::Thread { std::shared_ptr frame; /// The current frame + std::mutex frame_mutex; /// Protects frame publication and seek state transitions + std::atomic seek_generation{0}; int64_t playback_frames; /// The # of frames since playback started - int64_t video_position; /// The current frame position. + std::atomic video_position; /// The current frame position. int64_t audio_position; /// The current frame position. openshot::ReaderBase *reader; /// The reader which powers this player openshot::AudioPlaybackThread *audioPlayback; /// The audio thread openshot::VideoPlaybackThread *videoPlayback; /// The video thread openshot::VideoCacheThread *videoCache; /// The cache thread - int speed; /// The speed and direction to playback a reader (1=normal, 2=fast, 3=faster, -1=rewind, etc...) + std::atomic speed; /// The speed and direction to playback a reader (1=normal, 2=fast, 3=faster, -1=rewind, etc...) int last_speed; /// The previous speed and direction (used to detect a change) openshot::RendererBase *renderer; - int64_t last_video_position; /// The last frame actually displayed + std::atomic last_video_position; /// The last frame actually displayed int max_sleep_ms; /// The max milliseconds to sleep (when syncing audio and video) - bool is_dirty; /// Detect if a frame needs to be refreshed (calls to Seek() set this to true) + std::atomic is_dirty; /// Detect if a frame needs to be refreshed (calls to Seek() set this to true) /// Constructor PlayerPrivate(openshot::RendererBase *rb); diff --git a/src/Qt/VideoPlaybackThread.cpp b/src/Qt/VideoPlaybackThread.cpp index e716244c9..5a50a4a4b 100644 --- a/src/Qt/VideoPlaybackThread.cpp +++ b/src/Qt/VideoPlaybackThread.cpp @@ -17,6 +17,7 @@ #include "Frame.h" #include "RendererBase.h" #include "Logger.h" +#include namespace openshot { @@ -35,10 +36,14 @@ namespace openshot // Get the currently playing frame number (if any) int64_t VideoPlaybackThread::getCurrentFramePosition() { - if (frame) - return frame->number; - else - return 0; + std::lock_guard lock(frame_mutex); + return frame ? frame->number : 0; + } + + void VideoPlaybackThread::SetFrame(std::shared_ptr next_frame) + { + std::lock_guard lock(frame_mutex); + frame = std::move(next_frame); } // Start the thread @@ -47,17 +52,22 @@ namespace openshot while (!threadShouldExit()) { // Make other threads wait on the render event bool need_render = render.wait(500); + std::shared_ptr frame_to_render; + if (need_render) { + std::lock_guard lock(frame_mutex); + frame_to_render = frame; + } - if (need_render && frame) + if (frame_to_render) { // Debug Logger::Instance()->AppendDebugMethod( "VideoPlaybackThread::run (before render)", - "frame->number", frame->number, + "frame->number", frame_to_render->number, "need_render", need_render); // Render the frame to the screen - renderer->paint(frame); + renderer->paint(frame_to_render); } // Signal to other threads that the rendered event has completed diff --git a/src/Qt/VideoPlaybackThread.h b/src/Qt/VideoPlaybackThread.h index 67bd55c73..dbd9eda7d 100644 --- a/src/Qt/VideoPlaybackThread.h +++ b/src/Qt/VideoPlaybackThread.h @@ -16,6 +16,8 @@ #include #include +#include +#include namespace openshot { @@ -31,6 +33,7 @@ namespace openshot { RendererBase *renderer; std::shared_ptr frame; + std::mutex frame_mutex; WaitableEvent render; WaitableEvent rendered; bool reset; @@ -42,6 +45,7 @@ namespace openshot /// Get the currently playing frame number (if any) int64_t getCurrentFramePosition(); + void SetFrame(std::shared_ptr next_frame); /// Start the thread void run(); diff --git a/src/ReaderBase.h b/src/ReaderBase.h index 304ae4cc9..1815ed623 100644 --- a/src/ReaderBase.h +++ b/src/ReaderBase.h @@ -97,6 +97,7 @@ namespace openshot void ParentClip(openshot::ClipBase* new_clip); /// Set an optional maximum decoded frame size. Use 0,0 to disable the limit. + /// Reduced FFmpeg previews align both dimensions to four pixels (minimum 4x4). void SetMaxDecodeSize(int width, int height); /// Return the current maximum decoded frame width (0 when unlimited). diff --git a/src/ScreenCaptureReader.cpp b/src/ScreenCaptureReader.cpp index 2ef556b5d..8c6ee32a0 100644 --- a/src/ScreenCaptureReader.cpp +++ b/src/ScreenCaptureReader.cpp @@ -11,6 +11,7 @@ // SPDX-License-Identifier: LGPL-3.0-or-later #include "ScreenCaptureReader.h" +#include "FFmpegColorRange.h" #include "CaptureAudioBuffer.h" #include @@ -1118,7 +1119,9 @@ std::shared_ptr ScreenCaptureReader::DecodeNextFrame(int64_t number) const int width = source_frame->width > 0 ? source_frame->width : info.width; const int height = source_frame->height > 0 ? source_frame->height : info.height; - const PixelFormat src_fmt = static_cast(source_frame->format); + bool src_full_range = source_frame->color_range == AVCOL_RANGE_JPEG; + const PixelFormat src_fmt = NormalizeDeprecatedPixFmt( + static_cast(source_frame->format), src_full_range); sws_context = sws_getCachedContext( sws_context, @@ -1136,6 +1139,10 @@ std::shared_ptr ScreenCaptureReader::DecodeNextFrame(int64_t number) throw InvalidFile("Unable to create capture pixel conversion context.", InputName()); } + const int* coefficients = sws_getCoefficients(SWS_CS_DEFAULT); + sws_setColorspaceDetails(sws_context, coefficients, src_full_range ? 1 : 0, + coefficients, 1, 0, 1 << 16, 1 << 16); + const int bytes_per_pixel = 4; const size_t buffer_size = static_cast(width) * height * bytes_per_pixel; unsigned char* buffer = static_cast(aligned_malloc(buffer_size)); diff --git a/src/Timeline.cpp b/src/Timeline.cpp index 544c910ad..b06bc7df9 100644 --- a/src/Timeline.cpp +++ b/src/Timeline.cpp @@ -11,6 +11,7 @@ // SPDX-License-Identifier: LGPL-3.0-or-later #include "Timeline.h" +#include "PreviewSize.h" #include "CacheBase.h" #include "CacheDisk.h" @@ -21,6 +22,7 @@ #include "effects/Mask.h" #include +#include #include #include #include @@ -1083,8 +1085,9 @@ std::shared_ptr Timeline::GetFrame(int64_t requested_frame) if (!ci.intersects) continue; const int layer = ci.clip->Layer(); auto it = top_start_for_layer.find(layer); - if (it == top_start_for_layer.end() || ci.start_pos > it->second) { - top_start_for_layer[layer] = ci.start_pos; // strictly greater to match prior logic + // The last composited clip wins ties at the same start frame. + if (it == top_start_for_layer.end() || ci.start_pos >= it->second) { + top_start_for_layer[layer] = ci.start_pos; top_clip_for_layer[layer] = ci.clip; } } @@ -1400,24 +1403,25 @@ void Timeline::ApplyJsonDiff(std::string value) { // Parse JSON string into JSON objects try { - const Json::Value root = openshot::stringToJson(value); + Json::Value root = openshot::stringToJson(value); const uint64_t initial_cache_epoch = CacheEpoch(); - // Process the JSON change array, loop through each item - for (const Json::Value change : root) { + // Each change is owned here and consumed once. Move it through the + // existing by-value API instead of copying entire animation curves. + for (auto& change : root) { std::string change_key = change["key"][(uint)0].asString(); // Process each type of change if (change_key == "clips") // Apply to CLIPS - apply_json_to_clips(change); + apply_json_to_clips(std::move(change)); else if (change_key == "effects") // Apply to EFFECTS - apply_json_to_effects(change); + apply_json_to_effects(std::move(change)); else // Apply to TIMELINE - apply_json_to_timeline(change); + apply_json_to_timeline(std::move(change)); } @@ -1499,7 +1503,7 @@ void Timeline::apply_json_to_clips(Json::Value change) { { if (e->Id() == effect_id) { // Apply the change to the effect directly - apply_json_to_effects(change, e); + apply_json_to_effects(std::move(change), e); // Effect-only diffs must clear the owning clip cache. if (existing_clip->GetCache()) { @@ -1532,7 +1536,7 @@ void Timeline::apply_json_to_clips(Json::Value change) { clip->ParentTimeline(this); // Set properties of clip from JSON - clip->SetJsonValue(change["value"]); + clip->SetJsonValue(std::move(change["value"])); // Add clip to timeline AddClip(clip); @@ -1546,7 +1550,7 @@ void Timeline::apply_json_to_clips(Json::Value change) { int64_t old_ending_frame = ((existing_clip->Position() + existing_clip->Duration()) * info.fps.ToDouble()) + 1; // Update clip properties from JSON - existing_clip->SetJsonValue(change["value"]); + existing_clip->SetJsonValue(std::move(change["value"])); // Calculate new start and end frames after the update int64_t new_starting_frame = (existing_clip->Position() * info.fps.ToDouble()) + 1; @@ -1614,7 +1618,7 @@ void Timeline::apply_json_to_effects(Json::Value change) { // Now that we found the effect, apply the change to it if (existing_effect || change_type == "insert") { // Apply change to effect - apply_json_to_effects(change, existing_effect); + apply_json_to_effects(std::move(change), existing_effect); } } @@ -1647,7 +1651,7 @@ void Timeline::apply_json_to_effects(Json::Value change, EffectBase* existing_ef allocated_effects.insert(e); // Load Json into Effect - e->SetJsonValue(change["value"]); + e->SetJsonValue(std::move(change["value"])); // Add Effect to Timeline AddEffect(e); @@ -1664,7 +1668,7 @@ void Timeline::apply_json_to_effects(Json::Value change, EffectBase* existing_ef final_cache->Remove(old_starting_frame - 8, old_ending_frame + 8); // Update effect properties from JSON - existing_effect->SetJsonValue(change["value"]); + existing_effect->SetJsonValue(std::move(change["value"])); } } else if (change_type == "delete") { @@ -1705,16 +1709,16 @@ void Timeline::apply_json_to_timeline(Json::Value change) { // Check for valid property if (root_key == "color") // Set color - color.SetJsonValue(change["value"]); + color.SetJsonValue(std::move(change["value"])); else if (root_key == "viewport_scale") // Set viewport scale - viewport_scale.SetJsonValue(change["value"]); + viewport_scale.SetJsonValue(std::move(change["value"])); else if (root_key == "viewport_x") // Set viewport x offset - viewport_x.SetJsonValue(change["value"]); + viewport_x.SetJsonValue(std::move(change["value"])); else if (root_key == "viewport_y") // Set viewport y offset - viewport_y.SetJsonValue(change["value"]); + viewport_y.SetJsonValue(std::move(change["value"])); else if (root_key == "duration") { // Update duration of timeline info.duration = change["value"].asDouble(); @@ -1855,15 +1859,22 @@ void Timeline::ClearAllCache(bool deep) { BumpCacheEpoch(); } -// Set Max Image Size (used for performance optimization). Convenience function for setting -// Settings::Instance()->MAX_WIDTH and Settings::Instance()->MAX_HEIGHT. +// Set the preview size without changing the project's native dimensions. void Timeline::SetMaxSize(int width, int height) { + // Ignore transient invalid widget sizes (for example while hidden). + if (width <= 0 || height <= 0 || info.width <= 0 || info.height <= 0) + return; // Maintain aspect ratio regardless of what size is passed in QSize display_ratio_size = QSize(info.width, info.height); QSize proposed_size = QSize(std::min(width, info.width), std::min(height, info.height)); // Scale QSize up to proposed size display_ratio_size.scale(proposed_size, Qt::KeepAspectRatio); + // Preserve exact full-resolution output, including non-aligned profiles and + // tiny test images. Only reduced previews use the aligned sampling grid. + if (display_ratio_size != QSize(info.width, info.height) && + info.width >= 4 && info.height >= 4) + display_ratio_size = AlignPreviewSize(display_ratio_size); // Update preview settings preview_width = display_ratio_size.width(); @@ -1954,15 +1965,9 @@ std::pair Timeline::ResolveTransitionAudioGains(Clip* source_clip, // Keep the current top/non-top clip routing intact when two clips overlap. if (audible_clips.size() == 2) { - auto top_it = std::max_element( - audible_clips.begin(), - audible_clips.end(), - [](const AudibleClipInfo& lhs, const AudibleClipInfo& rhs) { - if (lhs.start_pos != rhs.start_pos) - return lhs.start_pos < rhs.start_pos; - return std::less()(lhs.clip, rhs.clip); - }); - if ((is_top_clip && source_clip != top_it->clip) || (!is_top_clip && source_clip == top_it->clip)) + // Collected in compositing order, including equal-position ties. + Clip* top_clip = audible_clips.back().clip; + if ((is_top_clip && source_clip != top_clip) || (!is_top_clip && source_clip == top_clip)) return {1.0f, 1.0f}; } diff --git a/src/Timeline.h b/src/Timeline.h index d797f0744..e70304125 100644 --- a/src/Timeline.h +++ b/src/Timeline.h @@ -53,8 +53,9 @@ namespace openshot { return lhs->Layer() < rhs->Layer(); if (lhs->Position() != rhs->Position()) return lhs->Position() < rhs->Position(); - // Stable tie-breaker on address to avoid equivalence when layer/position match - return std::less()(lhs, rhs); + // list::sort is stable: preserve compositing order for tied clips, + // including their project order when loading a saved timeline. + return false; } }; @@ -351,8 +352,8 @@ namespace openshot { Json::Value JsonValue() const override; ///< Generate Json::Value for this object void SetJsonValue(const Json::Value root) override; ///< Load Json::Value into this object - /// Set Max Image Size (used for performance optimization). Convenience function for setting - /// Settings::Instance()->MAX_WIDTH and Settings::Instance()->MAX_HEIGHT. + /// Fit the preview within these bounds, aligning reduced dimensions to four + /// pixels (minimum 4x4). Ignore nonpositive bounds; preserve native output size. void SetMaxSize(int width, int height); /// @brief Apply a special formatted JSON object, which represents a change to the timeline (add, update, delete) diff --git a/tests/Benchmark.cpp b/tests/Benchmark.cpp index f15593a10..9fdd9f05e 100644 --- a/tests/Benchmark.cpp +++ b/tests/Benchmark.cpp @@ -136,7 +136,7 @@ using namespace openshot; using namespace std; using Clock = chrono::steady_clock; -using TrialResult = pair; // (frames, elapsed_seconds) +using TrialResult = pair; // (frames or JSON updates, elapsed_seconds) using TrialFunc = function; using Trial = pair; @@ -198,7 +198,7 @@ static void print_results(const vector& records) { }; cout << "| " << left << setw(static_cast(max_name)) << "Trial" - << " | " << right << setw(8) << "FPS" + << " | " << right << setw(8) << "Ops/s" << " | Chart |\n"; cout << "|:" << string(max_name, '-') << "-|" << string(8, '-') << ":|:" @@ -219,6 +219,58 @@ static TrialResult timed_read(ReaderBase& r) { return {BENCH_FRAMES, chrono::duration(Clock::now() - t0).count()}; } +// Model dragging a clip's X/Y transform: each edit sends the complete, growing +// curves, as openshot-qt does. Prepare strings outside the measured interval so +// this measures ApplyJsonDiff (including parsing), not JSON generation or render. +static TrialResult timed_transform_json(const string& image, int existing_keys) { + constexpr int edits = 100; + Clip clip(image); + clip.Id("json-benchmark-clip"); + clip.Layer(5); + clip.End((existing_keys + edits + 24) / 24.0); + Timeline timeline(1280, 720, Fraction(24, 1), 44100, 2, LAYOUT_STEREO); + timeline.AddClip(&clip); + timeline.Open(); + + Json::Value diff(Json::arrayValue); + Json::Value change(Json::objectValue); + change["type"] = "update"; + change["key"].append("clips"); + Json::Value id(Json::objectValue); + id["id"] = clip.Id(); + change["key"].append(id); + diff.append(change); + auto& value = diff[0]["value"]; + value["location_x"]["Points"] = Json::Value(Json::arrayValue); + value["location_y"]["Points"] = Json::Value(Json::arrayValue); + Json::StreamWriterBuilder writer; + writer["indentation"] = ""; + vector payloads; + payloads.reserve(edits); + for (int frame = 1; frame <= existing_keys + edits; ++frame) { + // Include Bezier handles, matching real project keyframes. + value["location_x"]["Points"].append(Point(frame, sin(frame * 0.05) * 0.4, BEZIER).JsonValue()); + value["location_y"]["Points"].append(Point(frame, cos(frame * 0.05) * 0.4, BEZIER).JsonValue()); + if (frame == existing_keys) + timeline.ApplyJsonDiff(Json::writeString(writer, diff)); + if (frame > existing_keys) + payloads.push_back(Json::writeString(writer, diff)); + } + auto start = Clock::now(); + for (const auto& payload : payloads) + timeline.ApplyJsonDiff(payload); + double elapsed = chrono::duration(Clock::now() - start).count(); + // A broken/no-op diff must not masquerade as an optimization. + const int final_frame = existing_keys + edits; + if (clip.location_x.GetCount() != final_frame || clip.location_y.GetCount() != final_frame || + abs(clip.location_x.GetValue(final_frame) - sin(final_frame * 0.05) * 0.4) > 1e-6 || + abs(clip.location_y.GetValue(final_frame) - cos(final_frame * 0.05) * 0.4) > 1e-6) + throw runtime_error("Transform JSON benchmark did not apply the expected keyframes"); + timeline.Close(); + timeline.RemoveClip(&clip); + return {edits, elapsed}; +} + #if defined(OPENSHOT_HAS_AUDIOVISUALIZATION) || defined(OPENSHOT_HAS_BEATSYNC) static std::shared_ptr make_audio_visualization_frame(int64_t frame_number) { const int width = 1280; @@ -317,6 +369,12 @@ int main(int argc, char* argv[]) { vector trials; trials.reserve(40); + for (int keys : {0, 1000}) { + trials.emplace_back("Timeline JSON transforms (" + to_string(keys) + " existing keys)", + [&, keys]() { return timed_transform_json(overlay, keys); }); + } + + trials.emplace_back("FFmpegReader", [&]() -> TrialResult { FFmpegReader r(video); r.Open(); diff --git a/tests/Clip.cpp b/tests/Clip.cpp index cf4c10391..413cd2fbb 100644 --- a/tests/Clip.cpp +++ b/tests/Clip.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -1973,3 +1974,93 @@ TEST_CASE("Reverse time curve (sample-exact, no resampling)", "[libopenshot][cli r.Close(); cache.Clear(); } + +TEST_CASE("Location coordinate convention survives JSON and partial updates", "[libopenshot][clip][json][location-compat]") +{ + Clip clip; + CHECK(clip.location_coordinate_system == "auto"); + for (const std::string coordinates : {"canvas", "geometry"}) { + clip.SetJson("{\"location_coordinate_system\":\"" + coordinates + "\"}"); + clip.location_x.AddPoint(51, -0.25, BEZIER); + Clip restored; + restored.SetJson(clip.Json()); + CHECK(restored.location_coordinate_system == coordinates); + CHECK(restored.location_x.Json() == clip.location_x.Json()); + restored.SetJson("{\"scale\":0}"); + CHECK(restored.location_coordinate_system == coordinates); + } + clip.SetJson("{\"location_coordinate_system\":\"unknown\"}"); + CHECK(clip.location_coordinate_system == "auto"); + clip.SetJson("{\"location_coordinate_system\":123}"); + CHECK(clip.location_coordinate_system == "auto"); +} + +TEST_CASE("Imported locations match historical rendered image bounds", "[libopenshot][clip][transform][location-compat]") +{ + // Bounds measured from unmodified v0.7.0 (canvas) and v1.0.0 (geometry) + // engines, using QtImageReader. In particular SCALE_NONE must use the + // decoded image dimensions, not dimensions guessed from reader metadata. + struct HistoricalCase { + ScaleType scale; + bool animated; + const char* coordinates; + int bounds[3][4]; // left, top, right, bottom at frames 1, 51, 101 + }; + const HistoricalCase cases[] = { + {SCALE_FIT, false, "canvas", {{160,63,319,152},{160,63,319,152},{160,63,319,152}}}, + {SCALE_FIT, true, "canvas", {{160,63,319,152},{120,41,319,175},{80,18,319,179}}}, + {SCALE_STRETCH, false, "canvas", {{160,63,319,152},{160,63,319,152},{160,63,319,152}}}, + {SCALE_STRETCH, true, "canvas", {{160,63,319,152},{120,41,319,175},{80,18,319,179}}}, + {SCALE_CROP, false, "canvas", {{160,63,319,152},{160,63,319,152},{160,63,319,152}}}, + {SCALE_CROP, true, "canvas", {{160,63,319,152},{120,41,319,175},{80,18,319,179}}}, + {SCALE_NONE, false, "canvas", {{220,97,259,118},{220,97,259,118},{220,97,259,118}}}, + {SCALE_NONE, true, "canvas", {{200,86,279,130},{180,74,299,141},{160,63,319,152}}}, + {SCALE_FIT, false, "geometry", {{140,59,299,148},{140,59,299,148},{140,59,299,148}}}, + {SCALE_FIT, true, "geometry", {{140,59,299,148},{110,38,319,172},{80,18,319,179}}}, + {SCALE_STRETCH, false, "geometry", {{140,59,299,148},{140,59,299,148},{140,59,299,148}}}, + {SCALE_STRETCH, true, "geometry", {{140,59,299,148},{110,38,319,172},{80,18,319,179}}}, + {SCALE_CROP, false, "geometry", {{140,59,299,148},{140,59,299,148},{140,59,299,148}}}, + {SCALE_CROP, true, "geometry", {{140,59,299,148},{110,38,319,172},{80,18,319,179}}}, + {SCALE_NONE, false, "geometry", {{185,89,224,110},{185,89,224,110},{185,89,224,110}}}, + {SCALE_NONE, true, "geometry", {{170,79,249,123},{155,69,274,135},{140,59,299,148}}}, + }; + QTemporaryDir directory; + REQUIRE(directory.isValid()); + const QString path = directory.filePath("source.png"); + QImage source(160, 90, QImage::Format_RGBA8888); + source.fill(Qt::red); + REQUIRE(source.save(path)); + for (const auto& test : cases) { + INFO("scale=" << test.scale << " animated=" << test.animated << " units=" << test.coordinates); + QtImageReader reader(path.toStdString()); + reader.Open(); + Clip clip(&reader); + clip.End(4.0); + clip.Layer(1); + clip.scale = test.scale; + clip.location_coordinate_system = test.coordinates; + clip.location_x = Keyframe(0.25); + clip.location_y = Keyframe(0.1); + clip.scale_x = Keyframe(0.5); + clip.scale_y = Keyframe(0.5); + if (test.animated) { + clip.scale_x.AddPoint(101, 1.0, LINEAR); + clip.scale_y.AddPoint(101, 1.0, LINEAR); + } + Timeline timeline(320, 180, Fraction(30, 1), 44100, 2, LAYOUT_STEREO); + timeline.AddClip(&clip); + timeline.SetMaxSize(320, 180); + timeline.Open(); + for (int index = 0; index < 3; ++index) { + const int frame = 1 + 50 * index; + INFO("frame=" << frame); + const QRect bounds = red_bounds(*timeline.GetFrame(frame)->GetImage()); + REQUIRE_FALSE(bounds.isNull()); + CHECK(bounds.left() == Approx(test.bounds[index][0]).margin(1)); + CHECK(bounds.top() == Approx(test.bounds[index][1]).margin(1)); + CHECK(bounds.right() == Approx(test.bounds[index][2]).margin(1)); + CHECK(bounds.bottom() == Approx(test.bounds[index][3]).margin(1)); + } + timeline.Close(); + } +} diff --git a/tests/FFmpegReader.cpp b/tests/FFmpegReader.cpp index 4efd55236..16c5e12a7 100644 --- a/tests/FFmpegReader.cpp +++ b/tests/FFmpegReader.cpp @@ -314,6 +314,68 @@ TEST_CASE( "Max_Decode_Size_FFmpegReader", "[libopenshot][ffmpegreader]" ) r.Close(); } +TEST_CASE("Reduced decoded frames have aligned rows", "[libopenshot][ffmpegreader][preview-size]") +{ + for (const QSize bounds : {QSize(638, 359), QSize(640, 352), QSize(1280, 359), + QSize(638, 720), QSize(1, 1)}) { + CAPTURE(bounds.width(), bounds.height()); + FFmpegReader reader(std::string(TEST_MEDIA_PATH) + "sintel_trailer-720p.mp4"); + reader.SetMaxDecodeSize(bounds.width(), bounds.height()); + reader.Open(); + auto frame = reader.GetFrame(1); + REQUIRE(frame != nullptr); + CHECK(frame->GetWidth() % 4 == 0); + CHECK(frame->GetHeight() % 4 == 0); + CHECK(frame->GetWidth() >= 4); + CHECK(frame->GetHeight() >= 4); + CHECK(frame->GetWidth() <= std::max(4, bounds.width())); + CHECK(frame->GetHeight() <= std::max(4, bounds.height())); + CHECK(frame->GetImage()->bytesPerLine() % 16 == 0); + CHECK(reinterpret_cast(frame->GetPixels()) % 16 == 0); + reader.Close(); + } +} + +TEST_CASE("Clip fitting cannot undo preview alignment", "[libopenshot][ffmpegreader][preview-size]") +{ + FFmpegReader reader(std::string(TEST_MEDIA_PATH) + "sintel_trailer-720p.mp4"); + reader.Open(); + Clip clip(&reader); + // A 16:9 source in a portrait preview is fitted again by the reader. + Timeline timeline(1080, 1920, Fraction(30, 1), 44100, 2, LAYOUT_STEREO); + timeline.SetMaxSize(333, 591); + timeline.AddClip(&clip); + clip.scale_x = Keyframe(1.13); + clip.scale_y = Keyframe(1.07); + clip.Open(); + auto frame = reader.GetFrame(1); + CHECK(frame->GetWidth() < reader.info.width); + CHECK(frame->GetWidth() % 4 == 0); + CHECK(frame->GetHeight() % 4 == 0); + CHECK(frame->GetImage()->bytesPerLine() % 16 == 0); + clip.Close(); + timeline.Close(); +} + +TEST_CASE("Quarter-turn previews retain aligned rows", "[libopenshot][ffmpegreader][preview-size]") +{ + FFmpegReader reader(std::string(TEST_MEDIA_PATH) + "sintel_trailer-720p.mp4"); + reader.SetMaxDecodeSize(359, 638); + reader.ApplyOrientationMetadata(true); + reader.Open(); + // Model the display orientation discovered from container metadata. + reader.source_rotation = 90; + std::swap(reader.info.width, reader.info.height); + auto frame = reader.GetFrame(1); + CHECK(frame->GetWidth() <= 359); + CHECK(frame->GetHeight() <= 638); + CHECK(frame->GetHeight() > frame->GetWidth()); + CHECK(frame->GetWidth() % 4 == 0); + CHECK(frame->GetHeight() % 4 == 0); + CHECK(frame->GetImage()->bytesPerLine() % 16 == 0); + reader.Close(); +} + TEST_CASE( "Seek", "[libopenshot][ffmpegreader]" ) { // Create a reader diff --git a/tests/FFmpegWriter.cpp b/tests/FFmpegWriter.cpp index 87c318d06..7109b7b25 100644 --- a/tests/FFmpegWriter.cpp +++ b/tests/FFmpegWriter.cpp @@ -47,6 +47,7 @@ AVStream* first_video_stream(AVFormatContext* format_context) TEST_CASE("Raw video export preserves all color planes and frame ownership", "[libopenshot][ffmpegwriter][rawvideo]") { + const int width = GENERATE(64, 1080); QTemporaryDir directory; REQUIRE(directory.isValid()); // NUT uses a different stream time base from the codec's frame rate. @@ -55,11 +56,11 @@ TEST_CASE("Raw video export preserves all color planes and frame ownership", // Multiple frames and repeated exports exercise packet/frame cleanup. for (int pass = 0; pass < 2; ++pass) { FFmpegWriter writer(path); - writer.SetVideoOptions(true, "rawvideo", Fraction(30, 1), 64, 64, + writer.SetVideoOptions(true, "rawvideo", Fraction(30, 1), width, 64, Fraction(1, 1), false, false, 1000000); writer.Open(); for (int number = 1; number <= 3; ++number) { - auto frame = std::make_shared(number, 64, 64, number == 2 ? "blue" : "red"); + auto frame = std::make_shared(number, width, 64, number == 2 ? "blue" : "red"); writer.WriteFrame(frame); } writer.Close(); @@ -75,7 +76,7 @@ TEST_CASE("Raw video export preserves all color planes and frame ownership", int packets = 0; while (av_read_frame(input, &packet) >= 0) { if (packet.stream_index == stream->index) { - CHECK(packet.size == 64 * 64 * 3 / 2); // Complete YUV420P image + CHECK(packet.size == width * 64 * 3 / 2); // Complete YUV420P image CHECK(packet.pts * av_q2d(stream->time_base) == Approx(packets / 30.0).margin(0.00001)); ++packets; } @@ -92,9 +93,9 @@ TEST_CASE("Raw video export preserves all color planes and frame ownership", CHECK(reader.info.video_length == 3); for (int number = 1; number <= 3; ++number) { auto frame = reader.GetFrame(number); - REQUIRE(frame->GetWidth() == 64); + REQUIRE(frame->GetWidth() == width); REQUIRE(frame->GetHeight() == 64); - const QColor color = frame->GetImage()->pixelColor(32, 32); + const QColor color = frame->GetImage()->pixelColor(width - 1, 63); CHECK(color.green() < 10); CHECK(color.red() == Approx(number == 2 ? 0 : 255).margin(10)); CHECK(color.blue() == Approx(number == 2 ? 255 : 0).margin(10)); @@ -103,6 +104,31 @@ TEST_CASE("Raw video export preserves all color planes and frame ownership", } } +TEST_CASE("GIF export copies padded RGB8 rows without a synthetic palette", + "[libopenshot][ffmpegwriter][padded-gif]") +{ + QTemporaryDir directory; + REQUIRE(directory.isValid()); + const std::string path = directory.filePath("padded.gif").toStdString(); + FFmpegWriter writer(path); + writer.SetVideoOptions(true, "gif", Fraction(25, 1), 1080, 32, + Fraction(1, 1), false, false, 1000000); + writer.Open(); + for (int number = 1; number <= 3; ++number) + writer.WriteFrame(std::make_shared(number, 1080, 32, "red")); + writer.Close(); + FFmpegReader reader(path); + reader.Open(); + auto frame = reader.GetFrame(1); + REQUIRE(frame->GetWidth() == 1080); + REQUIRE(frame->GetHeight() == 32); + const QColor color = frame->GetImage()->pixelColor(1079, 31); + CHECK(color.red() > 230); + CHECK(color.green() < 20); + CHECK(color.blue() < 20); + reader.Close(); +} + TEST_CASE( "Webm", "[libopenshot][ffmpegwriter]" ) { // Reader @@ -437,3 +463,57 @@ TEST_CASE( "SizeOrdering_vp9_CRF", "[libopenshot][ffmpegwriter][filesize]" ) reader.Close(); } + +TEST_CASE("AAC export preserves delayed packet timestamps through flush", + "[libopenshot][ffmpegwriter][audio-timestamps]") +{ + QTemporaryDir directory; + REQUIRE(directory.isValid()); + const std::string path = directory.filePath("aac.mp4").toStdString(); + const Fraction fps(30000, 1001); + const int source_rate = GENERATE(48000, 44100); + FFmpegWriter writer(path); + writer.SetVideoOptions(true, "libx264", fps, 16, 16, Fraction(1, 1), false, false, 100000); + writer.SetAudioOptions(true, "aac", 48000, 2, LAYOUT_STEREO, 160000); + writer.Open(); + int total_samples = 0; + for (int number = 1; number <= 230; ++number) { + const int samples = Frame::GetSamplesPerFrame(number, fps, source_rate, 2); + auto frame = std::make_shared(number, 16, 16, "red", samples, 2); + frame->SampleRate(source_rate); + frame->ChannelsLayout(LAYOUT_STEREO); + frame->AddAudioSilence(samples); + writer.WriteFrame(frame); + total_samples += samples; + } + writer.Close(); + AVFormatContext* input = nullptr; + REQUIRE(avformat_open_input(&input, path.c_str(), nullptr, nullptr) == 0); + std::unique_ptr guard( + input, [](AVFormatContext* context) { avformat_close_input(&context); }); + REQUIRE(avformat_find_stream_info(input, nullptr) >= 0); + const int index = av_find_best_stream(input, AVMEDIA_TYPE_AUDIO, -1, -1, nullptr, 0); + REQUIRE(index >= 0); + AVStream* stream = input->streams[index]; + AVPacket packet = {}; + int count = 0; + int64_t previous = AV_NOPTS_VALUE; + while (av_read_frame(input, &packet) >= 0) { + if (packet.stream_index == index) { + REQUIRE(packet.dts != AV_NOPTS_VALUE); + CHECK(packet.pts == packet.dts); + const int64_t sample_dts = av_rescale_q(packet.dts, stream->time_base, AVRational{1, 48000}); + if (count) CHECK(sample_dts - previous == 1024); + else CHECK(sample_dts == -1024); // AAC encoder priming + previous = sample_dts; + ++count; + } + av_packet_unref(&packet); + } + const int64_t expected_samples = av_rescale(total_samples, 48000, source_rate); + CHECK(count == (expected_samples + 1023) / 1024 + 1); + // MP4 edit-list durations round to milliseconds (48 samples). Rate + // conversion can also retain a short filter tail in the resampler. + CHECK(av_rescale_q(stream->duration, stream->time_base, AVRational{1, 48000}) + == Approx(expected_samples).margin(source_rate == 48000 ? 48 : 96)); +} diff --git a/tests/KeyFrame.cpp b/tests/KeyFrame.cpp index 00c1050c0..b188b04a8 100644 --- a/tests/KeyFrame.cpp +++ b/tests/KeyFrame.cpp @@ -832,3 +832,46 @@ TEST_CASE( "Tracker stroke width compensates for preview raster scaling", "[libo CHECK(blue_run_at_center() >= 3); } #endif + +TEST_CASE("JSON keyframe loading preserves AddPoint semantics", "[libopenshot][keyframe][json]") +{ + std::vector frames; + SECTION("sorted") { frames = {1, 4, 8, 20}; } + SECTION("unordered and duplicate frames") { frames = {20, 4, 8, 4, 1, 20}; } + SECTION("fractional frames") { frames = {1, 1.25, 1.75, 1.25, 2}; } + SECTION("large sorted curve") { + for (int i = 1; i <= 5000; ++i) frames.push_back(i); + } + Json::Value data; + data["Points"] = Json::Value(Json::arrayValue); + Keyframe expected; + for (size_t i = 0; i < frames.size(); ++i) { + Point point(frames[i], i * 0.125, static_cast(i % 3)); + // Only Bezier points serialize custom handles. + if (point.interpolation == BEZIER) { + point.handle_left = Coordinate(0.2, 0.8); + point.handle_right = Coordinate(0.7, 0.1); + } + data["Points"].append(point.JsonValue()); + expected.AddPoint(point); + } + const Json::Value original = data; + Keyframe actual(99); + actual.SetJsonValue(data); + CHECK(data == original); + CHECK(actual.JsonValue() == expected.JsonValue()); + for (int frame = 1; frame <= 20; ++frame) + CHECK(actual.GetValue(frame) == Approx(expected.GetValue(frame))); + + // Replacing a large animation must discard old points, even when capacity + // is retained internally for the next drag sample. + actual.SetJsonValue(Json::Value(0.75)); + CHECK(actual.GetCount() == 1); + CHECK(actual.GetValue(1) == Approx(0.75)); + Json::Value empty; + empty["Points"] = Json::Value(Json::arrayValue); + actual.SetJsonValue(empty); + CHECK(actual.GetCount() == 0); + actual.SetJsonValue(data); + CHECK(actual.JsonValue() == expected.JsonValue()); +} diff --git a/tests/QtPlayer.cpp b/tests/QtPlayer.cpp index 1bc8fd5f6..87d87575d 100644 --- a/tests/QtPlayer.cpp +++ b/tests/QtPlayer.cpp @@ -13,7 +13,11 @@ #include "openshot_catch.h" #include +#include +#include +#include +#include "DummyReader.h" #include "QtPlayer.h" #include "Qt/VideoRenderer.h" @@ -36,6 +40,39 @@ class TestRenderer : public openshot::RendererBase (void) image; } }; + +class SlowRenderer : public openshot::RendererBase +{ +public: + std::atomic renders{0}; + void OverrideWidget(uintptr_t) override {} + +protected: + void render(std::shared_ptr image) override + { + if (image) + ++renders; + // Longer than the player's render timeout: the next frame may be + // submitted while this renderer still owns the previous one. + std::this_thread::sleep_for(std::chrono::milliseconds(120)); + } +}; + +class SlowReader : public openshot::DummyReader +{ +public: + std::atomic decoding{false}; + SlowReader() : DummyReader(openshot::Fraction(30, 1), 16, 16, 44100, 2, 30) {} + + std::shared_ptr GetFrame(int64_t frame_number) override + { + decoding = true; + std::this_thread::sleep_for(std::chrono::milliseconds(250)); + auto frame = DummyReader::GetFrame(frame_number); + decoding = false; + return frame; + } +}; } // namespace TEST_CASE("QtPlayer_GetRendererQObject_ReturnsVideoRendererAddress", "[libopenshot][qtplayer]") @@ -58,3 +95,43 @@ TEST_CASE("QtPlayer_SetQWidget_Overload_ForwardsPointer", "[libopenshot][qtplaye CHECK(renderer.last_widget == reinterpret_cast(widget)); } + +TEST_CASE("QtPlayer keeps frames alive during slow rendering and seeks", "[libopenshot][qtplayer][threading]") +{ + SlowRenderer renderer; + openshot::DummyReader reader(openshot::Fraction(30, 1), 16, 16, 44100, 2, 30); + reader.Open(); + openshot::QtPlayer player(&renderer); + player.Reader(&reader); + player.Play(); + + std::thread seeker([&player] { + for (int i = 0; i < 80; ++i) { + player.Seek(1 + (i % 60), false); + std::this_thread::sleep_for(std::chrono::milliseconds(2)); + } + }); + seeker.join(); + std::this_thread::sleep_for(std::chrono::milliseconds(350)); + player.Stop(); + CHECK(renderer.renders.load() > 0); +} + +TEST_CASE("QtPlayer seek does not wait for a slow frame decode", "[libopenshot][qtplayer][threading]") +{ + TestRenderer renderer; + SlowReader reader; + reader.Open(); + openshot::QtPlayer player(&renderer); + player.Reader(&reader); + player.Play(); + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(2); + while (!reader.decoding && std::chrono::steady_clock::now() < deadline) + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + REQUIRE(reader.decoding.load()); + const auto start = std::chrono::steady_clock::now(); + player.Seek(20, false); + const auto elapsed = std::chrono::steady_clock::now() - start; + CHECK(elapsed < std::chrono::milliseconds(100)); + player.Stop(); +} diff --git a/tests/ScreenCaptureReader.cpp b/tests/ScreenCaptureReader.cpp index 13455e14e..e8cd29f76 100644 --- a/tests/ScreenCaptureReader.cpp +++ b/tests/ScreenCaptureReader.cpp @@ -16,6 +16,7 @@ #include "CaptureAudioBuffer.h" #include "ScreenCaptureReader.h" #include "WaylandBufferUtilities.h" +#include "FFmpegColorRange.h" #include #include @@ -24,6 +25,42 @@ using namespace openshot; +TEST_CASE("JPEG capture conversion preserves full-range black and white", + "[libopenshot][screencapturereader][color-range]") +{ + for (auto format : {AV_PIX_FMT_YUVJ420P, AV_PIX_FMT_YUVJ422P, + AV_PIX_FMT_YUVJ444P, AV_PIX_FMT_YUVJ440P}) { + bool full_range = false; + const auto normalized = NormalizeDeprecatedPixFmt(format, full_range); + CHECK(full_range); + CHECK(normalized != format); + uint8_t* source[4] = {}; + int strides[4] = {}; + const int size = av_image_alloc(source, strides, 16, 16, normalized, 32); + REQUIRE(size > 0); + memset(source[0], 128, size); + uint8_t* dest[4] = {}; + int dest_strides[4] = {}; + REQUIRE(av_image_alloc(dest, dest_strides, 16, 16, AV_PIX_FMT_RGBA, 32) > 0); + SwsContext* context = sws_getContext(16, 16, normalized, 16, 16, + AV_PIX_FMT_RGBA, SWS_BILINEAR, nullptr, nullptr, nullptr); + REQUIRE(context != nullptr); + const int* coefficients = sws_getCoefficients(SWS_CS_DEFAULT); + REQUIRE(sws_setColorspaceDetails(context, coefficients, full_range, coefficients, + 1, 0, 1 << 16, 1 << 16) >= 0); + for (int level : {0, 16, 235, 255}) { + for (int row = 0; row < 16; ++row) + memset(source[0] + row * strides[0], level, 16); + REQUIRE(sws_scale(context, source, strides, 0, 16, dest, dest_strides) == 16); + for (int channel = 0; channel < 3; ++channel) + CHECK(int(dest[0][channel]) == Approx(level).margin(2)); + } + sws_freeContext(context); + av_freep(&source[0]); + av_freep(&dest[0]); + } +} + TEST_CASE("PulseAudio and WASAPI clocks align to the same recording samples", "[libopenshot][screencapturereader][audio][sync]") { diff --git a/tests/Timeline.cpp b/tests/Timeline.cpp index 9b87293ce..cc11be55d 100644 --- a/tests/Timeline.cpp +++ b/tests/Timeline.cpp @@ -10,6 +10,8 @@ // // SPDX-License-Identifier: LGPL-3.0-or-later +#include +#include #include #include #include @@ -37,6 +39,55 @@ using namespace openshot; +TEST_CASE("Preview sizes stay aligned after aspect fitting", "[libopenshot][timeline][preview-size]") +{ + for (const QSize native : {QSize(1920, 1080), QSize(1080, 1920), + QSize(1440, 1080), QSize(854, 480), QSize(427, 240)}) { + Timeline timeline(native.width(), native.height(), Fraction(30, 1), 44100, 2, LAYOUT_STEREO); + for (const QSize bounds : {QSize(640, 352), QSize(638, 359), QSize(333, 591), + QSize(799, 451), QSize(1, 1), QSize(0, 360), QSize(-1, 360)}) { + CAPTURE(native.width(), native.height(), bounds.width(), bounds.height()); + const QSize previous(timeline.preview_width, timeline.preview_height); + timeline.SetMaxSize(bounds.width(), bounds.height()); + if (bounds.width() <= 0 || bounds.height() <= 0) { + CHECK(QSize(timeline.preview_width, timeline.preview_height) == previous); + continue; + } + if (bounds.width() >= native.width() && bounds.height() >= native.height()) { + CHECK(QSize(timeline.preview_width, timeline.preview_height) == native); + continue; + } + CHECK(timeline.preview_width % 4 == 0); + CHECK(timeline.preview_height % 4 == 0); + CHECK(timeline.preview_width >= 4); + CHECK(timeline.preview_height >= 4); + CHECK(timeline.preview_width <= std::max(4, std::min(bounds.width(), native.width()))); + CHECK(timeline.preview_height <= std::max(4, std::min(bounds.height(), native.height()))); + QSize fitted = native.scaled(QSize(std::min(bounds.width(), native.width()), + std::min(bounds.height(), native.height())), Qt::KeepAspectRatio); + CHECK(std::abs(timeline.preview_width - fitted.width()) <= 4); + CHECK(std::abs(timeline.preview_height - fitted.height()) <= 4); + const QSize result(timeline.preview_width, timeline.preview_height); + timeline.SetMaxSize(bounds.width(), bounds.height()); + CHECK(QSize(timeline.preview_width, timeline.preview_height) == result); + } + // Restoring native size must not resize exports or saved full-size frames. + timeline.SetMaxSize(native.width(), native.height()); + CHECK(timeline.preview_width == native.width()); + CHECK(timeline.preview_height == native.height()); + CHECK(timeline.info.width == native.width()); + CHECK(timeline.info.height == native.height()); + } +} + +TEST_CASE("Native tiny timelines remain exact", "[libopenshot][timeline][preview-size]") +{ + Timeline timeline(2, 2, Fraction(30, 1), 44100, 2, LAYOUT_STEREO); + timeline.SetMaxSize(100, 100); + CHECK(timeline.preview_width == 2); + CHECK(timeline.preview_height == 2); +} + TEST_CASE("Deleting a JSON-owned clip invalidates its former range", "[libopenshot][timeline][sentry-delete]") { Timeline timeline(2, 2, Fraction(30, 1), 44100, 2, LAYOUT_STEREO); @@ -187,6 +238,99 @@ class TimelineSolidColorReader : public ReaderBase { void SetJsonValue(const Json::Value root) override { ReaderBase::SetJsonValue(root); } }; +TEST_CASE("Transition follows compositing order for clips at the same position", "[libopenshot][timeline][mask][same-position]") +{ + TimelineSolidColorReader red(32, 32, 30, 1, 1800, QColor(255, 0, 0)); + TimelineSolidColorReader blue(32, 32, 30, 1, 600, QColor(0, 0, 255)); + Clip first, second; + // Exercise both address orders: insertion order must determine visibility. + Clip* bottom = std::less()(&first, &second) ? &first : &second; + Clip* top = bottom == &first ? &second : &first; + if (GENERATE(false, true)) + std::swap(bottom, top); + bottom->Reader(&red); + top->Reader(&blue); + for (auto clip : {bottom, top}) { + clip->Layer(5); + clip->Position(197.0 / 30.0); + clip->End(10.0); + } + bottom->Start(9.2); + bottom->End(52.2); + // Different subframe positions can also resolve to the same start frame. + top->Position(top->Position() + GENERATE(0.0, 0.001)); + + Mask transition; + transition.MaskReader(new TimelineSolidColorReader(32, 32, 30, 1, 600, QColor(128, 128, 128))); + transition.Layer(5); + transition.Position(top->Position()); + transition.End(10.0); + transition.contrast = Keyframe(3.0); + const bool fade_in = GENERATE(true, false); + CAPTURE(fade_in); + transition.brightness = Keyframe(fade_in ? 1.0 : -1.0); + transition.brightness.AddPoint(301, fade_in ? -1.0 : 1.0, LINEAR); + + Timeline timeline(32, 32, Fraction(30, 1), 44100, 2, LAYOUT_STEREO); + timeline.AddClip(bottom); + timeline.AddClip(top); + timeline.AddEffect(&transition); + timeline.Open(); + const QColor begin = timeline.GetFrame(198)->GetImage()->pixelColor(16, 16); + const QColor middle = timeline.GetFrame(348)->GetImage()->pixelColor(16, 16); + const QColor end = timeline.GetFrame(497)->GetImage()->pixelColor(16, 16); + CHECK(begin == (fade_in ? QColor(255, 0, 0) : QColor(0, 0, 255))); + CHECK(middle.red() == Approx(128).margin(3)); + CHECK(middle.blue() == Approx(128).margin(3)); + CHECK(end == (fade_in ? QColor(0, 0, 255) : QColor(255, 0, 0))); + CHECK(timeline.GetFrame(498)->GetImage()->pixelColor(16, 16) == QColor(255, 0, 0)); + timeline.Close(); +} + +TEST_CASE("Timeline preserves tied clip order through sorting and JSON reload", "[libopenshot][timeline][same-position][json]") +{ + DummyReader reader; + Clip first(&reader), second(&reader), earlier(&reader), higher(&reader); + Clip* bottom = std::less()(&first, &second) ? &first : &second; + Clip* top = bottom == &first ? &second : &first; + if (GENERATE(false, true)) + std::swap(bottom, top); + bottom->Id("bottom"); + top->Id("top"); + earlier.Id("earlier"); + higher.Id("higher"); + for (auto clip : {bottom, top, &earlier, &higher}) { + clip->Layer(5); + clip->Position(1.0); + clip->End(3.0); + } + earlier.Position(0.0); + higher.Layer(6); + higher.Position(0.0); + Timeline timeline(32, 32, Fraction(30, 1), 44100, 2, LAYOUT_STEREO); + // Persist source readers, as project JSON does, rather than FrameMapper wrappers. + timeline.AutoMapClips(false); + // Add out of layer/position order to verify those still take precedence. + for (auto clip : {&higher, bottom, top, &earlier}) + timeline.AddClip(clip); + timeline.SortTimeline(); + timeline.SortTimeline(); + const std::vector expected{"earlier", "bottom", "top", "higher"}; + auto clip_ids = [](Timeline& value) { + const auto clips = value.Clips(); + std::vector ids; + std::transform(clips.begin(), clips.end(), std::back_inserter(ids), + [](const auto clip) { return clip->Id(); }); + return ids; + }; + CHECK(clip_ids(timeline) == expected); + Timeline restored(32, 32, Fraction(30, 1), 44100, 2, LAYOUT_STEREO); + restored.SetJson(timeline.Json()); + CHECK(clip_ids(restored) == expected); + restored.SortTimeline(); + CHECK(clip_ids(restored) == expected); +} + class TimelineConstantAudioReader : public ReaderBase { private: bool is_open = false; @@ -420,6 +564,53 @@ TEST_CASE("Timeline honors Mask fade_audio_hint with equal-power overlapping aud t.Close(); } +TEST_CASE("Transition audio follows compositing order for tied start frames", "[libopenshot][timeline][audio][transition][same-position]") +{ + TimelineConstantAudioReader bottom_reader(32, 32, 30, 1, 48000, 2, 90, 1.0f); + TimelineConstantAudioReader top_reader(32, 32, 30, 1, 48000, 2, 90, 1.0f); + Clip first, second; + Clip* bottom = std::less()(&first, &second) ? &first : &second; + Clip* top = bottom == &first ? &second : &first; + if (GENERATE(false, true)) + std::swap(bottom, top); + bottom->Reader(&bottom_reader); + top->Reader(&top_reader); + for (auto clip : {bottom, top}) { + clip->Layer(0); + clip->Position(1.0); + clip->End(2.0); + } + top->Position(top->Position() + GENERATE(0.0, 0.001)); + bottom->channel_filter = Keyframe(0.0); + top->channel_filter = Keyframe(1.0); + Mask transition; + transition.Layer(0); + transition.Position(1.0); + transition.End(1.0); + transition.fade_audio_hint = GENERATE(false, true); + Timeline timeline(32, 32, Fraction(30, 1), 48000, 2, LAYOUT_STEREO); + timeline.AddClip(bottom); + timeline.AddClip(top); + timeline.AddEffect(&transition); + timeline.Open(); + for (int number : {31, 45, 60}) { + auto frame = timeline.GetFrame(number); + REQUIRE(frame->GetAudioSamplesCount() > 0); + // Both clips begin at the left transition edge, so both fade in. + // Channel isolation ensures neither clip silently bypasses its gain. + const double previous = transition.fade_audio_hint + ? expected_equal_power_gain(number - 1, 31, 60, true) : 1.0; + const double current = transition.fade_audio_hint + ? expected_equal_power_gain(number, 31, 60, true) : 1.0; + for (int channel : {0, 1}) { + CHECK(frame->GetAudioSamples(channel)[0] == Approx(previous).margin(0.0002)); + CHECK(frame->GetAudioSamples(channel)[frame->GetAudioSamplesCount() - 1] + == Approx(current).margin(0.002)); + } + } + timeline.Close(); +} + TEST_CASE("Timeline uses transition edge proximity for single-clip fade audio", "[libopenshot][timeline][audio][transition][single]") { const Fraction fps(30, 1); const int sample_rate = 48000; @@ -2183,3 +2374,53 @@ TEST_CASE("GetMaxFrame ignores tiny float overshoot at clip end", "[libopenshot] REQUIRE(t.GetMaxTime() * t.info.fps.ToDouble() < 505.0001); CHECK(t.GetMaxFrame() == 505); } + +TEST_CASE("JSON transform edits preserve other properties and multiple actions", "[libopenshot][timeline][json]") +{ + DummyReader reader; + Clip clip(&reader); + clip.Id("transform-json"); + clip.End(300); + clip.Layer(5); + clip.alpha = Keyframe(0.75); + Timeline timeline(1280, 720, Fraction(24, 1), 44100, 2, LAYOUT_STEREO); + timeline.AddClip(&clip); + Json::Value change; + change["type"] = "update"; + change["key"].append("clips"); + Json::Value id; + id["id"] = clip.Id(); + change["key"].append(id); + Keyframe x, y; + for (int frame = 1; frame <= 1000; ++frame) { + x.AddPoint(frame, frame * 0.001, LINEAR); + y.AddPoint(frame, -frame * 0.001, BEZIER); + } + change["value"]["location_x"] = x.JsonValue(); + change["value"]["location_y"] = y.JsonValue(); + Json::Value changes(Json::arrayValue); + changes.append(change); + Json::Value alpha_change = change; + alpha_change["value"] = Json::Value(Json::objectValue); + alpha_change["value"]["alpha"] = Keyframe(0.5).JsonValue(); + changes.append(alpha_change); + const auto epoch = timeline.CacheEpoch(); + timeline.ApplyJsonDiff(changes.toStyledString()); + CHECK(clip.location_x.JsonValue() == x.JsonValue()); + CHECK(clip.location_y.JsonValue() == y.JsonValue()); + CHECK(clip.alpha.GetValue(1) == Approx(0.5)); + CHECK(clip.Layer() == 5); + CHECK(clip.End() == Approx(300)); + CHECK(timeline.CacheEpoch() > epoch); + // A subsequent edit replaces the curve and leaves the other axis intact. + x.AddPoint(500, 0.9, CONSTANT); + change["value"].removeMember("location_y"); + change["value"]["location_x"] = x.JsonValue(); + changes.clear(); + changes.append(change); + timeline.ApplyJsonDiff(changes.toStyledString()); + CHECK(clip.location_x.JsonValue() == x.JsonValue()); + CHECK(clip.location_y.JsonValue() == y.JsonValue()); + CHECK(clip.alpha.GetValue(1) == Approx(0.5)); + timeline.RemoveClip(&clip); +}