From a4109419ac22161b553cb7c6ce4bf6596123efde Mon Sep 17 00:00:00 2001 From: Brenno Date: Sat, 10 Oct 2020 17:01:24 -0300 Subject: [PATCH] Implemented position remapper inside FrameMapper to fix audio noise when exporting to different fps The FrameMapper class now receives the updated clip position and returns the correct amount of samples for a given frame number --- include/FrameMapper.h | 8 +- src/FrameMapper.cpp | 39 +++++-- src/Timeline.cpp | 10 +- tests/FrameMapper_Tests.cpp | 197 ++++++++++++++++++++++++++++++++++-- 4 files changed, 229 insertions(+), 25 deletions(-) diff --git a/include/FrameMapper.h b/include/FrameMapper.h index e78401a9b..85c933d25 100644 --- a/include/FrameMapper.h +++ b/include/FrameMapper.h @@ -147,6 +147,9 @@ namespace openshot bool is_dirty; // When this is true, the next call to GetFrame will re-init the mapping SWRCONTEXT *avr; // Audio resampling context object + float position; + float start; + // Internal methods used by init void AddField(int64_t frame); void AddField(Field field); @@ -166,13 +169,13 @@ namespace openshot std::vector frames; // List of all frames /// Default constructor for openshot::FrameMapper class - FrameMapper(ReaderBase *reader, Fraction target_fps, PulldownType target_pulldown, int target_sample_rate, int target_channels, ChannelLayout target_channel_layout); + FrameMapper(ReaderBase *reader, Fraction target_fps, PulldownType target_pulldown, int target_sample_rate, int target_channels, ChannelLayout target_channel_layout, float clipPosition, float clipStart); /// Destructor virtual ~FrameMapper(); /// Change frame rate or audio mapping details - void ChangeMapping(Fraction target_fps, PulldownType pulldown, int target_sample_rate, int target_channels, ChannelLayout target_channel_layout); + void ChangeMapping(Fraction target_fps, PulldownType pulldown, int target_sample_rate, int target_channels, ChannelLayout target_channel_layout, float clipPosition, float clipStart); /// Close the openshot::FrameMapper and internal reader void Close() override; @@ -218,6 +221,7 @@ namespace openshot /// Resample audio and map channels (if needed) void ResampleMappedAudio(std::shared_ptr frame, int64_t original_frame_number); + int64_t ConvPositon(int64_t clip_frame_number); }; } diff --git a/src/FrameMapper.cpp b/src/FrameMapper.cpp index 8eff6e702..977302915 100644 --- a/src/FrameMapper.cpp +++ b/src/FrameMapper.cpp @@ -33,7 +33,7 @@ using namespace std; using namespace openshot; -FrameMapper::FrameMapper(ReaderBase *reader, Fraction target, PulldownType target_pulldown, int target_sample_rate, int target_channels, ChannelLayout target_channel_layout) : +FrameMapper::FrameMapper(ReaderBase *reader, Fraction target, PulldownType target_pulldown, int target_sample_rate, int target_channels, ChannelLayout target_channel_layout, float clipPosition, float clipStart) : reader(reader), target(target), pulldown(target_pulldown), is_dirty(true), avr(NULL) { // Set the original frame rate from the reader @@ -52,6 +52,8 @@ FrameMapper::FrameMapper(ReaderBase *reader, Fraction target, PulldownType targe info.width = reader->info.width; info.height = reader->info.height; + position = clipPosition; + start = clipStart; // Used to toggle odd / even fields field_toggle = true; @@ -257,12 +259,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(frame_number, target, reader->info.sample_rate, reader->info.channels); + int remaining_samples = Frame::GetSamplesPerFrame(ConvPositon(frame_number), target, reader->info.sample_rate, reader->info.channels); while (remaining_samples > 0) { // get original samples - int original_samples = Frame::GetSamplesPerFrame(end_samples_frame, original, reader->info.sample_rate, reader->info.channels) - end_samples_position; + int original_samples = Frame::GetSamplesPerFrame(ConvPositon(end_samples_frame), original, reader->info.sample_rate, reader->info.channels) - end_samples_position; // Enough samples if (original_samples >= remaining_samples) @@ -282,12 +284,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(frame_number, target, reader->info.sample_rate, reader->info.channels)}; + SampleRange Samples = {start_samples_frame, start_samples_position, end_samples_frame, end_samples_position, Frame::GetSamplesPerFrame(ConvPositon(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(start_samples_frame, original, reader->info.sample_rate, reader->info.channels)) + if (start_samples_position >= Frame::GetSamplesPerFrame(ConvPositon(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 @@ -354,7 +356,7 @@ std::shared_ptr FrameMapper::GetOrCreateFrame(int64_t number) std::shared_ptr new_frame; // 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(number, target, reader->info.sample_rate, reader->info.channels); + int samples_in_frame = Frame::GetSamplesPerFrame(ConvPositon(number), target, reader->info.sample_rate, reader->info.channels); try { // Debug output @@ -427,7 +429,7 @@ std::shared_ptr 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(frame_number, target, mapped_frame->SampleRate(), channels_in_frame); + int samples_in_frame = Frame::GetSamplesPerFrame(ConvPositon(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, @@ -689,6 +691,7 @@ Json::Value FrameMapper::JsonValue() const { // Create root json object Json::Value root = ReaderBase::JsonValue(); // get parent properties root["type"] = "FrameMapper"; + root["position"] = position; // return JsonValue return root; @@ -717,6 +720,10 @@ void FrameMapper::SetJsonValue(const Json::Value root) { // Set parent data ReaderBase::SetJsonValue(root); + if(!root["position"].isNull()){ + position = root["position"].asDouble(); + } + // Re-Open path, and re-init everything (if needed) if (reader) { @@ -726,7 +733,7 @@ void FrameMapper::SetJsonValue(const Json::Value root) { } // Change frame rate or audio mapping details -void FrameMapper::ChangeMapping(Fraction target_fps, PulldownType target_pulldown, int target_sample_rate, int target_channels, ChannelLayout target_channel_layout) +void FrameMapper::ChangeMapping(Fraction target_fps, PulldownType target_pulldown, int target_sample_rate, int target_channels, ChannelLayout target_channel_layout, float clipPosition, float clipStart) { ZmqLogger::Instance()->AppendDebugMethod("FrameMapper::ChangeMapping", "target_fps.num", target_fps.num, "target_fps.den", target_fps.den, "target_pulldown", target_pulldown, "target_sample_rate", target_sample_rate, "target_channels", target_channels, "target_channel_layout", target_channel_layout); @@ -745,6 +752,9 @@ void FrameMapper::ChangeMapping(Fraction target_fps, PulldownType target_pulldow info.channels = target_channels; info.channel_layout = target_channel_layout; + position = clipPosition; + start = clipStart; + // Clear cache final_cache.Clear(); @@ -828,7 +838,7 @@ void FrameMapper::ResampleMappedAudio(std::shared_ptr frame, int64_t orig } // Update total samples & input frame size (due to bigger or smaller data types) - total_frame_samples = Frame::GetSamplesPerFrame(frame->number, target, info.sample_rate, info.channels); + total_frame_samples = Frame::GetSamplesPerFrame(ConvPositon(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); @@ -937,3 +947,14 @@ void FrameMapper::ResampleMappedAudio(std::shared_ptr frame, int64_t orig delete[] resampled_samples; resampled_samples = NULL; } + +int64_t FrameMapper::ConvPositon(int64_t clip_frame_number){ + + 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; +} \ No newline at end of file diff --git a/src/Timeline.cpp b/src/Timeline.cpp index 86fc55ff9..b635b4995 100644 --- a/src/Timeline.cpp +++ b/src/Timeline.cpp @@ -360,14 +360,14 @@ void Timeline::apply_mapper_to_clip(Clip* clip) } else { // Create a new FrameMapper to wrap the current reader - FrameMapper* mapper = new FrameMapper(clip->Reader(), info.fps, PULLDOWN_NONE, info.sample_rate, info.channels, info.channel_layout); + FrameMapper* mapper = new FrameMapper(clip->Reader(), info.fps, PULLDOWN_NONE, info.sample_rate, info.channels, info.channel_layout, clip->Position(), clip->Start()); allocated_frame_mappers.insert(mapper); clip_reader = (ReaderBase*) mapper; } // Update the mapping FrameMapper* clip_mapped_reader = (FrameMapper*) clip_reader; - clip_mapped_reader->ChangeMapping(info.fps, PULLDOWN_NONE, info.sample_rate, info.channels, info.channel_layout); + clip_mapped_reader->ChangeMapping(info.fps, PULLDOWN_NONE, info.sample_rate, info.channels, info.channel_layout, clip->Position(), clip->Start()); // Update clip reader clip->Reader(clip_reader); @@ -546,11 +546,12 @@ void Timeline::add_layer(std::shared_ptr new_frame, Clip* source_clip, in // Currently, the ResampleContext sometimes leaves behind a few samples for the next call, and the // number of samples returned is variable... and does not match the number expected. // This is a crude solution at best. =) - if (new_frame->GetAudioSamplesCount() != source_frame->GetAudioSamplesCount()) + if (new_frame->GetAudioSamplesCount() != source_frame->GetAudioSamplesCount()){ // Force timeline frame to match the source frame #pragma omp critical (T_addLayer) - new_frame->ResizeAudio(info.channels, source_frame->GetAudioSamplesCount(), info.sample_rate, info.channel_layout); + new_frame->ResizeAudio(info.channels, source_frame->GetAudioSamplesCount(), info.sample_rate, info.channel_layout); + } // Copy audio samples (and set initial volume). Mix samples with existing audio samples. The gains are added together, to // be sure to set the gain's correctly, so the sum does not exceed 1.0 (of audio distortion will happen). #pragma omp critical (T_addLayer) @@ -682,6 +683,7 @@ bool Timeline::isEqual(double a, double b) // Get an openshot::Frame object for a specific frame number of this reader. std::shared_ptr Timeline::GetFrame(int64_t requested_frame) { + // Adjust out of bounds frame number if (requested_frame < 1) requested_frame = 1; diff --git a/tests/FrameMapper_Tests.cpp b/tests/FrameMapper_Tests.cpp index 921f3a155..02634c373 100644 --- a/tests/FrameMapper_Tests.cpp +++ b/tests/FrameMapper_Tests.cpp @@ -42,7 +42,7 @@ TEST(FrameMapper_Get_Valid_Frame) DummyReader r(Fraction(24,1), 720, 480, 22000, 2, 5.0); // Create mapping between 24 fps and 29.97 fps using classic pulldown - FrameMapper mapping(&r, Fraction(30000, 1001), PULLDOWN_CLASSIC, 22000, 2, LAYOUT_STEREO); + FrameMapper mapping(&r, Fraction(30000, 1001), PULLDOWN_CLASSIC, 22000, 2, LAYOUT_STEREO, 0.0, 0.0); try { @@ -63,7 +63,7 @@ TEST(FrameMapper_Invalid_Frame_Too_Small) DummyReader r(Fraction(24,1), 720, 480, 22000, 2, 5.0); // Create mapping 24 fps and 29.97 fps - FrameMapper mapping(&r, Fraction(30000, 1001), PULLDOWN_CLASSIC, 22000, 2, LAYOUT_STEREO); + FrameMapper mapping(&r, Fraction(30000, 1001), PULLDOWN_CLASSIC, 22000, 2, LAYOUT_STEREO, 0.0, 0.0); // Check invalid frame number CHECK_THROW(mapping.GetMappedFrame(0), OutOfBoundsFrame); @@ -76,7 +76,7 @@ TEST(FrameMapper_24_fps_to_30_fps_Pulldown_Classic) DummyReader r(Fraction(24,1), 720, 480, 22000, 2, 5.0); // Create mapping 24 fps and 30 fps - FrameMapper mapping(&r, Fraction(30, 1), PULLDOWN_CLASSIC, 22000, 2, LAYOUT_STEREO); + FrameMapper mapping(&r, Fraction(30, 1), PULLDOWN_CLASSIC, 22000, 2, LAYOUT_STEREO, 0.0, 0.0); MappedFrame frame2 = mapping.GetMappedFrame(2); MappedFrame frame3 = mapping.GetMappedFrame(3); @@ -93,7 +93,7 @@ TEST(FrameMapper_24_fps_to_30_fps_Pulldown_Advanced) DummyReader r(Fraction(24,1), 720, 480, 22000, 2, 5.0); // Create mapping 24 fps and 30 fps - FrameMapper mapping(&r, Fraction(30, 1), PULLDOWN_ADVANCED, 22000, 2, LAYOUT_STEREO); + FrameMapper mapping(&r, Fraction(30, 1), PULLDOWN_ADVANCED, 22000, 2, LAYOUT_STEREO, 0.0, 0.0); MappedFrame frame2 = mapping.GetMappedFrame(2); MappedFrame frame3 = mapping.GetMappedFrame(3); MappedFrame frame4 = mapping.GetMappedFrame(4); @@ -113,7 +113,7 @@ TEST(FrameMapper_24_fps_to_30_fps_Pulldown_None) DummyReader r(Fraction(24,1), 720, 480, 22000, 2, 5.0); // Create mapping 24 fps and 30 fps - FrameMapper mapping(&r, Fraction(30, 1), PULLDOWN_NONE, 22000, 2, LAYOUT_STEREO); + FrameMapper mapping(&r, Fraction(30, 1), PULLDOWN_NONE, 22000, 2, LAYOUT_STEREO, 0.0, 0.0); MappedFrame frame4 = mapping.GetMappedFrame(4); MappedFrame frame5 = mapping.GetMappedFrame(5); @@ -130,7 +130,7 @@ TEST(FrameMapper_30_fps_to_24_fps_Pulldown_Classic) DummyReader r(Fraction(30, 1), 720, 480, 22000, 2, 5.0); // Create mapping between 30 fps and 24 fps - FrameMapper mapping(&r, Fraction(24, 1), PULLDOWN_CLASSIC, 22000, 2, LAYOUT_STEREO); + FrameMapper mapping(&r, Fraction(24, 1), PULLDOWN_CLASSIC, 22000, 2, LAYOUT_STEREO, 0.0, 0.0); MappedFrame frame3 = mapping.GetMappedFrame(3); MappedFrame frame4 = mapping.GetMappedFrame(4); MappedFrame frame5 = mapping.GetMappedFrame(5); @@ -150,7 +150,7 @@ TEST(FrameMapper_30_fps_to_24_fps_Pulldown_Advanced) DummyReader r(Fraction(30, 1), 720, 480, 22000, 2, 5.0); // Create mapping between 30 fps and 24 fps - FrameMapper mapping(&r, Fraction(24, 1), PULLDOWN_ADVANCED, 22000, 2, LAYOUT_STEREO); + FrameMapper mapping(&r, Fraction(24, 1), PULLDOWN_ADVANCED, 22000, 2, LAYOUT_STEREO, 0.0, 0.0); MappedFrame frame2 = mapping.GetMappedFrame(2); MappedFrame frame3 = mapping.GetMappedFrame(3); MappedFrame frame4 = mapping.GetMappedFrame(4); @@ -170,7 +170,7 @@ TEST(FrameMapper_30_fps_to_24_fps_Pulldown_None) DummyReader r(Fraction(30, 1), 720, 480, 22000, 2, 5.0); // Create mapping between 30 fps and 24 fps - FrameMapper mapping(&r, Fraction(24, 1), PULLDOWN_NONE, 22000, 2, LAYOUT_STEREO); + FrameMapper mapping(&r, Fraction(24, 1), PULLDOWN_NONE, 22000, 2, LAYOUT_STEREO, 0.0, 0.0); MappedFrame frame4 = mapping.GetMappedFrame(4); MappedFrame frame5 = mapping.GetMappedFrame(5); @@ -189,7 +189,7 @@ TEST(FrameMapper_resample_audio_48000_to_41000) FFmpegReader r(path.str()); // Map to 30 fps, 3 channels surround, 44100 sample rate - FrameMapper map(&r, Fraction(30,1), PULLDOWN_NONE, 44100, 3, LAYOUT_SURROUND); + FrameMapper map(&r, Fraction(30,1), PULLDOWN_NONE, 44100, 3, LAYOUT_SURROUND, 0.0, 0.0); map.Open(); // Check details @@ -199,7 +199,7 @@ TEST(FrameMapper_resample_audio_48000_to_41000) CHECK_EQUAL(1470, map.GetFrame(50)->GetAudioSamplesCount()); // Change mapping data - map.ChangeMapping(Fraction(25,1), PULLDOWN_NONE, 22050, 1, LAYOUT_MONO); + map.ChangeMapping(Fraction(25,1), PULLDOWN_NONE, 22050, 1, LAYOUT_MONO, 0.0, 0.0); // Check details CHECK_EQUAL(1, map.GetFrame(1)->GetAudioChannelsCount()); @@ -210,3 +210,180 @@ TEST(FrameMapper_resample_audio_48000_to_41000) // Close mapper map.Close(); } + +TEST(FrameMapper_AudioSample_Distribution) +{ + + CacheMemory cache; + int OFFSET = 0; + float AMPLITUDE = 0.2; + double ANGLE = 0.0; + int NUM_SAMPLES = 100; + std::cout << "Starting Resample Test" << std::endl; + + for (int64_t frame_number = 1; frame_number <= 90; 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; + std::shared_ptr f(new openshot::Frame(frame_number, sample_count, 2)); + + // Create test samples with sin wave (predictable values) + float *audio_buffer = new float[sample_count * 2]; + + for (int sample_number = 0; sample_number < sample_count; sample_number++) + { + // Calculate sin wave + // TODO: I'm using abs(), because calling AddAudio only seems to be adding the positive values and it's bizarre + float sample_value = float(AMPLITUDE * sin(ANGLE) + OFFSET); + audio_buffer[sample_number] = sample_value;//abs(sample_value); + ANGLE += (2 * M_PI) / NUM_SAMPLES; + + // 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); + } + } + // Create a default fraction (should be 1/1) + openshot::DummyReader r(openshot::Fraction(30, 1), 1920, 1080, 44100, 2, 30.0, &cache); + r.info.has_audio = true; + r.Open(); // Open the reader + + // Map to 24 fps, which should create a variable # of samples per frame + ///FrameMapper map(&r, Fraction(24, 1), PULLDOWN_NONE, 44100, 2, LAYOUT_STEREO); + //map.info.has_audio = true; + //map.Open(); + + Timeline t1(1920, 1080, Fraction(24, 1), 44100, 2, LAYOUT_STEREO); + + Clip c1; + c1.Reader(&r); + c1.Layer(1); + c1.Position(0.0); + c1.Start(0.0); + c1.End(10.0); + + // Create 2nd map to 24 fps, which should create a variable # of samples per frame + + //FrameMapper map2(&r, Fraction(24, 1), PULLDOWN_NONE, 44100, 2, LAYOUT_STEREO); + + //map2.info.has_audio = true; + //map2.Open(); + + Clip c2; + c2.Reader(&r); + c2.Layer(2); + + // Position 1 frame into the video, this should mis-align the audio and create situations + // which overlapping Frame instances have different # of samples for the Timeline. + // TODO: Moving to 0.0 position, to simplify this test for now + + + c2.Position(0.041666667 * 14); + c2.Start(1.0); + c2.End(10.0); + + // Add clips + + t1.AddClip(&c1); + t1.AddClip(&c2); + + std::string json_val = t1.Json(); + + std::cout << json_val << std::endl; + + //t1.SetJson(t1.Json()); + t1.Open(); + + FFmpegWriter w("output-resample.mp4"); + + // Set options + w.SetAudioOptions("aac", 44100, 192000); + w.SetVideoOptions("libx264", 1280, 720, Fraction(24,1), 5000000); + + // Open writer + w.Open(); + + + w.WriteFrame(&t1, 5, 50); + + //for (int64_t frame_number = 1; frame_number <= 90; frame_number++){ + // w.WriteFrame(t1.GetFrame(frame_number)); + //} + + // Close writer & reader + w.Close(); + + //map.Close(); + //map2.Close(); + + t1.Close(); + + // Clean up + cache.Clear(); + + r.Close(); + +} + + +/* +TEST(FrameMapperVideoEdition){ + + stringstream path; + path << TEST_MEDIA_PATH << "baseline.mkv"; + FFmpegReader r(path.str()); + r.Open(); + + Clip c1; + c1.Reader(&r); + c1.Layer(1); + c1.Position(0.0); + c1.Start(0.0); + c1.End(45.0); + + Clip c2; + c2.Reader(&r); + c2.Layer(1); + c2.Position(30.0); + c2.Start(0.0); + c2.End(45.0); + + Timeline t1(1280, 720, Fraction(24, 1), 44100, 2, LAYOUT_STEREO); + t1.AddClip(&c1); + t1.AddClip(&c2); + + t1.Open(); + + + FFmpegWriter w("simple-edit.mp4"); + + // Set options + w.SetAudioOptions("aac", 44100, 192000); + w.SetVideoOptions("libx264", 1280, 720, Fraction(24,1), 5000000); + + // Open writer + w.Open(); + + + w.WriteFrame(&t1, 1, t1.GetMaxFrame()); + + // Close writer & reader + w.Close(); + + //map.Close(); + //map2.Close(); + + t1.Close(); + + + r.Close(); + + + +}*/ \ No newline at end of file