From 8f385c112a4705b919e01a40a70239ba6a286852 Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Wed, 6 Mar 2019 15:35:03 -0600 Subject: [PATCH] Improved Keyframe Performance (#197) * Refactor of Coorindate and Keyframe optimized for performance (much faster than previously). Also refactored FrameMapper to not use a Keyframe, and to only process frame mapping when needed... speeding up Json loading of project files. * Fixing FrameMapper linear calculation to match existing Keyframe calculation * Fixing Keyframe to pass original unit tests, and correctly calculate the Keyframe repeat fractions and deltas. * Small refactor of time mapped logic in Clip.cpp --- include/Clip.h | 2 +- include/Coordinate.h | 26 ----- src/Clip.cpp | 32 ++---- src/Coordinate.cpp | 15 +-- src/FrameMapper.cpp | 33 +++--- src/KeyFrame.cpp | 213 ++++++++++++++++++++++----------------- tests/KeyFrame_Tests.cpp | 16 +++ 7 files changed, 166 insertions(+), 171 deletions(-) diff --git a/include/Clip.h b/include/Clip.h index 26cdd211..3cd33b3a 100644 --- a/include/Clip.h +++ b/include/Clip.h @@ -127,7 +127,7 @@ namespace openshot { std::shared_ptr GetOrCreateFrame(int64_t number); /// Adjust the audio and image of a time mapped frame - std::shared_ptr get_time_mapped_frame(std::shared_ptr frame, int64_t frame_number); + void get_time_mapped_frame(std::shared_ptr frame, int64_t frame_number); /// Init default settings for a clip void init_settings(); diff --git a/include/Coordinate.h b/include/Coordinate.h index 0ca6b7b8..bf561084 100644 --- a/include/Coordinate.h +++ b/include/Coordinate.h @@ -52,11 +52,6 @@ namespace openshot { * \endcode */ class Coordinate { - private: - bool increasing; ///< Is the Y value increasing or decreasing? - Fraction repeated; ///< Fraction of repeated Y values (for example, 1/3 would be the first Y value of 3 repeated values) - double delta; ///< This difference in Y value (from the previous unique Y value) - public: double X; ///< The X value of the coordinate (usually representing the frame #) double Y; ///< The Y value of the coordinate (usually representing the value of the property being animated) @@ -69,27 +64,6 @@ namespace openshot { /// @param y The Y coordinate (usually representing the value of the property being animated) Coordinate(double x, double y); - /// @brief Set the repeating Fraction (used internally on the timeline, to track changes to coordinates) - /// @param is_repeated The fraction representing how many times this coordinate Y value repeats (only used on the timeline) - void Repeat(Fraction is_repeated) { repeated=is_repeated; } - - /// Get the repeating Fraction (used internally on the timeline, to track changes to coordinates) - Fraction Repeat() { return repeated; } - - /// @brief Set the increasing flag (used internally on the timeline, to track changes to coordinates) - /// @param is_increasing Indicates if this coordinate Y value is increasing (when compared to the previous coordinate) - void IsIncreasing(bool is_increasing) { increasing = is_increasing; } - - /// Get the increasing flag (used internally on the timeline, to track changes to coordinates) - bool IsIncreasing() { return increasing; } - - /// @brief Set the delta / difference between previous coordinate value (used internally on the timeline, to track changes to coordinates) - /// @param new_delta Indicates how much this Y value differs from the previous Y value - void Delta(double new_delta) { delta=new_delta; } - - /// Get the delta / difference between previous coordinate value (used internally on the timeline, to track changes to coordinates) - float Delta() { return delta; } - /// Get and Set JSON methods string Json(); ///< Generate JSON string of this object Json::Value JsonValue(); ///< Generate Json::JsonValue for this object diff --git a/src/Clip.cpp b/src/Clip.cpp index bd85d340..207494e3 100644 --- a/src/Clip.cpp +++ b/src/Clip.cpp @@ -340,13 +340,13 @@ std::shared_ptr Clip::GetFrame(int64_t requested_frame) frame->AddAudio(true, channel, 0, original_frame->GetAudioSamples(channel), original_frame->GetAudioSamplesCount(), 1.0); // Get time mapped frame number (used to increase speed, change direction, etc...) - std::shared_ptr new_frame = get_time_mapped_frame(frame, requested_frame); + get_time_mapped_frame(frame, requested_frame); // Apply effects to the frame (if any) - apply_effects(new_frame); + apply_effects(frame); // Return processed 'frame' - return new_frame; + return frame; } else // Throw error if reader not initialized @@ -389,7 +389,7 @@ void Clip::reverse_buffer(juce::AudioSampleBuffer* buffer) } // Adjust the audio and image of a time mapped frame -std::shared_ptr Clip::get_time_mapped_frame(std::shared_ptr frame, int64_t frame_number) +void Clip::get_time_mapped_frame(std::shared_ptr frame, int64_t frame_number) { // Check for valid reader if (!reader) @@ -400,7 +400,6 @@ std::shared_ptr Clip::get_time_mapped_frame(std::shared_ptr frame, if (time.Values.size() > 1) { const GenericScopedLock lock(getFrameCriticalSection); - std::shared_ptr new_frame; // create buffer and resampler juce::AudioSampleBuffer *samples = NULL; @@ -408,14 +407,7 @@ std::shared_ptr Clip::get_time_mapped_frame(std::shared_ptr frame, resampler = new AudioResampler(); // Get new frame number - int new_frame_number = adjust_frame_number_minimum(round(time.GetValue(frame_number))); - - // Create a new frame - int samples_in_frame = Frame::GetSamplesPerFrame(new_frame_number, reader->info.fps, reader->info.sample_rate, frame->GetAudioChannelsCount()); - new_frame = std::make_shared(new_frame_number, 1, 1, "#000000", samples_in_frame, frame->GetAudioChannelsCount()); - - // Copy the image from the new frame - new_frame->AddImage(std::shared_ptr(new QImage(*GetOrCreateFrame(new_frame_number)->GetImage()))); + int new_frame_number = frame->number; // Get delta (difference in previous Y value) int delta = int(round(time.GetDelta(frame_number))); @@ -463,7 +455,7 @@ std::shared_ptr Clip::get_time_mapped_frame(std::shared_ptr frame, start -= 1; for (int channel = 0; channel < channels; channel++) // Add new (slower) samples, to the frame object - new_frame->AddAudio(true, channel, 0, resampled_buffer->getReadPointer(channel, start), + frame->AddAudio(true, channel, 0, resampled_buffer->getReadPointer(channel, start), number_of_samples, 1.0f); // Clean up @@ -571,7 +563,7 @@ std::shared_ptr Clip::get_time_mapped_frame(std::shared_ptr frame, // Add the newly resized audio samples to the current frame for (int channel = 0; channel < channels; channel++) // Add new (slower) samples, to the frame object - new_frame->AddAudio(true, channel, 0, buffer->getReadPointer(channel), number_of_samples, 1.0f); + frame->AddAudio(true, channel, 0, buffer->getReadPointer(channel), number_of_samples, 1.0f); // Clean up buffer = NULL; @@ -592,7 +584,7 @@ std::shared_ptr Clip::get_time_mapped_frame(std::shared_ptr frame, // Add reversed samples to the frame object for (int channel = 0; channel < channels; channel++) - new_frame->AddAudio(true, channel, 0, samples->getReadPointer(channel), number_of_samples, 1.0f); + frame->AddAudio(true, channel, 0, samples->getReadPointer(channel), number_of_samples, 1.0f); } @@ -600,13 +592,7 @@ std::shared_ptr Clip::get_time_mapped_frame(std::shared_ptr frame, delete samples; samples = NULL; } - - // Return new time mapped frame - return new_frame; - - } else - // Use original frame - return frame; + } } // Adjust frame number minimum value diff --git a/src/Coordinate.cpp b/src/Coordinate.cpp index 41ee420a..60ea90b2 100644 --- a/src/Coordinate.cpp +++ b/src/Coordinate.cpp @@ -32,12 +32,12 @@ using namespace openshot; // Default constructor for a coordinate, which defaults the X and Y to zero (0,0) Coordinate::Coordinate() : - X(0), Y(0), increasing(true), repeated(1,1), delta(0.0) { + X(0), Y(0) { } // Constructor which also allows the user to set the X and Y Coordinate::Coordinate(double x, double y) : - X(x), Y(y), increasing(true), repeated(1,1), delta(0.0) { + X(x), Y(y) { } @@ -96,15 +96,4 @@ void Coordinate::SetJsonValue(Json::Value root) { X = root["X"].asDouble(); if (!root["Y"].isNull()) Y = root["Y"].asDouble(); - if (!root["increasing"].isNull()) - increasing = root["increasing"].asBool(); - if (!root["repeated"].isNull() && root["repeated"].isObject()) - { - if (!root["repeated"]["num"].isNull()) - repeated.num = root["repeated"]["num"].asInt(); - if (!root["repeated"]["den"].isNull()) - repeated.den = root["repeated"]["den"].asInt(); - } - if (!root["delta"].isNull()) - delta = root["delta"].asDouble(); } diff --git a/src/FrameMapper.cpp b/src/FrameMapper.cpp index 2a5d4276..73b7bb22 100644 --- a/src/FrameMapper.cpp +++ b/src/FrameMapper.cpp @@ -54,9 +54,6 @@ FrameMapper::FrameMapper(ReaderBase *reader, Fraction target, PulldownType targe // Adjust cache size based on size of frame and audio final_cache.SetMaxBytesFromInfo(OPEN_MP_NUM_PROCESSORS * 2, info.width, info.height, info.sample_rate, info.channels); - - // init mapping between original and target frames - Init(); } // Destructor @@ -205,22 +202,23 @@ void FrameMapper::Init() } } else { - // Map the remaining framerates using a simple Keyframe curve - // Calculate the difference (to be used as a multiplier) + // Map the remaining framerates using a linear algorithm double rate_diff = target.ToDouble() / original.ToDouble(); int64_t new_length = reader->info.video_length * rate_diff; - // Build curve for framerate mapping - Keyframe rate_curve; - rate_curve.AddPoint(1, 1, LINEAR); - rate_curve.AddPoint(new_length, reader->info.video_length, LINEAR); + // Calculate the value difference + double value_increment = (reader->info.video_length + 1) / (double) (new_length); // Loop through curve, and build list of frames + double original_frame_num = 1.0f; for (int64_t frame_num = 1; frame_num <= new_length; frame_num++) { // Add 2 fields per frame - AddField(rate_curve.GetInt(frame_num)); - AddField(rate_curve.GetInt(frame_num)); + AddField(round(original_frame_num)); + AddField(round(original_frame_num)); + + // Increment original frame number + original_frame_num += value_increment; } } @@ -310,6 +308,11 @@ void FrameMapper::Init() MappedFrame FrameMapper::GetMappedFrame(int64_t TargetFrameNumber) { + // Check if mappings are dirty (and need to be recalculated) + if (is_dirty) + // Recalculate mappings + Init(); + // Ignore mapping on single image readers if (info.has_video and !info.has_audio and info.has_single_image) { // Return the same number @@ -743,14 +746,16 @@ void FrameMapper::ChangeMapping(Fraction target_fps, PulldownType target_pulldow SWR_FREE(&avr); avr = NULL; } - - // Re-init mapping - Init(); } // Resample audio and map channels (if needed) void FrameMapper::ResampleMappedAudio(std::shared_ptr frame, int64_t original_frame_number) { + // Check if mappings are dirty (and need to be recalculated) + if (is_dirty) + // Recalculate mappings + Init(); + // Init audio buffers / variables int total_frame_samples = 0; int channels_in_frame = frame->GetAudioChannelsCount(); diff --git a/src/KeyFrame.cpp b/src/KeyFrame.cpp index 05a8a769..025484a3 100644 --- a/src/KeyFrame.cpp +++ b/src/KeyFrame.cpp @@ -296,17 +296,48 @@ bool Keyframe::IsIncreasing(int index) Process(); // Is index a valid point? - if (index >= 0 && index < Values.size()) - // Return value - return long(round(Values[index].IsIncreasing())); - else if (index < 0 && Values.size() > 0) - // Return the minimum value - return long(round(Values[0].IsIncreasing())); - else if (index >= Values.size() && Values.size() > 0) - // return the maximum value - return long(round(Values[Values.size() - 1].IsIncreasing())); + if (index >= 1 && (index + 1) < Values.size()) { + int64_t current_value = GetLong(index); + int64_t previous_value = 0; + int64_t next_value = 0; + int64_t previous_repeats = 0; + int64_t next_repeats = 0; + + // Loop backwards and look for the next unique value + for (vector::iterator backwards_it = Values.begin() + index; backwards_it != Values.begin(); backwards_it--) { + previous_value = long(round((*backwards_it).Y)); + if (previous_value == current_value) { + // Found same value + previous_repeats++; + } else { + // Found non repeating value, no more repeats found + break; + } + } + + // Loop forwards and look for the next unique value + for (vector::iterator forwards_it = Values.begin() + (index + 1); forwards_it != Values.end(); forwards_it++) { + next_value = long(round((*forwards_it).Y)); + if (next_value == current_value) { + // Found same value + next_repeats++; + } else { + // Found non repeating value, no more repeats found + break; + } + } + + if (current_value < next_value) { + // Increasing + return true; + } + else if (current_value >= next_value) { + // Decreasing + return false; + } + } else - // return the default direction of most curves (i.e. increasing is true) + // return default true (since most curves increase) return true; } @@ -385,6 +416,7 @@ void Keyframe::SetJsonValue(Json::Value root) { } // Get the fraction that represents how many times this value is repeated in the curve +// This is depreciated and will be removed soon. Fraction Keyframe::GetRepeatFraction(int64_t index) { // Check if it needs to be processed @@ -392,17 +424,42 @@ Fraction Keyframe::GetRepeatFraction(int64_t index) Process(); // Is index a valid point? - if (index >= 0 && index < Values.size()) - // Return value - return Values[index].Repeat(); - else if (index < 0 && Values.size() > 0) - // Return the minimum value - return Values[0].Repeat(); - else if (index >= Values.size() && Values.size() > 0) - // return the maximum value - return Values[Values.size() - 1].Repeat(); + if (index >= 1 && (index + 1) < Values.size()) { + int64_t current_value = GetLong(index); + int64_t previous_value = 0; + int64_t next_value = 0; + int64_t previous_repeats = 0; + int64_t next_repeats = 0; + + // Loop backwards and look for the next unique value + for (vector::iterator backwards_it = Values.begin() + index; backwards_it != Values.begin(); backwards_it--) { + previous_value = long(round((*backwards_it).Y)); + if (previous_value == current_value) { + // Found same value + previous_repeats++; + } else { + // Found non repeating value, no more repeats found + break; + } + } + + // Loop forwards and look for the next unique value + for (vector::iterator forwards_it = Values.begin() + (index + 1); forwards_it != Values.end(); forwards_it++) { + next_value = long(round((*forwards_it).Y)); + if (next_value == current_value) { + // Found same value + next_repeats++; + } else { + // Found non repeating value, no more repeats found + break; + } + } + + int64_t total_repeats = previous_repeats + next_repeats; + return Fraction(previous_repeats, total_repeats); + } else - // return a blank coordinate (0,0) + // return a blank coordinate return Fraction(1,1); } @@ -414,17 +471,48 @@ double Keyframe::GetDelta(int64_t index) Process(); // Is index a valid point? - if (index >= 0 && index < Values.size()) - // Return value - return Values[index].Delta(); - else if (index < 0 && Values.size() > 0) - // Return the minimum value - return Values[0].Delta(); - else if (index >= Values.size() && Values.size() > 0) - // return the maximum value - return Values[Values.size() - 1].Delta(); + if (index >= 1 && (index + 1) < Values.size()) { + int64_t current_value = GetLong(index); + int64_t previous_value = 0; + int64_t next_value = 0; + int64_t previous_repeats = 0; + int64_t next_repeats = 0; + + // Loop backwards and look for the next unique value + for (vector::iterator backwards_it = Values.begin() + index; backwards_it != Values.begin(); backwards_it--) { + previous_value = long(round((*backwards_it).Y)); + if (previous_value == current_value) { + // Found same value + previous_repeats++; + } else { + // Found non repeating value, no more repeats found + break; + } + } + + // Loop forwards and look for the next unique value + for (vector::iterator forwards_it = Values.begin() + (index + 1); forwards_it != Values.end(); forwards_it++) { + next_value = long(round((*forwards_it).Y)); + if (next_value == current_value) { + // Found same value + next_repeats++; + } else { + // Found non repeating value, no more repeats found + break; + } + } + + // Check for matching previous value (special case for 1st element) + if (current_value == previous_value) + previous_value = 0; + + if (previous_repeats == 1) + return current_value - previous_value; + else + return 0.0; + } else - // return a blank coordinate (0,0) + // return a blank coordinate return 0.0; } @@ -529,7 +617,7 @@ void Keyframe::PrintValues() { for (vector::iterator it = Values.begin() + 1; it != Values.end(); it++) { Coordinate c = *it; - cout << long(round(c.X)) << "\t" << c.Y << "\t" << c.IsIncreasing() << "\t" << c.Repeat().num << "\t" << c.Repeat().den << "\t" << c.Delta() << endl; + cout << long(round(c.X)) << "\t" << c.Y << "\t" << IsIncreasing(c.X) << "\t" << GetRepeatFraction(c.X).num << "\t" << GetRepeatFraction(c.X).den << "\t" << GetDelta(c.X) << endl; } } @@ -567,69 +655,6 @@ void Keyframe::Process() { // process segment p1,p2 ProcessSegment(x, p1, p2); } - - // Loop through each Value, and set the direction of the coordinate. This is used - // when time mapping, to determine what direction the audio waveforms play. - bool increasing = true; - int repeat_count = 1; - int64_t last_value = 0; - for (vector::iterator it = Values.begin() + 1; it != Values.end(); it++) { - int current_value = long(round((*it).Y)); - int64_t next_value = long(round((*it).Y)); - int64_t prev_value = long(round((*it).Y)); - if (it + 1 != Values.end()) - next_value = long(round((*(it + 1)).Y)); - if (it - 1 >= Values.begin()) - prev_value = long(round((*(it - 1)).Y)); - - // Loop forward and look for the next unique value (to determine direction) - for (vector::iterator direction_it = it + 1; direction_it != Values.end(); direction_it++) { - int64_t next = long(round((*direction_it).Y)); - - // Detect direction - if (current_value < next) - { - increasing = true; - break; - } - else if (current_value > next) - { - increasing = false; - break; - } - } - - // Set direction - (*it).IsIncreasing(increasing); - - // Detect repeated Y value - if (current_value == last_value) - // repeated, so increment count - repeat_count++; - else - // reset repeat counter - repeat_count = 1; - - // Detect how many 'more' times it's repeated - int additional_repeats = 0; - for (vector::iterator repeat_it = it + 1; repeat_it != Values.end(); repeat_it++) { - int64_t next = long(round((*repeat_it).Y)); - if (next == current_value) - // repeated, so increment count - additional_repeats++; - else - break; // stop looping - } - - // Set repeat fraction - (*it).Repeat(Fraction(repeat_count, repeat_count + additional_repeats)); - - // Set delta (i.e. different from previous unique Y value) - (*it).Delta(current_value - last_value); - - // track the last value - last_value = current_value; - } } // reset flag diff --git a/tests/KeyFrame_Tests.cpp b/tests/KeyFrame_Tests.cpp index fe96ea5a..cbd1a0e0 100644 --- a/tests/KeyFrame_Tests.cpp +++ b/tests/KeyFrame_Tests.cpp @@ -377,4 +377,20 @@ TEST(Keyframe_Remove_Duplicate_Point) // Spot check values from the curve CHECK_EQUAL(kf.GetLength(), 1); CHECK_CLOSE(kf.GetPoint(0).co.Y, 2.0, 0.01); +} + +TEST(Keyframe_Large_Number_Values) +{ + // Large value + int64_t large_value = 30 * 60 * 90; + + // Create a keyframe curve with 2 points + Keyframe kf; + kf.AddPoint(1, 1.0); + kf.AddPoint(large_value, 100.0); // 90 minutes long + + // Spot check values from the curve + CHECK_EQUAL(kf.GetLength(), large_value + 1); + CHECK_CLOSE(kf.GetPoint(0).co.Y, 1.0, 0.01); + CHECK_CLOSE(kf.GetPoint(1).co.Y, 100.0, 0.01); } \ No newline at end of file