From c30dbb90d84efd521bbd7e0772e8c10d7be2b1cb Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Wed, 19 Jul 2017 16:05:07 -0500 Subject: [PATCH] Adding additional locks when adding/changing audio data. Reducing FrameMapper to a single frame at a time (increase seek speed and decrease crashes). Fixing crash on Time keyframes where it would sometimes calculate an invalid frame number. --- include/Frame.h | 1 + src/Clip.cpp | 6 ++--- src/Frame.cpp | 63 ++++++++++++++++++++++++++------------------- src/FrameMapper.cpp | 11 ++++---- 4 files changed, 47 insertions(+), 34 deletions(-) diff --git a/include/Frame.h b/include/Frame.h index df050cb5..cc68492d 100644 --- a/include/Frame.h +++ b/include/Frame.h @@ -120,6 +120,7 @@ namespace openshot tr1::shared_ptr audio; tr1::shared_ptr previewApp; CriticalSection addingImageSection; + CriticalSection addingAudioSection; const unsigned char *qbuffer; Fraction pixel_ratio; int channels; diff --git a/src/Clip.cpp b/src/Clip.cpp index e139612b..5e0ff740 100644 --- a/src/Clip.cpp +++ b/src/Clip.cpp @@ -57,7 +57,7 @@ void Clip::init_settings() rotation = Keyframe(0.0); // Init time & volume - time = Keyframe(0.0); + time = Keyframe(1.0); volume = Keyframe(1.0); // Init audio waveform color @@ -277,7 +277,7 @@ tr1::shared_ptr Clip::GetFrame(long int requested_frame) throw(ReaderClos // Is a time map detected long int new_frame_number = requested_frame; if (time.Values.size() > 1) - new_frame_number = time.GetLong(requested_frame); + new_frame_number = adjust_frame_number_minimum(time.GetLong(requested_frame)); // Now that we have re-mapped what frame number is needed, go and get the frame pointer tr1::shared_ptr original_frame = GetOrCreateFrame(new_frame_number); @@ -365,7 +365,7 @@ tr1::shared_ptr Clip::get_time_mapped_frame(tr1::shared_ptr frame, resampler = new AudioResampler(); // Get new frame number - int new_frame_number = round(time.GetValue(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()); diff --git a/src/Frame.cpp b/src/Frame.cpp index 3096f25d..9fca6615 100644 --- a/src/Frame.cpp +++ b/src/Frame.cpp @@ -410,19 +410,21 @@ float* Frame::GetInterleavedAudioSamples(int new_sample_rate, AudioResampler* re // Get number of audio channels int Frame::GetAudioChannelsCount() { - int i; - #pragma omp critical - i = audio->getNumChannels(); - return i; + const GenericScopedLock lock(addingAudioSection); + if (audio) + return audio->getNumChannels(); + else + return 0; } // Get number of audio samples int Frame::GetAudioSamplesCount() { - int i; - #pragma omp critical - i = audio->getNumSamples(); - return i; + const GenericScopedLock lock(addingAudioSection); + if (audio) + return audio->getNumSamples(); + else + return 0; } juce::AudioSampleBuffer *Frame::GetAudioSampleBuffer() @@ -779,36 +781,43 @@ void Frame::AddImage(tr1::shared_ptr new_image, bool only_odd_lines) // Resize audio container to hold more (or less) samples and channels void Frame::ResizeAudio(int channels, int length, int rate, ChannelLayout layout) { - // Resize JUCE audio buffer + const GenericScopedLock lock(addingAudioSection); + + // Resize JUCE audio buffer audio->setSize(channels, length, true, true, false); channel_layout = layout; sample_rate = rate; } // Add audio samples to a specific channel -void Frame::AddAudio(bool replaceSamples, int destChannel, int destStartSample, const float* source, int numSamples, float gainToApplyToSource = 1.0f) -{ - // Extend audio container to hold more (or less) samples and channels.. if needed - int new_length = destStartSample + numSamples; - int new_channel_length = audio->getNumChannels(); - if (destChannel >= new_channel_length) - new_channel_length = destChannel + 1; - if (new_length > audio->getNumSamples() || new_channel_length > audio->getNumChannels()) - audio->setSize(new_channel_length, new_length, true, true, false); +void Frame::AddAudio(bool replaceSamples, int destChannel, int destStartSample, const float* source, int numSamples, float gainToApplyToSource = 1.0f) { + const GenericScopedLock lock(addingAudioSection); + #pragma omp critical (adding_audio) + { + // Extend audio container to hold more (or less) samples and channels.. if needed + int new_length = destStartSample + numSamples; + int new_channel_length = audio->getNumChannels(); + if (destChannel >= new_channel_length) + new_channel_length = destChannel + 1; + if (new_length > audio->getNumSamples() || new_channel_length > audio->getNumChannels()) + audio->setSize(new_channel_length, new_length, true, true, false); - // Clear the range of samples first (if needed) - if (replaceSamples) - audio->clear(destChannel, destStartSample, numSamples); + // Clear the range of samples first (if needed) + if (replaceSamples) + audio->clear(destChannel, destStartSample, numSamples); - // Add samples to frame's audio buffer - audio->addFrom(destChannel, destStartSample, source, numSamples, gainToApplyToSource); - has_audio_data = true; + // Add samples to frame's audio buffer + audio->addFrom(destChannel, destStartSample, source, numSamples, gainToApplyToSource); + has_audio_data = true; + } } // Apply gain ramp (i.e. fading volume) void Frame::ApplyGainRamp(int destChannel, int destStartSample, int numSamples, float initial_gain = 0.0f, float final_gain = 1.0f) { - // Apply gain ramp + const GenericScopedLock lock(addingAudioSection); + + // Apply gain ramp audio->applyGainRamp(destChannel, destStartSample, numSamples, initial_gain, final_gain); } @@ -962,7 +971,9 @@ void Frame::cleanUpBuffer(void *info) // Add audio silence void Frame::AddAudioSilence(int numSamples) { - // Resize audio container + const GenericScopedLock lock(addingAudioSection); + + // Resize audio container audio->setSize(channels, numSamples, false, true, false); audio->clear(); has_audio_data = true; diff --git a/src/FrameMapper.cpp b/src/FrameMapper.cpp index f10b3597..1ce6946b 100644 --- a/src/FrameMapper.cpp +++ b/src/FrameMapper.cpp @@ -399,7 +399,8 @@ tr1::shared_ptr FrameMapper::GetFrame(long int requested_frame) throw(Rea if (final_frame) return final_frame; // Minimum number of frames to process (for performance reasons) - int minimum_frames = OPEN_MP_NUM_PROCESSORS; + // Dialing this down to 1 for now, as it seems to improve performance, and reduce export crashes + int minimum_frames = 1; // Debug output ZmqLogger::Instance()->AppendDebugMethod("FrameMapper::GetFrame (Loop through frames)", "requested_frame", requested_frame, "minimum_frames", minimum_frames, "", -1, "", -1, "", -1, "", -1); @@ -522,13 +523,13 @@ tr1::shared_ptr FrameMapper::GetFrame(long int requested_frame) throw(Rea int remaining_samples = copy_samples.total - samples_copied; int number_to_copy = 0; + // number of original samples on this frame + tr1::shared_ptr original_frame = GetOrCreateFrame(starting_frame); + int original_samples = original_frame->GetAudioSamplesCount(); + // Loop through each channel for (int channel = 0; channel < channels_in_frame; channel++) { - // number of original samples on this frame - tr1::shared_ptr original_frame = GetOrCreateFrame(starting_frame); - int original_samples = original_frame->GetAudioSamplesCount(); - if (starting_frame == copy_samples.frame_start) { // Starting frame (take the ending samples)