Fixed an issue with reversed resampled audio in FrameMapper, which caused lots of clicks / seams between frames... Also, added a new overload to GetInterleavedAudioSamples, to reverse the samples before returning the float* array. Essentially, the FrameMapper is now aware of it's parent clip, and especially the 'time' keyframe, if the audio is in the forward or reverse direction. Also fixed a memory leak in time remapping.

Author: jonoomph

src/Clip.cpp

@@ -510,7 +510,6 @@ void Clip::reverse_buffer(juce::AudioBuffer<float>* buffer)
 		buffer->addFrom(channel, 0, reversed->getReadPointer(channel), number_of_samples, 1.0f);
 
 	delete reversed;
-	reversed = nullptr;
 }
 
 // Adjust the audio and image of a time mapped frame
@@ -569,7 +568,7 @@ void Clip::apply_timemapping(std::shared_ptr<Frame> frame)
 			frame->AddAudioSilence(target_sample_count);
 			return;
 		}
-		
+
 		// Allocate a new sample buffer for these delta frames
 		source_samples = new juce::AudioBuffer<float>(Reader()->info.channels, source_sample_count);
 		source_samples->clear();
@@ -578,26 +577,23 @@ void Clip::apply_timemapping(std::shared_ptr<Frame> frame)
 		int remaining_samples = source_sample_count;
 		int source_pos = 0;
 		while (remaining_samples > 0) {
-			int frame_sample_count = GetOrCreateFrame(location.frame, false)->GetAudioSamplesCount() - location.sample_start;
+			std::shared_ptr<Frame> source_frame = GetOrCreateFrame(location.frame, false);
+			int frame_sample_count = source_frame->GetAudioSamplesCount() - location.sample_start;
 
 			if (frame_sample_count == 0) {
 				// No samples found in source frame (fill with silence)
-				int expected_frame_sample_count = Frame::GetSamplesPerFrame(clip_frame_number, Reader()->info.fps, Reader()->info.sample_rate, Reader()->info.channels);
-				source_samples->setSize(Reader()->info.channels, expected_frame_sample_count, false, true, false);
-				source_samples->clear();
 				if (is_increasing) {
 					location.frame++;
 				} else {
 					location.frame--;
 				}
 				location.sample_start = 0;
-				remaining_samples = 0;
 				break;
 			}
 			if (remaining_samples - frame_sample_count >= 0) {
 				// Use all frame samples & increment location
-				for (int channel = 0; channel < Reader()->info.channels; channel++) {
-					source_samples->addFrom(channel, source_pos, GetOrCreateFrame(location.frame, false)->GetAudioSamples(channel, !is_increasing) + location.sample_start, frame_sample_count, 1.0f);
+				for (int channel = 0; channel < source_frame->GetAudioChannelsCount(); channel++) {
+					source_samples->addFrom(channel, source_pos, source_frame->GetAudioSamples(channel) + location.sample_start, frame_sample_count, 1.0f);
 				}
 				if (is_increasing) {
 					location.frame++;
@@ -608,10 +604,10 @@ void Clip::apply_timemapping(std::shared_ptr<Frame> frame)
 				remaining_samples -= frame_sample_count;
 				source_pos += frame_sample_count;
 
-			} else if (remaining_samples - frame_sample_count < 0) {
+			} else {
 				// Use just what is needed (and reverse samples)
-				for (int channel = 0; channel < Reader()->info.channels; channel++) {
-					source_samples->addFrom(channel, source_pos, GetOrCreateFrame(location.frame, false)->GetAudioSamples(channel, !is_increasing) + location.sample_start, remaining_samples, 1.0f);
+				for (int channel = 0; channel < source_frame->GetAudioChannelsCount(); channel++) {
+					source_samples->addFrom(channel, source_pos, source_frame->GetAudioSamples(channel) + location.sample_start, remaining_samples, 1.0f);
 				}
 				location.sample_start += remaining_samples;
 				remaining_samples = 0;
@@ -620,31 +616,33 @@ void Clip::apply_timemapping(std::shared_ptr<Frame> frame)
 
 		}
 
-		// Resample audio (if needed)
+		// Resize audio for current frame object + fill with silence
+		// We are fixing to clobber this with actual audio data (possibly resampled)
+		frame->AddAudioSilence(target_sample_count);
+
 		if (source_samples->getNumSamples() != target_sample_count) {
+			// Resample audio (if needed)
 			resampler->SetBuffer(source_samples, fabs(delta));
 
 			// Resample the data
 			juce::AudioBuffer<float> *resampled_buffer = resampler->GetResampledBuffer();
 
 			// Fill the frame with resampled data
-			frame->ResizeAudio(Reader()->info.channels, target_sample_count, Reader()->info.sample_rate, Reader()->info.channel_layout);
 			for (int channel = 0; channel < Reader()->info.channels; channel++) {
 				// Add new (slower) samples, to the frame object
 				frame->AddAudio(true, channel, 0, resampled_buffer->getReadPointer(channel, 0), target_sample_count, 1.0f);
 			}
-
-			// Clean up
-			resampled_buffer = nullptr;
 		} else {
 			// Fill the frame
-			frame->ResizeAudio(Reader()->info.channels, target_sample_count, Reader()->info.sample_rate, Reader()->info.channel_layout);
 			for (int channel = 0; channel < Reader()->info.channels; channel++) {
 				// Add new (slower) samples, to the frame object
 				frame->AddAudio(true, channel, 0, source_samples->getReadPointer(channel, 0), target_sample_count, 1.0f);
 			}
 		}
 
+		// Clean up
+		delete source_samples;
+
 		// Set previous location
 		previous_location = location;
 	}

src/Frame.cpp

@@ -372,13 +372,18 @@ float* Frame::GetPlanarAudioSamples(int new_sample_rate, AudioResampler* resampl
 
 
 // Get an array of sample data (all channels interleaved together), using any sample rate
-float* Frame::GetInterleavedAudioSamples(int new_sample_rate, AudioResampler* resampler, int* sample_count)
+float* Frame::GetInterleavedAudioSamples(int new_sample_rate, AudioResampler* resampler, int* sample_count, bool reverse)
 {
 	float *output = NULL;
 	juce::AudioBuffer<float> *buffer(audio.get());
 	int num_of_channels = audio->getNumChannels();
 	int num_of_samples = GetAudioSamplesCount();
 
+	if (reverse) {
+		// Reverse audio samples (if needed)
+		buffer->reverse(0, buffer->getNumSamples());
+	}
+
 	// Resample to new sample rate (if needed)
 	if (new_sample_rate != sample_rate && resampler)
 	{

src/Frame.h

@@ -190,7 +190,7 @@ namespace openshot
 		float* GetAudioSamples(int channel, bool reverse=false);
 
 		/// Get an array of sample data (all channels interleaved together), using any sample rate
-		float* GetInterleavedAudioSamples(int new_sample_rate, openshot::AudioResampler* resampler, int* sample_count);
+		float* GetInterleavedAudioSamples(int new_sample_rate, openshot::AudioResampler* resampler, int* sample_count, bool reverse=false);
 
 		// Get a planar array of sample data, using any sample rate
 		float* GetPlanarAudioSamples(int new_sample_rate, openshot::AudioResampler* resampler, int* sample_count);

src/FrameMapper.cpp

@@ -831,6 +831,14 @@ void FrameMapper::ResampleMappedAudio(std::shared_ptr<Frame> frame, int64_t orig
 		// Recalculate mappings
 		Init();
 
+	// Determine direction of parent clip at this frame (forward or reverse direction)
+	// This is important for reversing audio in our resampler, for smooth reversed audio.
+	Clip *parent = (Clip *) ParentClip();
+	bool is_increasing = true;
+	if (parent) {
+		is_increasing = parent->time.IsIncreasing(original_frame_number);
+	}
+
 	// Init audio buffers / variables
 	int total_frame_samples = 0;
 	int channels_in_frame = frame->GetAudioChannelsCount();
@@ -849,7 +857,7 @@ void FrameMapper::ResampleMappedAudio(std::shared_ptr<Frame> frame, int64_t orig
 	// Get audio sample array
 	float* frame_samples_float = NULL;
 	// Get samples interleaved together (c1 c2 c1 c2 c1 c2)
-	frame_samples_float = frame->GetInterleavedAudioSamples(sample_rate_in_frame, NULL, &samples_in_frame);
+	frame_samples_float = frame->GetInterleavedAudioSamples(sample_rate_in_frame, NULL, &samples_in_frame, !is_increasing);
 
 	// Calculate total samples
 	total_frame_samples = samples_in_frame * channels_in_frame;
@@ -875,21 +883,10 @@ void FrameMapper::ResampleMappedAudio(std::shared_ptr<Frame> frame, int64_t orig
 		frame_samples[s] = conv;
 	}
 
-
 	// Deallocate float array
 	delete[] frame_samples_float;
 	frame_samples_float = NULL;
 
-	ZmqLogger::Instance()->AppendDebugMethod(
-		"FrameMapper::ResampleMappedAudio (got sample data from frame)",
-		"frame->number", frame->number,
-		"total_frame_samples", total_frame_samples,
-		"target channels", info.channels,
-		"channels_in_frame", channels_in_frame,
-		"target sample_rate", info.sample_rate,
-		"samples_in_frame", samples_in_frame);
-
-
 	// Create input frame (and allocate arrays)
 	AVFrame *audio_frame = AV_ALLOCATE_FRAME();
 	AV_RESET_FRAME(audio_frame);
@@ -911,30 +908,12 @@ 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), 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);
-
 	// Create output frame (and allocate arrays)
 	AVFrame *audio_converted = AV_ALLOCATE_FRAME();
 	AV_RESET_FRAME(audio_converted);
 	audio_converted->nb_samples = total_frame_samples;
 	av_samples_alloc(audio_converted->data, audio_converted->linesize, info.channels, total_frame_samples, AV_SAMPLE_FMT_S16, 0);
 
-	ZmqLogger::Instance()->AppendDebugMethod(
-		"FrameMapper::ResampleMappedAudio (preparing for resample)",
-		"in_sample_fmt", AV_SAMPLE_FMT_S16,
-		"out_sample_fmt", AV_SAMPLE_FMT_S16,
-		"in_sample_rate", sample_rate_in_frame,
-		"out_sample_rate", info.sample_rate,
-		"in_channels", channels_in_frame,
-		"out_channels", info.channels);
-
 	int nb_samples = 0;
 
 	// setup resample context
@@ -1023,11 +1002,6 @@ void FrameMapper::ResampleMappedAudio(std::shared_ptr<Frame> frame, int64_t orig
 
 		// Add samples to frame for this channel
 		frame->AddAudio(true, channel_filter, 0, channel_buffer, position, 1.0f);
-
-		ZmqLogger::Instance()->AppendDebugMethod(
-			"FrameMapper::ResampleMappedAudio (Add audio to channel)",
-			"number of samples", position,
-			"channel_filter", channel_filter);
 	}
 
 	// Update frame's audio meta data

tests/Clip.cpp

@@ -23,6 +23,7 @@
 #include "DummyReader.h"
 #include "Enums.h"
 #include "Exceptions.h"
+#include "FFmpegReader.h"
 #include "Frame.h"
 #include "Fraction.h"
 #include "FrameMapper.h"
@@ -469,8 +470,16 @@ TEST_CASE( "resample_audio_48000_to_41000_reverse", "[libopenshot][clip]" )
 	// Create a reader
 	std::stringstream path;
 	path << TEST_MEDIA_PATH << "sintel_trailer-720p.mp4";
-	Clip clip(path.str());
-	int original_video_length = clip.Reader()->info.video_length;
+	openshot::FFmpegReader reader(path.str(), true);
+
+	// Map to 24 fps, 2 channels stereo, 44100 sample rate
+	FrameMapper map(&reader, Fraction(24,1), PULLDOWN_NONE, 44100, 2, LAYOUT_STEREO);
+	map.Open();
+
+	Clip clip;
+	clip.Reader(&map);
+	clip.Open();
+	int original_video_length = clip.Reader()->info.video_length + 1;
 
 	clip.Position(0.0);
 	clip.Start(0.0);
@@ -479,18 +488,14 @@ TEST_CASE( "resample_audio_48000_to_41000_reverse", "[libopenshot][clip]" )
 	clip.time.AddPoint(1, original_video_length, openshot::LINEAR);
 	clip.time.AddPoint(original_video_length, 1.0, openshot::LINEAR);
 
-	// Map to 24 fps, 2 channels stereo, 44100 sample rate
-	FrameMapper map(&clip, Fraction(24,1), PULLDOWN_NONE, 44100, 2, LAYOUT_STEREO);
-	map.Open();
-
 	// Loop again through frames
 	// Time-remapping should start over (detect a gap)
 	for (int64_t frame = 1; frame < 100; frame++) {
 		int expected_sample_count = Frame::GetSamplesPerFrame(frame, map.info.fps,
 															  map.info.sample_rate,
 															  map.info.channels);
 
-		std::shared_ptr<Frame> f = map.GetFrame(frame);
+		std::shared_ptr<Frame> f = clip.GetFrame(frame);
 		if (expected_sample_count != f->GetAudioSamplesCount()) {
 			CHECK(expected_sample_count == f->GetAudioSamplesCount());
 		}
@@ -506,12 +511,14 @@ TEST_CASE( "resample_audio_48000_to_41000_reverse", "[libopenshot][clip]" )
 															  map.info.sample_rate,
 															  map.info.channels);
 
-		std::shared_ptr<Frame> f = map.GetFrame(frame);
+		std::shared_ptr<Frame> f = clip.GetFrame(frame);
 		if (expected_sample_count != f->GetAudioSamplesCount()) {
 			CHECK(expected_sample_count == f->GetAudioSamplesCount());
 		}
 	}
 
 	// Close mapper
 	map.Close();
+	reader.Close();
+	clip.Close();
 }