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

Author: jonoomph

include/Clip.h

@@ -127,7 +127,7 @@ namespace openshot {
 		std::shared_ptr<Frame> GetOrCreateFrame(int64_t number);
 
 		/// Adjust the audio and image of a time mapped frame
-		std::shared_ptr<Frame> get_time_mapped_frame(std::shared_ptr<Frame> frame, int64_t frame_number);
+		void get_time_mapped_frame(std::shared_ptr<Frame> frame, int64_t frame_number);
 
 		/// Init default settings for a clip
 		void init_settings();

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

src/Clip.cpp

@@ -340,13 +340,13 @@ std::shared_ptr<Frame> 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<Frame> 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<Frame> Clip::get_time_mapped_frame(std::shared_ptr<Frame> frame, int64_t frame_number)
+void Clip::get_time_mapped_frame(std::shared_ptr<Frame> frame, int64_t frame_number)
 {
 	// Check for valid reader
 	if (!reader)
@@ -400,22 +400,14 @@ std::shared_ptr<Frame> Clip::get_time_mapped_frame(std::shared_ptr<Frame> frame,
 	if (time.Values.size() > 1)
 	{
 		const GenericScopedLock<CriticalSection> lock(getFrameCriticalSection);
-		std::shared_ptr<Frame> new_frame;
 
 		// create buffer and resampler
 		juce::AudioSampleBuffer *samples = NULL;
 		if (!resampler)
 			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<Frame>(new_frame_number, 1, 1, "#000000", samples_in_frame, frame->GetAudioChannelsCount());
-
-		// Copy the image from the new frame
-		new_frame->AddImage(std::shared_ptr<QImage>(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<Frame> Clip::get_time_mapped_frame(std::shared_ptr<Frame> 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<Frame> Clip::get_time_mapped_frame(std::shared_ptr<Frame> 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,21 +584,15 @@ std::shared_ptr<Frame> Clip::get_time_mapped_frame(std::shared_ptr<Frame> 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);
 
 
 			}
 
 			delete samples;
 			samples = NULL;
 		}
-
-		// Return new time mapped frame
-		return new_frame;
-
-	} else
-		// Use original frame
-		return frame;
+	}
 }
 
 // Adjust frame number minimum value

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();
 }

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> 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();