Merge pull request #812 from OpenShot/clip-silence-past-reader-length

Silence Clip's audio when requesting a Frame past the end of the reader

src/Clip.cpp

@@ -712,10 +712,14 @@ std::shared_ptr<Frame> Clip::GetOrCreateFrame(int64_t number)
 			// This allows a clip to modify the pixels and audio of this frame without
 			// changing the underlying reader's frame data
 			auto reader_copy = std::make_shared<Frame>(*reader_frame.get());
-                        if (has_video.GetInt(number) == 0)
-                            reader_copy->AddColor(QColor(Qt::transparent));
-                        if (has_audio.GetInt(number) == 0)
-                            reader_copy->AddAudioSilence(reader_copy->GetAudioSamplesCount());
+            if (has_video.GetInt(number) == 0) {
+                // No video, so add transparent pixels
+                reader_copy->AddColor(QColor(Qt::transparent));
+            }
+            if (has_audio.GetInt(number) == 0 || number > reader->info.video_length) {
+                // No audio, so include silence (also, mute audio if past end of reader)
+                reader_copy->AddAudioSilence(reader_copy->GetAudioSamplesCount());
+            }
 			return reader_copy;
 		}
 

src/DummyReader.cpp

@@ -47,21 +47,23 @@ void DummyReader::init(Fraction fps, int width, int height, int sample_rate, int
 }
 
 // Blank constructor for DummyReader, with default settings.
