Merge remote-tracking branch 'origin/develop' into opencv

.github/stale.yml

@@ -10,7 +10,7 @@ exemptLabels:
 # Label to use when marking an issue as stale
 staleLabel: stale
 # Comment to post when marking an issue as stale. Set to `false` to disable
-markComment: >
+markComment: |
   Thank you so much for submitting an issue to help improve OpenShot Video Editor. We are sorry about this, but this particular issue has gone unnoticed for quite some time. To help keep the OpenShot GitHub Issue Tracker organized and focused, we must ensure that every issue is correctly labelled and triaged, to get the proper attention.
 
   This issue will be closed, as it meets the following criteria:

src/Clip.cpp

@@ -161,6 +161,8 @@ Clip::Clip(ReaderBase* new_reader) : resampler(NULL), reader(new_reader), alloca
 	if (reader) {
 		End(reader->info.duration);
 		reader->ParentClip(this);
+		// Init reader info struct and cache size
+		init_reader_settings();
 	}
 }
 
@@ -220,7 +222,8 @@ Clip::Clip(std::string path) : resampler(NULL), reader(NULL), allocated_reader(N
 		reader->ParentClip(this);
 		allocated_reader = reader;
 		// Init reader info struct and cache size
-		init_reader_settings();	}
+		init_reader_settings();
+	}
 }
 
 // Destructor
