From 2f8c4161dfb84e0dbfbc6c616fc3e96e639b0244 Mon Sep 17 00:00:00 2001 From: Jonathan Thomas Date: Thu, 24 Dec 2015 16:44:45 -0600 Subject: [PATCH] Improved changing Reader() on the QtPlayer class, and made many thread safety improvements for the FrameMapper.GetFrame method. Also improved the destructors and Close() methods of FFmpegReader and FrameMapper. Some audio-related bug fixes also, related to audio playback. --- include/FFmpegReader.h | 3 ++ include/FrameMapper.h | 6 +++ src/AudioReaderSource.cpp | 18 +++++--- src/FFmpegReader.cpp | 15 +++++-- src/FrameMapper.cpp | 75 ++++++++++++++++++++++++++-------- src/Qt/AudioPlaybackThread.cpp | 58 ++++++++++++-------------- src/Qt/VideoCacheThread.cpp | 5 ++- src/QtPlayer.cpp | 19 +-------- 8 files changed, 121 insertions(+), 78 deletions(-) diff --git a/include/FFmpegReader.h b/include/FFmpegReader.h index 73fdc452..8519ed02 100644 --- a/include/FFmpegReader.h +++ b/include/FFmpegReader.h @@ -237,6 +237,9 @@ namespace openshot /// frame 1, or it throws one of the following exceptions. FFmpegReader(string path) throw(InvalidFile, NoStreamsFound, InvalidCodec); + /// Destructor + ~FFmpegReader(); + /// Close File void Close(); diff --git a/include/FrameMapper.h b/include/FrameMapper.h index 37a735da..c37a37b2 100644 --- a/include/FrameMapper.h +++ b/include/FrameMapper.h @@ -152,6 +152,9 @@ namespace openshot void AddField(long int frame); void AddField(Field field); + // Get Frame or Generate Blank Frame + tr1::shared_ptr GetOrCreateFrame(long int 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. // This might repeat or skip fields and frames of the original video, depending on @@ -166,6 +169,9 @@ namespace openshot /// 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); + /// Destructor + ~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); diff --git a/src/AudioReaderSource.cpp b/src/AudioReaderSource.cpp index aceb14f4..48adeab1 100644 --- a/src/AudioReaderSource.cpp +++ b/src/AudioReaderSource.cpp @@ -177,10 +177,17 @@ void AudioReaderSource::getNextAudioBlock (const AudioSourceChannelInfo& info) int number_to_copy = 0; // Do we need more samples? - if ((reader && reader->IsOpen() && !frame) or - (reader && reader->IsOpen() && buffer_samples - position < info.numSamples)) - // Refill buffer from reader - GetMoreSamplesFromReader(); + if (speed == 1) { + // Only refill buffers if speed is normal + if ((reader && reader->IsOpen() && !frame) or + (reader && reader->IsOpen() && buffer_samples - position < info.numSamples)) + // Refill buffer from reader + GetMoreSamplesFromReader(); + } else { + // Fill buffer with silence and clear current frame + info.buffer->clear(); + return; + } // Determine how many samples to copy if (position + info.numSamples <= buffer_samples) @@ -218,8 +225,7 @@ void AudioReaderSource::getNextAudioBlock (const AudioSourceChannelInfo& info) // Adjust estimate frame number (the estimated frame number that is being played) estimated_samples_per_frame = Frame::GetSamplesPerFrame(estimated_frame, reader->info.fps, reader->info.sample_rate, buffer_channels); - if (speed == 1) - estimated_frame += double(info.numSamples) / double(estimated_samples_per_frame); + estimated_frame += double(info.numSamples) / double(estimated_samples_per_frame); } } diff --git a/src/FFmpegReader.cpp b/src/FFmpegReader.cpp index 0f97586c..8ca65b08 100644 --- a/src/FFmpegReader.cpp +++ b/src/FFmpegReader.cpp @@ -54,6 +54,12 @@ FFmpegReader::FFmpegReader(string path) throw(InvalidFile, NoStreamsFound, Inval Close(); } +FFmpegReader::~FFmpegReader() { + if (is_open) + // Auto close reader if not already done + Close(); +} + // Init a collection of software rescalers (thread safe) void FFmpegReader::InitScalers() { @@ -221,6 +227,12 @@ void FFmpegReader::Close() // Close all objects, if reader is 'open' if (is_open) { + // Mark as "closed" + is_open = false; + + // Create a scoped lock, allowing only a single thread to run the following code at one time + const GenericScopedLock lock(getFrameCriticalSection); + // Close the codec if (info.has_video) { @@ -258,9 +270,6 @@ void FFmpegReader::Close() avformat_close_input(&pFormatCtx); av_freep(&pFormatCtx); - // Mark as "closed" - is_open = false; - // Reset some variables last_frame = 0; largest_frame_processed = 0; diff --git a/src/FrameMapper.cpp b/src/FrameMapper.cpp index 2736181c..1274fb7e 100644 --- a/src/FrameMapper.cpp +++ b/src/FrameMapper.cpp @@ -56,6 +56,13 @@ FrameMapper::FrameMapper(ReaderBase *reader, Fraction target, PulldownType targe Init(); } +// Destructor +FrameMapper::~FrameMapper() { + if (is_open) + // Auto Close if not already + Close(); +} + void FrameMapper::AddField(long int frame) { // Add a field, and toggle the odd / even field @@ -317,6 +324,36 @@ MappedFrame FrameMapper::GetMappedFrame(long int TargetFrameNumber) throw(OutOfB return frames[TargetFrameNumber - 1]; } +// Get or generate a blank frame +tr1::shared_ptr FrameMapper::GetOrCreateFrame(long int number) +{ + tr1::shared_ptr new_frame; + + // Init some basic properties about this frame + int samples_in_frame = Frame::GetSamplesPerFrame(number, target, info.sample_rate, info.channels); + + try { + // Attempt to get a frame (but this could fail if a reader has just been closed) + new_frame = reader->GetFrame(number); + + // Return real frame + return new_frame; + + } catch (const ReaderClosed & e) { + // ... + } catch (const TooManySeeks & e) { + // ... + } catch (const OutOfBoundsFrame & e) { + // ... + } + + // Create blank frame + new_frame = tr1::shared_ptr(new Frame(number, info.width, info.height, "#000000", samples_in_frame, info.channels)); + new_frame->SampleRate(info.sample_rate); + new_frame->ChannelsLayout(info.channel_layout); + return new_frame; +} + // Get an openshot::Frame object for a specific frame number of this reader. tr1::shared_ptr FrameMapper::GetFrame(long int requested_frame) throw(ReaderClosed) { @@ -337,7 +374,9 @@ 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; + // TODO: Find a safe way to deal with Closing the reader while multi-processing is happening + // In the meantime, I'm leaving this at 1 + int minimum_frames = OPEN_MP_NUM_PROCESSORS; //OPEN_MP_NUM_PROCESSORS // Set the number of threads in OpenMP omp_set_num_threads(OPEN_MP_NUM_PROCESSORS); @@ -358,11 +397,10 @@ tr1::shared_ptr FrameMapper::GetFrame(long int requested_frame) throw(Rea tr1::shared_ptr mapped_frame; #pragma omp ordered - mapped_frame = reader->GetFrame(mapped.Odd.Frame); + mapped_frame = GetOrCreateFrame(mapped.Odd.Frame); + // Get # of channels in the actual frame int channels_in_frame = mapped_frame->GetAudioChannelsCount(); - - // Init some basic properties about this frame int samples_in_frame = Frame::GetSamplesPerFrame(frame_number, target, mapped_frame->SampleRate(), channels_in_frame); // Create a new frame @@ -373,19 +411,19 @@ tr1::shared_ptr FrameMapper::GetFrame(long int requested_frame) throw(Rea // Copy the image from the odd field tr1::shared_ptr odd_frame; - #pragma omp ordered - odd_frame = reader->GetFrame(mapped.Odd.Frame); - if (odd_frame) - frame->AddImage(tr1::shared_ptr(new QImage(*odd_frame->GetImage())), true); - if (mapped.Odd.Frame != mapped.Even.Frame) { - // Add even lines (if different than the previous image) - tr1::shared_ptr even_frame; - #pragma omp ordered - even_frame = reader->GetFrame(mapped.Even.Frame); - if (even_frame) - frame->AddImage(tr1::shared_ptr(new QImage(*even_frame->GetImage())), false); - } + #pragma omp ordered + odd_frame = GetOrCreateFrame(mapped.Odd.Frame); + if (odd_frame) + frame->AddImage(tr1::shared_ptr(new QImage(*odd_frame->GetImage())), true); + if (mapped.Odd.Frame != mapped.Even.Frame) { + // Add even lines (if different than the previous image) + tr1::shared_ptr even_frame; + #pragma omp ordered + even_frame = GetOrCreateFrame(mapped.Even.Frame); + if (even_frame) + frame->AddImage(tr1::shared_ptr(new QImage(*even_frame->GetImage())), false); + } // Copy the samples int samples_copied = 0; @@ -400,7 +438,7 @@ tr1::shared_ptr FrameMapper::GetFrame(long int requested_frame) throw(Rea for (int channel = 0; channel < channels_in_frame; channel++) { // number of original samples on this frame - tr1::shared_ptr original_frame = reader->GetFrame(starting_frame); + tr1::shared_ptr original_frame = GetOrCreateFrame(starting_frame); int original_samples = original_frame->GetAudioSamplesCount(); if (starting_frame == mapped.Samples.frame_start) @@ -520,6 +558,9 @@ void FrameMapper::Close() { if (reader) { + // Create a scoped lock, allowing only a single thread to run the following code at one time + const GenericScopedLock lock(getFrameCriticalSection); + AppendDebugMethod("FrameMapper::Open", "", -1, "", -1, "", -1, "", -1, "", -1, "", -1); // Close internal reader diff --git a/src/Qt/AudioPlaybackThread.cpp b/src/Qt/AudioPlaybackThread.cpp index dfca232f..c603ad84 100644 --- a/src/Qt/AudioPlaybackThread.cpp +++ b/src/Qt/AudioPlaybackThread.cpp @@ -64,46 +64,22 @@ namespace openshot } // Set the reader object - void AudioPlaybackThread::Reader(ReaderBase *reader) - { - // Stop any existing audio - if (source) { - // Stop playing - Stop(); - transport.stop(); - - // Wait for transport to stop - while (transport.isPlaying()) { - cout << "waiting for transport to stop" << endl; - sleep(25); - } - - // Kill previous audio - transport.setSource(0); - - player.setSource(0); - audioDeviceManager.removeAudioCallback(&player); - audioDeviceManager.closeAudioDevice(); - audioDeviceManager.removeAllChangeListeners(); - audioDeviceManager.dispatchPendingMessages(); - - // Remove source - delete source; - source = NULL; + void AudioPlaybackThread::Reader(ReaderBase *reader) { + if (source) + source->Reader(reader); + else { + // Create new audio source reader + source = new AudioReaderSource(reader, 1, buffer_size); + source->setLooping(true); // prevent this source from terminating when it reaches the end } // Set local vars sampleRate = reader->info.sample_rate; numChannels = reader->info.channels; - // Create new audio source reader - source = new AudioReaderSource(reader, 1, buffer_size); - source->setLooping(true); // prevent this source from terminating when it reaches the end - // Mark as 'playing' Play(); - - } + } // Get the current frame object (which is filling the buffer) tr1::shared_ptr AudioPlaybackThread::getFrame() @@ -178,7 +154,23 @@ namespace openshot while (!threadShouldExit() && transport.isPlaying() && is_playing) sleep(100); - is_playing = false; + + // Stop audio and shutdown transport + Stop(); + transport.stop(); + + // Kill previous audio + transport.setSource(NULL); + + player.setSource(NULL); + audioDeviceManager.removeAudioCallback(&player); + audioDeviceManager.closeAudioDevice(); + audioDeviceManager.removeAllChangeListeners(); + audioDeviceManager.dispatchPendingMessages(); + + // Remove source + delete source; + source = NULL; // Stop time slice thread thread.stopThread(-1); diff --git a/src/Qt/VideoCacheThread.cpp b/src/Qt/VideoCacheThread.cpp index 5073a4b9..49c20b26 100644 --- a/src/Qt/VideoCacheThread.cpp +++ b/src/Qt/VideoCacheThread.cpp @@ -84,7 +84,7 @@ namespace openshot // Cache frames before the other threads need them // Cache frames up to the max frames - while ((position - current_display_frame) < max_frames) + while (speed == 1 && (position - current_display_frame) < max_frames) { // Only cache up till the max_frames amount... then sleep try @@ -102,6 +102,9 @@ namespace openshot // Increment frame number position++; } + + // Sleep for 1 frame length + sleep(frame_time); } return; diff --git a/src/QtPlayer.cpp b/src/QtPlayer.cpp index 6b887262..754d1c53 100644 --- a/src/QtPlayer.cpp +++ b/src/QtPlayer.cpp @@ -59,23 +59,6 @@ void QtPlayer::SetSource(const std::string &source) reader->debug = false; reader->Open(); - // experimental timeline code - //Clip *c = new Clip(source); - //c->scale = SCALE_NONE; - //c->rotation.AddPoint(1, 0.0); - //c->rotation.AddPoint(1000, 360.0); - //c->Waveform(true); - - //Timeline *t = new Timeline(c->Reader()->info.width, c->Reader()->info.height, c->Reader()->info.fps, c->Reader()->info.sample_rate, c->Reader()->info.channels); - //Timeline *t = new Timeline(1280, 720, openshot::Fraction(24,1), 44100, 2, LAYOUT_STEREO); - //t->debug = true; openshot::Fraction(30,1) - //t->info = c->Reader()->info; - //t->info.fps = openshot::Fraction(12,1); - //t->GetCache()->SetMaxBytesFromInfo(40, c->Reader()->info.width, c->Reader()->info.height, c->Reader()->info.sample_rate, c->Reader()->info.channels); - - //t->AddClip(c); - //t->Open(); - // Set the reader Reader(reader); } @@ -154,7 +137,7 @@ void QtPlayer::Stop() // Set the reader object void QtPlayer::Reader(ReaderBase *new_reader) { - cout << "Reader SET: " << new_reader << endl; + // Set new reader. Note: Be sure to close and dispose of the old reader after calling this reader = new_reader; p->reader = new_reader; p->videoCache->Reader(new_reader);