-DummyReader::DummyReader() : dummy_cache(NULL), is_open(false) {
+DummyReader::DummyReader() : dummy_cache(NULL), last_cached_frame(NULL), image_frame(NULL), is_open(false) {
 
 	// Initialize important variables
 	init(Fraction(24,1), 1280, 768, 44100, 2, 30.0);
 }
 
 // Constructor for DummyReader.  Pass a framerate and samplerate.
-DummyReader::DummyReader(Fraction fps, int width, int height, int sample_rate, int channels, float duration) : dummy_cache(NULL), is_open(false) {
+DummyReader::DummyReader(Fraction fps, int width, int height, int sample_rate, int channels, float duration) :
+    dummy_cache(NULL), last_cached_frame(NULL), image_frame(NULL), is_open(false) {
 
 	// Initialize important variables
 	init(fps, width, height, sample_rate, channels, duration);
 }
 
 // Constructor which also takes a cache object
-DummyReader::DummyReader(Fraction fps, int width, int height, int sample_rate, int channels, float duration, CacheBase* cache) : is_open(false) {
+DummyReader::DummyReader(Fraction fps, int width, int height, int sample_rate, int channels, float duration,
+                         CacheBase* cache) :  last_cached_frame(NULL), image_frame(NULL), is_open(false) {
 
 	// Initialize important variables
 	init(fps, width, height, sample_rate, channels, duration);
@@ -117,6 +119,7 @@ std::shared_ptr<Frame> DummyReader::GetFrame(int64_t requested_frame)
 
 		// Always return same frame (regardless of which frame number was requested)
 		image_frame->number = requested_frame;
+		last_cached_frame = image_frame;
 		return image_frame;
 
 	} else if (dummy_cache_count > 0) {
@@ -126,8 +129,12 @@ std::shared_ptr<Frame> DummyReader::GetFrame(int64_t requested_frame)
 		// Get a frame from the dummy cache
 		std::shared_ptr<openshot::Frame> f = dummy_cache->GetFrame(requested_frame);
 		if (f) {
-			// return frame from cache (if found)
-			return f;
+            // return frame from cache (if found)
+            last_cached_frame = f;
+            return f;
+        } else if (last_cached_frame) {
+		    // If available, return last cached frame
+            return last_cached_frame;
 		} else {
 			// No cached frame found
 			throw InvalidFile("Requested frame not found. You can only access Frame numbers that exist in the Cache object.", "dummy");

src/DummyReader.h

@@ -87,6 +87,7 @@ namespace openshot
 	private:
 		CacheBase* dummy_cache;
 		std::shared_ptr<openshot::Frame> image_frame;
+        std::shared_ptr<openshot::Frame> last_cached_frame;
 		bool is_open;
 
 		/// Initialize variables used by constructor

tests/Clip.cpp

@@ -20,6 +20,7 @@
 #include <QSize>
 
 #include "Clip.h"
+#include "DummyReader.h"
 #include "Enums.h"
 #include "Exceptions.h"
 #include "Frame.h"
@@ -277,3 +278,62 @@ TEST_CASE( "has_video", "[libopenshot][clip]" )
     CHECK(i3->size() == f3_size);
     CHECK(i3->pixelColor(20, 20) != trans_color);
 }
+
+TEST_CASE( "access frames past reader length", "[libopenshot][clip]" )
+{
+    // Create cache object to hold test frames
+    openshot::CacheMemory cache;
+
+    // Let's create some test frames
+    for (int64_t frame_number = 1; frame_number <= 30; frame_number++) {
+        // Create blank frame (with specific frame #, samples, and channels)
+        // Sample count should be 44100 / 30 fps = 1470 samples per frame
+        int sample_count = 1470;
+        auto f = std::make_shared<openshot::Frame>(frame_number, sample_count, 2);
+
+        // Create test samples with incrementing value
+        float *audio_buffer = new float[sample_count];
+        for (int64_t sample_number = 0; sample_number < sample_count; sample_number++) {
+            // Generate an incrementing audio sample value (just as an example)
+            audio_buffer[sample_number] = float(frame_number) + (float(sample_number) / float(sample_count));
+        }
+
+        // Add custom audio samples to Frame (bool replaceSamples, int destChannel, int destStartSample, const float* source,
+        f->AddAudio(true, 0, 0, audio_buffer, sample_count, 1.0); // add channel 1
+        f->AddAudio(true, 1, 0, audio_buffer, sample_count, 1.0); // add channel 2
+
+        // Add test frame to dummy reader
+        cache.Add(f);
+
+        delete[] audio_buffer;
+    }
+
+    // Create a dummy reader, with a pre-existing cache
+    openshot::DummyReader r(openshot::Fraction(30, 1), 1920, 1080, 44100, 2, 1.0, &cache);
+    r.Open(); // Open the reader
+
+    openshot::Clip c1;
+    c1.Reader(&r);
+    c1.Open();
+
+    // Get the last valid frame #
+    std::shared_ptr<openshot::Frame> frame = c1.GetFrame(30);
+
+    CHECK(frame->GetAudioSamples(0)[0] == Approx(30.0).margin(0.00001));
+    CHECK(frame->GetAudioSamples(0)[600] == Approx(30.4081631).margin(0.00001));
+    CHECK(frame->GetAudioSamples(0)[1200] == Approx(30.8163261).margin(0.00001));
+
+    // Get the +1 past the end of the reader (should be audio silence)
+    frame = c1.GetFrame(31);
+
+    CHECK(frame->GetAudioSamples(0)[0] == Approx(0.0).margin(0.00001));
+    CHECK(frame->GetAudioSamples(0)[600] == Approx(0.0).margin(0.00001));
+    CHECK(frame->GetAudioSamples(0)[1200] == Approx(0.0).margin(0.00001));
+
+    // Get the +2 past the end of the reader (should be audio silence)
+    frame = c1.GetFrame(32);
+
+    CHECK(frame->GetAudioSamples(0)[0] == Approx(0.0).margin(0.00001));
+    CHECK(frame->GetAudioSamples(0)[600] == Approx(0.0).margin(0.00001));
+    CHECK(frame->GetAudioSamples(0)[1200] == Approx(0.0).margin(0.00001));
+}

tests/DummyReader.cpp

@@ -21,7 +21,6 @@
 #include "Frame.h"
 
 TEST_CASE( "Default constructor", "[libopenshot][dummyreader]" ) {
-	// Create a default fraction (should be 1/1)
 	openshot::DummyReader r;
 	r.Open(); // Open the reader
 
@@ -41,7 +40,6 @@ TEST_CASE( "Default constructor", "[libopenshot][dummyreader]" ) {
 }
 
 TEST_CASE( "Constructor", "[libopenshot][dummyreader]" ) {
-	// Create a default fraction (should be 1/1)
 	openshot::DummyReader r(openshot::Fraction(30, 1), 1920, 1080, 44100, 2, 60.0);
 	r.Open(); // Open the reader
 
@@ -56,7 +54,6 @@ TEST_CASE( "Constructor", "[libopenshot][dummyreader]" ) {
 }
 
 TEST_CASE( "Blank_Frame", "[libopenshot][dummyreader]" ) {
-	// Create a default fraction (should be 1/1)
 	openshot::DummyReader r(openshot::Fraction(30, 1), 1920, 1080, 44100, 2, 30.0);
 	r.Open(); // Open the reader
 
@@ -96,7 +93,7 @@ TEST_CASE( "Fake_Frame", "[libopenshot][dummyreader]" ) {
 		delete[] audio_buffer;
 	}
 
-	// Create a default fraction (should be 1/1)
+	// Create a dummy reader, with a pre-existing cache
 	openshot::DummyReader r(openshot::Fraction(30, 1), 1920, 1080, 44100, 2, 30.0, &cache);
 	r.Open(); // Open the reader
 
@@ -114,30 +111,6 @@ TEST_CASE( "Fake_Frame", "[libopenshot][dummyreader]" ) {
 	r.Close();
 }
 
-TEST_CASE( "Invalid_Fake_Frame", "[libopenshot][dummyreader]" ) {
-	// Create fake frames (with specific frame #, samples, and channels)
-	auto f1 = std::make_shared<openshot::Frame>(1, 1470, 2);
-	auto f2 = std::make_shared<openshot::Frame>(2, 1470, 2);
-
-	// Add test frames to cache object
-	openshot::CacheMemory cache;
-	cache.Add(f1);
-	cache.Add(f2);
-
-	// Create a default fraction (should be 1/1)
-	openshot::DummyReader r(openshot::Fraction(30, 1), 1920, 1080, 44100, 2, 30.0, &cache);
-	r.Open();
-
-	// Verify exception
-	CHECK(r.GetFrame(1)->number == 1);
-	CHECK(r.GetFrame(2)->number == 2);
-	CHECK_THROWS_AS(r.GetFrame(3)->number, openshot::InvalidFile);
-
-	// Clean up
-	cache.Clear();
-	r.Close();
-}
-
 TEST_CASE( "Json", "[libopenshot][dummyreader]") {
 	openshot::DummyReader r1;
 	openshot::DummyReader r2(openshot::Fraction(24, 1), 1280, 768, 44100, 2, 30.0);

tests/FrameMapper.cpp

@@ -243,7 +243,7 @@ TEST_CASE( "resample_audio_mapper", "[libopenshot][framemapper]" ) {
 		delete[] audio_buffer;
 	}
 
-	// Create a default fraction (should be 1/1)
+	// Create a dummy reader, with a pre-existing cache
 	openshot::DummyReader r(openshot::Fraction(30, 1), 1, 1, 44100, 2, 30.0, &cache);
 	r.Open(); // Open the reader
 
@@ -383,7 +383,7 @@ TEST_CASE( "redistribute_samples_per_frame", "[libopenshot][framemapper]" ) {
 		delete[] audio_buffer;
 	}
 
-	// Create a default fraction (should be 1/1)
+	// Create a dummy reader, with a pre-existing cache
 	openshot::DummyReader r(openshot::Fraction(30, 1), 1920, 1080, 44100, 2, 30.0, &cache);
 	r.Open(); // Open the reader