@@ -325,7 +328,7 @@ std::shared_ptr<Frame> Clip::GetFrame(int64_t frame_number)
 {
 	// Check for open reader (or throw exception)
 	if (!is_open)
-		throw ReaderClosed("The Clip is closed.  Call Open() before calling this method", "N/A");
+		throw ReaderClosed("The Clip is closed.  Call Open() before calling this method.");
 
 	if (reader)
 	{
@@ -346,7 +349,7 @@ std::shared_ptr<Frame> Clip::GetFrame(std::shared_ptr<openshot::Frame> frame, in
 {
 	// Check for open reader (or throw exception)
 	if (!is_open)
-		throw ReaderClosed("The Clip is closed.  Call Open() before calling this method", "N/A");
+		throw ReaderClosed("The Clip is closed.  Call Open() before calling this method.");
 
 	if (reader)
 	{
@@ -698,19 +701,17 @@ std::shared_ptr<Frame> Clip::GetOrCreateFrame(int64_t number)
 		ZmqLogger::Instance()->AppendDebugMethod("Clip::GetOrCreateFrame (from reader)", "number", number);
 
 		// Attempt to get a frame (but this could fail if a reader has just been closed)
-		std::shared_ptr<Frame> reader_frame = reader->GetFrame(number);
+		auto reader_frame = reader->GetFrame(number);
 
 		// Return real frame
 		if (reader_frame) {
 			// Create a new copy of reader frame
 			// This allows a clip to modify the pixels and audio of this frame without
 			// changing the underlying reader's frame data
 			//std::shared_ptr<Frame> reader_copy(new Frame(number, 1, 1, "#000000", reader_frame->GetAudioSamplesCount(), reader_frame->GetAudioChannelsCount()));
-			std::shared_ptr<Frame> reader_copy(new Frame(*reader_frame.get()));
-			{
-				reader_copy->SampleRate(reader_frame->SampleRate());
-				reader_copy->ChannelsLayout(reader_frame->ChannelsLayout());
-			}
+			auto reader_copy = std::make_shared<Frame>(*reader_frame.get());
+			reader_copy->SampleRate(reader_frame->SampleRate());
+			reader_copy->ChannelsLayout(reader_frame->ChannelsLayout());
 			return reader_copy;
 		}
 
@@ -729,7 +730,9 @@ std::shared_ptr<Frame> Clip::GetOrCreateFrame(int64_t number)
 	ZmqLogger::Instance()->AppendDebugMethod("Clip::GetOrCreateFrame (create blank)", "number", number, "estimated_samples_in_frame", estimated_samples_in_frame);
 
 	// Create blank frame
-	std::shared_ptr<Frame> new_frame = std::make_shared<Frame>(number, reader->info.width, reader->info.height, "#000000", estimated_samples_in_frame, reader->info.channels);
+	auto new_frame = std::make_shared<Frame>(
+		number, reader->info.width, reader->info.height,
+		"#000000", estimated_samples_in_frame, reader->info.channels);
 	new_frame->SampleRate(reader->info.sample_rate);
 	new_frame->ChannelsLayout(reader->info.channel_layout);
 	new_frame->AddAudioSilence(estimated_samples_in_frame);
@@ -1176,30 +1179,21 @@ void Clip::apply_keyframes(std::shared_ptr<Frame> frame, int width, int height)
 	switch (scale)
 	{
 		case (SCALE_FIT): {
-			// keep aspect ratio
 			source_size.scale(width, height, Qt::KeepAspectRatio);
 
 			// Debug output
 			ZmqLogger::Instance()->AppendDebugMethod("Clip::apply_keyframes (Scale: SCALE_FIT)", "frame->number", frame->number, "source_width", source_size.width(), "source_height", source_size.height());
 			break;
 		}
 		case (SCALE_STRETCH): {
-			// ignore aspect ratio
 			source_size.scale(width, height, Qt::IgnoreAspectRatio);
 
 			// Debug output
 			ZmqLogger::Instance()->AppendDebugMethod("Clip::apply_keyframes (Scale: SCALE_STRETCH)", "frame->number", frame->number, "source_width", source_size.width(), "source_height", source_size.height());
 			break;
 		}
 		case (SCALE_CROP): {
-			QSize width_size(width, round(width / (float(source_size.width()) / float(source_size.height()))));
-			QSize height_size(round(height / (float(source_size.height()) / float(source_size.width()))), height);
-
-			// respect aspect ratio
-			if (width_size.width() >= width && width_size.height() >= height)
-				source_size.scale(width_size.width(), width_size.height(), Qt::KeepAspectRatio);
-			else
-				source_size.scale(height_size.width(), height_size.height(), Qt::KeepAspectRatio);
+			source_size.scale(width, height, Qt::KeepAspectRatioByExpanding);
 
 			// Debug output
 			ZmqLogger::Instance()->AppendDebugMethod("Clip::apply_keyframes (Scale: SCALE_CROP)", "frame->number", frame->number, "source_width", source_size.width(), "source_height", source_size.height());
@@ -1276,7 +1270,6 @@ void Clip::apply_keyframes(std::shared_ptr<Frame> frame, int width, int height)
 	float origin_x_value = origin_x.GetValue(frame->number);
 	float origin_y_value = origin_y.GetValue(frame->number);
 
-	bool transformed = false;
 	QTransform transform;
 
 	// Transform source image (if needed)
@@ -1285,7 +1278,6 @@ void Clip::apply_keyframes(std::shared_ptr<Frame> frame, int width, int height)
 	if (!isEqual(x, 0) || !isEqual(y, 0)) {
 		// TRANSLATE/MOVE CLIP
 		transform.translate(x, y);
-		transformed = true;
 	}
 
 	if (!isEqual(r, 0) || !isEqual(shear_x_value, 0) || !isEqual(shear_y_value, 0)) {
@@ -1296,7 +1288,6 @@ void Clip::apply_keyframes(std::shared_ptr<Frame> frame, int width, int height)
 		transform.rotate(r);
 		transform.shear(shear_x_value, shear_y_value);
 		transform.translate(-origin_x_offset,-origin_y_offset);
-		transformed = true;
 	}
 
 	// SCALE CLIP (if needed)
@@ -1305,24 +1296,21 @@ void Clip::apply_keyframes(std::shared_ptr<Frame> frame, int width, int height)
 
 	if (!isEqual(source_width_scale, 1.0) || !isEqual(source_height_scale, 1.0)) {
 		transform.scale(source_width_scale, source_height_scale);
-		transformed = true;
 	}
 
 	// Debug output
-	ZmqLogger::Instance()->AppendDebugMethod("Clip::apply_keyframes (Transform: Composite Image Layer: Prepare)", "frame->number", frame->number, "transformed", transformed);
+	ZmqLogger::Instance()->AppendDebugMethod("Clip::apply_keyframes (Transform: Composite Image Layer: Prepare)", "frame->number", frame->number);
 
 	/* COMPOSITE SOURCE IMAGE (LAYER) ONTO FINAL IMAGE */
-	std::shared_ptr<QImage> new_image;
-	new_image = std::shared_ptr<QImage>(new QImage(QSize(width, height), source_image->format()));
+	auto new_image = std::make_shared<QImage>(QSize(width, height), source_image->format());
 	new_image->fill(QColor(QString::fromStdString("#00000000")));
 
 	// Load timeline's new frame image into a QPainter
 	QPainter painter(new_image.get());
 	painter.setRenderHints(QPainter::Antialiasing | QPainter::SmoothPixmapTransform | QPainter::TextAntialiasing, true);
 
-	// Apply transform (translate, rotate, scale)... if any
-	if (transformed)
-		painter.setTransform(transform);
+	// Apply transform (translate, rotate, scale)
+	painter.setTransform(transform);
 
 	// Composite a new layer onto the image
 	painter.setCompositionMode(QPainter::CompositionMode_SourceOver);

src/Clip.h

@@ -192,7 +192,7 @@ namespace openshot {
 		virtual ~Clip();
 
 		/// Get the cache object used by this clip
-		CacheMemory* GetCache() { return &cache; };
+		CacheMemory* GetCache() override { return &cache; };
 
 		/// Determine if reader is open or closed
 		bool IsOpen() override { return is_open; };
@@ -232,10 +232,10 @@ namespace openshot {
 		/// @returns The modified openshot::Frame object
 		/// @param frame This is ignored on Clip, due to caching optimizations. This frame instance is clobbered with the source frame.
 		/// @param frame_number The frame number (starting at 1) of the clip or effect on the timeline.
-		std::shared_ptr<openshot::Frame> GetFrame(std::shared_ptr<openshot::Frame> frame, int64_t frame_number);
+		std::shared_ptr<openshot::Frame> GetFrame(std::shared_ptr<openshot::Frame> frame, int64_t frame_number) override;
 
 		/// Open the internal reader
-		void Open();
+		void Open() override;
 
 		/// @brief Set the current reader
 		/// @param new_reader The reader to be used by this clip

src/ClipBase.h

@@ -41,8 +41,8 @@
 #include "Json.h"
 #include "TimelineBase.h"
 
-namespace openshot {
 
+namespace openshot {
 	/**
 	 * @brief This abstract class is the base class, used by all clips in libopenshot.
 	 *

src/FrameMapper.cpp

@@ -117,15 +117,6 @@ void FrameMapper::Init()
 	// Clear cache
 	final_cache.Clear();
 
-	// Get clip position from parent clip (if any)
-	float clipPosition = 0.0;
-	float clipStart = 0.0;
-	Clip *parent = (Clip *) ParentClip();
-	if (parent) {
-		clipPosition = parent->Position();
-		clipStart = parent->Start();
-	}
-
 	// Some framerates are handled special, and some use a generic Keyframe curve to
 	// map the framerates. These are the special framerates:
 	if ((fabs(original.ToFloat() - 24.0) < 1e-7 || fabs(original.ToFloat() - 25.0) < 1e-7 || fabs(original.ToFloat() - 30.0) < 1e-7) &&
@@ -267,12 +258,12 @@ void FrameMapper::Init()
 			// the original sample rate.
 			int64_t end_samples_frame = start_samples_frame;
 			int end_samples_position = start_samples_position;
-			int remaining_samples = Frame::GetSamplesPerFrame(AdjustFrameNumber(frame_number, clipPosition, clipStart), target, reader->info.sample_rate, reader->info.channels);
+			int remaining_samples = Frame::GetSamplesPerFrame(AdjustFrameNumber(frame_number), target, reader->info.sample_rate, reader->info.channels);
 
 			while (remaining_samples > 0)
 			{
 				// get original samples
-				int original_samples = Frame::GetSamplesPerFrame(AdjustFrameNumber(end_samples_frame, clipPosition, clipStart), original, reader->info.sample_rate, reader->info.channels) - end_samples_position;
+				int original_samples = Frame::GetSamplesPerFrame(AdjustFrameNumber(end_samples_frame), original, reader->info.sample_rate, reader->info.channels) - end_samples_position;
 
 				// Enough samples
 				if (original_samples >= remaining_samples)
@@ -292,12 +283,12 @@ void FrameMapper::Init()
 
 
 			// Create the sample mapping struct
-			SampleRange Samples = {start_samples_frame, start_samples_position, end_samples_frame, end_samples_position, Frame::GetSamplesPerFrame(AdjustFrameNumber(frame_number, clipPosition, clipStart), target, reader->info.sample_rate, reader->info.channels)};
+			SampleRange Samples = {start_samples_frame, start_samples_position, end_samples_frame, end_samples_position, Frame::GetSamplesPerFrame(AdjustFrameNumber(frame_number), target, reader->info.sample_rate, reader->info.channels)};
 
 			// Reset the audio variables
 			start_samples_frame = end_samples_frame;
 			start_samples_position = end_samples_position + 1;
-			if (start_samples_position >= Frame::GetSamplesPerFrame(AdjustFrameNumber(start_samples_frame, clipPosition, clipStart), original, reader->info.sample_rate, reader->info.channels))
+			if (start_samples_position >= Frame::GetSamplesPerFrame(AdjustFrameNumber(start_samples_frame), original, reader->info.sample_rate, reader->info.channels))
 			{
 				start_samples_frame += 1; // increment the frame (since we need to wrap onto the next one)
 				start_samples_position = 0; // reset to 0, since we wrapped
@@ -363,17 +354,8 @@ std::shared_ptr<Frame> FrameMapper::GetOrCreateFrame(int64_t number)
 {
 	std::shared_ptr<Frame> new_frame;
 
-	// Get clip position from parent clip (if any)
-	float clipPosition = 0.0;
-	float clipStart = 0.0;
-	Clip *parent = (Clip *) ParentClip();
-	if (parent) {
-		clipPosition = parent->Position();
-		clipStart = parent->Start();
-	}
-
 	// Init some basic properties about this frame (keep sample rate and # channels the same as the original reader for now)
-	int samples_in_frame = Frame::GetSamplesPerFrame(AdjustFrameNumber(number, clipPosition, clipStart), target, reader->info.sample_rate, reader->info.channels);
+	int samples_in_frame = Frame::GetSamplesPerFrame(AdjustFrameNumber(number), target, reader->info.sample_rate, reader->info.channels);
 
 	try {
 		// Debug output
@@ -423,15 +405,6 @@ std::shared_ptr<Frame> FrameMapper::GetFrame(int64_t requested_frame)
 	final_frame = final_cache.GetFrame(requested_frame);
 	if (final_frame) return final_frame;
 
-	// Get clip position from parent clip (if any)
-	float clipPosition = 0.0;
-	float clipStart = 0.0;
-	Clip *parent = (Clip *) ParentClip();
-	if (parent) {
-		clipPosition = parent->Position();
-		clipStart = parent->Start();
-	}
-
 	// Minimum number of frames to process (for performance reasons)
 	// Dialing this down to 1 for now, as it seems to improve performance, and reduce export crashes
 	int minimum_frames = 1;
@@ -455,7 +428,7 @@ std::shared_ptr<Frame> FrameMapper::GetFrame(int64_t requested_frame)
 
 		// Get # of channels in the actual frame
 		int channels_in_frame = mapped_frame->GetAudioChannelsCount();
-		int samples_in_frame = Frame::GetSamplesPerFrame(AdjustFrameNumber(frame_number, clipPosition, clipStart), target, mapped_frame->SampleRate(), channels_in_frame);
+		int samples_in_frame = Frame::GetSamplesPerFrame(AdjustFrameNumber(frame_number), target, mapped_frame->SampleRate(), channels_in_frame);
 
 		// Determine if mapped frame is identical to source frame
 		// including audio sample distribution according to mapped.Samples,
@@ -778,15 +751,6 @@ void FrameMapper::ResampleMappedAudio(std::shared_ptr<Frame> frame, int64_t orig
 		// Recalculate mappings
 		Init();
 
-	// Get clip position from parent clip (if any)
-	float clipPosition = 0.0;
-	float clipStart = 0.0;
-	Clip *parent = (Clip *) ParentClip();
-	if (parent) {
-		clipPosition = parent->Position();
-		clipStart = parent->Start();
-	}
-
 	// Init audio buffers / variables
 	int total_frame_samples = 0;
 	int channels_in_frame = frame->GetAudioChannelsCount();
@@ -848,7 +812,7 @@ void FrameMapper::ResampleMappedAudio(std::shared_ptr<Frame> frame, int64_t orig
 	}
 
 	// Update total samples & input frame size (due to bigger or smaller data types)
-	total_frame_samples = Frame::GetSamplesPerFrame(AdjustFrameNumber(frame->number, clipPosition, clipStart), target, info.sample_rate, info.channels);
+	total_frame_samples = Frame::GetSamplesPerFrame(AdjustFrameNumber(frame->number), target, info.sample_rate, info.channels);
 
 	ZmqLogger::Instance()->AppendDebugMethod("FrameMapper::ResampleMappedAudio (adjust # of samples)", "total_frame_samples", total_frame_samples, "info.sample_rate", info.sample_rate, "sample_rate_in_frame", sample_rate_in_frame, "info.channels", info.channels, "channels_in_frame", channels_in_frame, "original_frame_number", original_frame_number);
 
@@ -959,13 +923,23 @@ void FrameMapper::ResampleMappedAudio(std::shared_ptr<Frame> frame, int64_t orig
 }
 
 // Adjust frame number for Clip position and start (which can result in a different number)
-int64_t FrameMapper::AdjustFrameNumber(int64_t clip_frame_number, float position, float start) {
+int64_t FrameMapper::AdjustFrameNumber(int64_t clip_frame_number) {
+
+	// Get clip position from parent clip (if any)
+	float position = 0.0;
+	float start = 0.0;
+	Clip *parent = (Clip *) ParentClip();
+	if (parent) {
+		position = parent->Position();
+		start = parent->Start();
+	}
 
+	// Adjust start frame and position based on parent clip. This prevents ensures the same
+	// frame # is used by mapped readers and clips, when calculating samples per frame. Thus,
+	// this prevents gaps and mismatches in # of samples.
 	int64_t clip_start_frame = (start * info.fps.ToDouble()) + 1;
 	int64_t clip_start_position = round(position * info.fps.ToDouble()) + 1;
-
 	int64_t frame_number = clip_frame_number + clip_start_position - clip_start_frame;
 
-	///std::cout << "Conv Position " << round(position * info.fps.ToDouble()) << " position: " << position << " info::fps: " << info.fps.ToDouble() << std::endl; 
 	return frame_number;
 }

src/FrameMapper.h

@@ -155,7 +155,7 @@ namespace openshot
 		std::shared_ptr<Frame> GetOrCreateFrame(int64_t number);
 
 		/// Adjust frame number for Clip position and start (which can result in a different number)
-		int64_t AdjustFrameNumber(int64_t clip_frame_number, float position, float start);
+		int64_t AdjustFrameNumber(int64_t clip_frame_number);
 
 		// Use the original and target frame rates and a pull-down technique to create
 		// a mapping between the original fields and frames or a video to a new frame rate.

src/Qt/VideoCacheThread.cpp

@@ -94,8 +94,10 @@ namespace openshot
 		while (!threadShouldExit() && is_playing) {
 
 	    // Cache frames before the other threads need them
-	    // Cache frames up to the max frames
-	    while (speed == 1 && (position - current_display_frame) < max_frames)
+	    // Cache frames up to the max frames. Reset to current position
+		// if cache gets too far away from display frame. Cache frames
+		// even when player is paused (i.e. speed 0).
+	    while ((position - current_display_frame) < max_frames)
 	    {
 	    	// Only cache up till the max_frames amount... then sleep
 			try
@@ -104,6 +106,14 @@ namespace openshot
 					ZmqLogger::Instance()->AppendDebugMethod("VideoCacheThread::run (cache frame)", "position", position, "current_display_frame", current_display_frame, "max_frames", max_frames, "needed_frames", (position - current_display_frame));
 
 					// Force the frame to be generated
+					if (reader->GetCache()->GetSmallestFrame()) {
+						int64_t smallest_cached_frame = reader->GetCache()->GetSmallestFrame()->number;
+						if (smallest_cached_frame > current_display_frame) {
+							// Cache position has gotten too far away from current display frame.
+							// Reset the position to the current display frame.
+							position = current_display_frame;
+						}
+					}
 					reader->GetFrame(position);
 				}
 
@@ -113,14 +123,8 @@ namespace openshot
 				// Ignore out of bounds frame exceptions
 			}
 
-			// Is cache position behind current display frame?
-			if (position < current_display_frame) {
-				// Jump ahead
-				position = current_display_frame;
-			}
-
 	    	// Increment frame number
-	    	position++;
+			position++;
 	    }
 
 		// Sleep for 1 frame length