From 473e955da3d03aa1ccca4c6a6e5428f79bfe7e07 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Wed, 6 Apr 2016 19:09:53 -0400 Subject: [PATCH] Remove some naked new amd delete in: ondemand --- src/Project.cpp | 19 +++---- src/import/ImportFFmpeg.cpp | 19 +++---- src/import/ImportFLAC.cpp | 12 ++-- src/import/ImportPCM.cpp | 8 +-- src/ondemand/ODComputeSummaryTask.cpp | 4 +- src/ondemand/ODComputeSummaryTask.h | 2 +- src/ondemand/ODDecodeFFmpegTask.cpp | 37 ++++++------- src/ondemand/ODDecodeFFmpegTask.h | 2 +- src/ondemand/ODDecodeFlacTask.cpp | 16 ++---- src/ondemand/ODDecodeFlacTask.h | 4 +- src/ondemand/ODManager.cpp | 80 +++++++++++++-------------- src/ondemand/ODManager.h | 11 ++-- src/ondemand/ODTask.h | 2 +- src/ondemand/ODWaveTrackTaskQueue.cpp | 18 +++--- src/ondemand/ODWaveTrackTaskQueue.h | 5 +- 15 files changed, 112 insertions(+), 127 deletions(-) diff --git a/src/Project.cpp b/src/Project.cpp index f5ee51616..d092c75d3 100644 --- a/src/Project.cpp +++ b/src/Project.cpp @@ -3000,8 +3000,7 @@ void AudacityProject::OpenFile(const wxString &fileNameArg, bool addtohistory) TrackListIterator triter(GetTracks()); tr = triter.First(); - std::vector newTasks; - ODTask* newTask; + std::vector> newTasks; //std::vector decodeTasks; unsigned int createdODTasks=0; while (tr) { @@ -3021,16 +3020,16 @@ void AudacityProject::OpenFile(const wxString &fileNameArg, bool addtohistory) //we want at most one instance of each class for the project while((odFlags|createdODTasks) != createdODTasks) { - newTask=NULL; + movable_ptr newTask; #ifdef EXPERIMENTAL_OD_FLAC if(!(createdODTasks&ODTask::eODFLAC) && odFlags & ODTask::eODFLAC) { - newTask= new ODDecodeFlacTask; + newTask = make_movable(); createdODTasks= createdODTasks | ODTask::eODFLAC; } else #endif if(!(createdODTasks&ODTask::eODPCMSummary) && odFlags & ODTask::eODPCMSummary) { - newTask=new ODComputeSummaryTask; + newTask = make_movable(); createdODTasks= createdODTasks | ODTask::eODPCMSummary; } else { @@ -3041,14 +3040,14 @@ void AudacityProject::OpenFile(const wxString &fileNameArg, bool addtohistory) if(newTask) { newTask->AddWaveTrack((WaveTrack*)tr); - newTasks.push_back(newTask); + newTasks.push_back(std::move(newTask)); } } } tr = triter.Next(); } for(unsigned int i=0;iAddNewTask(newTasks[i]); + ODManager::Instance()->AddNewTask(std::move(newTasks[i])); //release the flag. ODManager::UnmarkLoadedODFlag(); @@ -4167,7 +4166,7 @@ void AudacityProject::PopState(const UndoState &state) TrackListIterator iter(tracks); Track *t = iter.First(); bool odUsed = false; - ODComputeSummaryTask* computeTask = NULL; + movable_ptr computeTask; while (t) { @@ -4187,7 +4186,7 @@ void AudacityProject::PopState(const UndoState &state) { if(!odUsed) { - computeTask=new ODComputeSummaryTask; + computeTask = make_movable(); odUsed=true; } computeTask->AddWaveTrack((WaveTrack*)copyTrack); @@ -4198,7 +4197,7 @@ void AudacityProject::PopState(const UndoState &state) //add the task. if(odUsed) - ODManager::Instance()->AddNewTask(computeTask); + ODManager::Instance()->AddNewTask(std::move(computeTask)); HandleResize(); diff --git a/src/import/ImportFFmpeg.cpp b/src/import/ImportFFmpeg.cpp index 95315d892..b5c9bb390 100644 --- a/src/import/ImportFFmpeg.cpp +++ b/src/import/ImportFFmpeg.cpp @@ -578,13 +578,13 @@ int FFmpegImportFileHandle::Import(TrackFactory *trackFactory, //at this point we know the file is good and that we have to load the number of channels in mScs[s]->m_stream->codec->channels; //so for OD loading we create the tracks and releasee the modal lock after starting the ODTask. if (mUsingOD) { - std::vector tasks; + std::vector> tasks; //append blockfiles to each stream and add an individual ODDecodeTask for each one. s = -1; for (const auto &stream : mChannels) { ++s; - ODDecodeFFmpegTask* odTask = - new ODDecodeFFmpegTask(mScs, ODDecodeFFmpegTask::FromList(mChannels), mContext, s); + auto odTask = + make_movable(mScs, ODDecodeFFmpegTask::FromList(mChannels), mContext, s); odTask->CreateFileDecoder(mFilename); //each stream has different duration. We need to know it if seeking is to be allowed. @@ -619,17 +619,12 @@ int FFmpegImportFileHandle::Import(TrackFactory *trackFactory, break; } } - tasks.push_back(odTask); + tasks.push_back(std::move(odTask)); } //Now we add the tasks and let them run, or DELETE them if the user cancelled - for(int i=0; i < (int)tasks.size(); i++) { - if(res==eProgressSuccess) - ODManager::Instance()->AddNewTask(tasks[i]); - else - { - delete tasks[i]; - } - } + if (res == eProgressSuccess) + for (int i = 0; i < (int)tasks.size(); i++) + ODManager::Instance()->AddNewTask(std::move(tasks[i])); } else { #endif diff --git a/src/import/ImportFLAC.cpp b/src/import/ImportFLAC.cpp index 86b6673b3..a3e9ebb23 100644 --- a/src/import/ImportFLAC.cpp +++ b/src/import/ImportFLAC.cpp @@ -178,7 +178,7 @@ private: bool mStreamInfoDone; int mUpdateResult; TrackHolders mChannels; - ODDecodeFlacTask *mDecoderTask; + movable_ptr mDecoderTask; }; @@ -349,13 +349,11 @@ FLACImportFileHandle::FLACImportFileHandle(const wxString & name) bool FLACImportFileHandle::Init() { #ifdef EXPERIMENTAL_OD_FLAC - mDecoderTask=new ODDecodeFlacTask; + mDecoderTask = make_movable(); ODFlacDecoder* odDecoder = (ODFlacDecoder*)mDecoderTask->CreateFileDecoder(mFilename); if(!odDecoder || !odDecoder->ReadHeader()) { - //DELETE the task only if it failed to read - otherwise the OD man takes care of it. - delete mDecoderTask; return false; } //copy the meta data over to the class @@ -511,13 +509,13 @@ int FLACImportFileHandle::Import(TrackFactory *trackFactory, if(moreThanStereo) { //if we have 3 more channels, they get imported on seperate tracks, so we add individual tasks for each. - ODManager::Instance()->AddNewTask(mDecoderTask); - mDecoderTask = new ODDecodeFlacTask; //TODO: see if we need to use clone to keep the metadata. + ODManager::Instance()->AddNewTask(std::move(mDecoderTask)); + mDecoderTask = make_movable(); //TODO: see if we need to use clone to keep the metadata. } } //if we have mono or a linked track (stereo), we add ONE task for the one linked wave track if(!moreThanStereo) - ODManager::Instance()->AddNewTask(mDecoderTask); + ODManager::Instance()->AddNewTask(std::move(mDecoderTask)); } //END OD diff --git a/src/import/ImportPCM.cpp b/src/import/ImportPCM.cpp index 11e7d57ba..816295bdf 100644 --- a/src/import/ImportPCM.cpp +++ b/src/import/ImportPCM.cpp @@ -408,7 +408,7 @@ int PCMImportFileHandle::Import(TrackFactory *trackFactory, if(useOD) { - ODComputeSummaryTask* computeTask=new ODComputeSummaryTask; + auto computeTask = make_movable(); bool moreThanStereo = mInfo.channels>2; for (const auto &channel : channels) { @@ -416,13 +416,13 @@ int PCMImportFileHandle::Import(TrackFactory *trackFactory, if(moreThanStereo) { //if we have 3 more channels, they get imported on seperate tracks, so we add individual tasks for each. - ODManager::Instance()->AddNewTask(computeTask); - computeTask=new ODComputeSummaryTask; + ODManager::Instance()->AddNewTask(std::move(computeTask)); + computeTask = make_movable(); } } //if we have a linked track, we add ONE task. if(!moreThanStereo) - ODManager::Instance()->AddNewTask(computeTask); + ODManager::Instance()->AddNewTask(std::move(computeTask)); } } else { diff --git a/src/ondemand/ODComputeSummaryTask.cpp b/src/ondemand/ODComputeSummaryTask.cpp index 1f8d293f6..7a78101fe 100644 --- a/src/ondemand/ODComputeSummaryTask.cpp +++ b/src/ondemand/ODComputeSummaryTask.cpp @@ -35,9 +35,9 @@ ODComputeSummaryTask::ODComputeSummaryTask() mHasUpdateRan=false; } -std::unique_ptr ODComputeSummaryTask::Clone() const +movable_ptr ODComputeSummaryTask::Clone() const { - auto clone = std::make_unique(); + auto clone = make_movable(); clone->mDemandSample = GetDemandSample(); return std::move(clone); } diff --git a/src/ondemand/ODComputeSummaryTask.h b/src/ondemand/ODComputeSummaryTask.h index bf15f5432..374122d47 100644 --- a/src/ondemand/ODComputeSummaryTask.h +++ b/src/ondemand/ODComputeSummaryTask.h @@ -36,7 +36,7 @@ class ODComputeSummaryTask final : public ODTask ODComputeSummaryTask(); virtual ~ODComputeSummaryTask(){}; - std::unique_ptr Clone() const override; + movable_ptr Clone() const override; ///Subclasses should override to return respective type. unsigned int GetODType() override { return eODPCMSummary; } diff --git a/src/ondemand/ODDecodeFFmpegTask.cpp b/src/ondemand/ODDecodeFFmpegTask.cpp index 3e7bdf785..c1c63807e 100644 --- a/src/ondemand/ODDecodeFFmpegTask.cpp +++ b/src/ondemand/ODDecodeFFmpegTask.cpp @@ -41,15 +41,18 @@ extern FFmpegLibs *FFmpegLibsInst(); #define kMaxSamplesInCache 4410000 //struct for caching the decoded samples to be used over multiple blockfiles -typedef struct _FFmpegDecodeCache +struct FFMpegDecodeCache { - uint8_t* samplePtr;//interleaved samples + FFMpegDecodeCache() {} + ~FFMpegDecodeCache() { free(samplePtr); } + + uint8_t* samplePtr{};//interleaved samples sampleCount start; sampleCount len; int numChannels; AVSampleFormat samplefmt; // input (from libav) sample format -} FFMpegDecodeCache; +}; //------ ODFFmpegDecoder declaration and defs - here because we strip dependencies from .h files @@ -82,7 +85,7 @@ public: bool SeekingAllowed() ; private: - void InsertCache(FFMpegDecodeCache* cache); + void InsertCache(movable_ptr &&cache); //puts the actual audio samples into the blockfile's data array int FillDataFromCache(samplePtr & data, sampleFormat outFormat, sampleCount & start, sampleCount& len, unsigned int channel); @@ -100,7 +103,7 @@ private: ScsPtr mScs; //!< Pointer to array of pointers to stream contexts. ODDecodeFFmpegTask::Streams mChannels; std::shared_ptr mContext; //!< Format description, also contains metadata and some useful info - std::vector mDecodeCache; + std::vector> mDecodeCache; int mNumSamplesInCache; sampleCount mCurrentPos; //the index of the next sample to be decoded sampleCount mCurrentLen; //length of the last packet decoded @@ -140,9 +143,9 @@ ODDecodeFFmpegTask::~ODDecodeFFmpegTask() } -std::unique_ptr ODDecodeFFmpegTask::Clone() const +movable_ptr ODDecodeFFmpegTask::Clone() const { - auto clone = std::make_unique(mScs, Streams{ mChannels }, mContext, mStreamIndex); + auto clone = make_movable(mScs, Streams{ mChannels }, mContext, mStreamIndex); clone->mDemandSample=GetDemandSample(); //the decoders and blockfiles should not be copied. They are created as the task runs. @@ -275,11 +278,7 @@ ODFFmpegDecoder::~ODFFmpegDecoder() //DELETE our caches. while(mDecodeCache.size()) - { - free(mDecodeCache[0]->samplePtr); - delete mDecodeCache[0]; mDecodeCache.erase(mDecodeCache.begin()); - } DropFFmpegLibs(); } @@ -389,7 +388,7 @@ int ODFFmpegDecoder::Decode(SampleBuffer & data, sampleFormat & format, sampleCo if(actualDecodeStart>start && firstpass) { // find the number of samples for the leading silence int amt = actualDecodeStart - start; - FFMpegDecodeCache* cache = new FFMpegDecodeCache; + auto cache = make_movable(); //printf("skipping/zeroing %i samples. - now:%llu (%f), last:%llu, lastlen:%llu, start %llu, len %llu\n",amt,actualDecodeStart, actualDecodeStartdouble, mCurrentPos, mCurrentLen, start, len); @@ -409,7 +408,7 @@ int ODFFmpegDecoder::Decode(SampleBuffer & data, sampleFormat & format, sampleCo memset(cache->samplePtr, 0, amt * cache->numChannels * SAMPLE_SIZE(format)); - InsertCache(cache); + InsertCache(std::move(cache)); } firstpass=false; mCurrentPos = actualDecodeStart; @@ -596,7 +595,7 @@ int ODFFmpegDecoder::DecodeFrame(streamContext *sc, bool flushing) //TODO- consider growing/unioning a few cache buffers like WaveCache does. //however we can't use wavecache as it isn't going to handle our stereo interleaved part, and isn't for samples //However if other ODDecode tasks need this, we should do a NEW class for caching. - FFMpegDecodeCache* cache = new FFMpegDecodeCache; + auto cache = make_movable(); //len is number of samples per channel cache->numChannels = sc->m_stream->codec->channels; @@ -604,14 +603,14 @@ int ODFFmpegDecoder::DecodeFrame(streamContext *sc, bool flushing) cache->start = mCurrentPos; cache->samplePtr = (uint8_t*) malloc(sc->m_decodedAudioSamplesValidSiz); cache->samplefmt = sc->m_samplefmt; - memcpy(cache->samplePtr, sc->m_decodedAudioSamples, sc->m_decodedAudioSamplesValidSiz); + memcpy(cache->samplePtr, sc->m_decodedAudioSamples.get(), sc->m_decodedAudioSamplesValidSiz); - InsertCache(cache); + InsertCache(std::move(cache)); } return ret; } -void ODFFmpegDecoder::InsertCache(FFMpegDecodeCache* cache) { +void ODFFmpegDecoder::InsertCache(movable_ptr &&cache) { int searchStart = 0; int searchEnd = mDecodeCache.size(); //size() is also a valid insert index. int guess = 0; @@ -634,7 +633,7 @@ void ODFFmpegDecoder::InsertCache(FFMpegDecodeCache* cache) { } mCurrentLen = cache->len; mCurrentPos=cache->start+cache->len; - mDecodeCache.insert(mDecodeCache.begin()+guess, cache); + mDecodeCache.insert(mDecodeCache.begin()+guess, std::move(cache)); // mDecodeCache.push_back(cache); mNumSamplesInCache+=cache->len; @@ -646,8 +645,6 @@ void ODFFmpegDecoder::InsertCache(FFMpegDecodeCache* cache) { //drop which ever index is further from our newly added one. dropindex = (guess > (int)mDecodeCache.size()/2) ? 0 : (mDecodeCache.size()-1); mNumSamplesInCache-=mDecodeCache[dropindex]->len; - free(mDecodeCache[dropindex]->samplePtr); - delete mDecodeCache[dropindex]; mDecodeCache.erase(mDecodeCache.begin()+dropindex); } } diff --git a/src/ondemand/ODDecodeFFmpegTask.h b/src/ondemand/ODDecodeFFmpegTask.h index 404e0b58b..ca4151cdd 100644 --- a/src/ondemand/ODDecodeFFmpegTask.h +++ b/src/ondemand/ODDecodeFFmpegTask.h @@ -37,7 +37,7 @@ public: ODDecodeFFmpegTask(const ScsPtr &scs, Streams &&channels, const std::shared_ptr &context, int streamIndex); virtual ~ODDecodeFFmpegTask(); - std::unique_ptr Clone() const override; + movable_ptr Clone() const override; ///Creates an ODFileDecoder that decodes a file of filetype the subclass handles. ODFileDecoder* CreateFileDecoder(const wxString & fileName) override; diff --git a/src/ondemand/ODDecodeFlacTask.cpp b/src/ondemand/ODDecodeFlacTask.cpp index 202e234d7..edd574dd4 100644 --- a/src/ondemand/ODDecodeFlacTask.cpp +++ b/src/ondemand/ODDecodeFlacTask.cpp @@ -33,9 +33,9 @@ ODDecodeFlacTask::~ODDecodeFlacTask() } -std::unique_ptr ODDecodeFlacTask::Clone() const +movable_ptr ODDecodeFlacTask::Clone() const { - auto clone = std::make_unique(); + auto clone = make_movable(); clone->mDemandSample = GetDemandSample(); //the decoders and blockfiles should not be copied. They are created as the task runs. @@ -220,9 +220,7 @@ bool ODFlacDecoder::ReadHeader() //we want to use the native flac type for quick conversion. /* (sampleFormat) gPrefs->Read(wxT("/SamplingRate/DefaultProjectSampleFormat"), floatSample);*/ - if(mFile) - delete mFile; - mFile = new ODFLACFile(this); + mFile = std::make_unique(this); if (!mHandle.Open(mFName, wxT("rb"))) { @@ -261,15 +259,12 @@ bool ODFlacDecoder::ReadHeader() ODFLACFile* ODFlacDecoder::GetFlacFile() { - return mFile; + return mFile.get(); } ODFlacDecoder::~ODFlacDecoder(){ if(mFile) - { mFile->finish(); - delete mFile; - } } ///Creates an ODFileDecoder that decodes a file of filetype the subclass handles. @@ -303,12 +298,11 @@ ODFileDecoder* ODDecodeFlacTask::CreateFileDecoder(const wxString & fileName) } // Open the file for import - ODFlacDecoder *decoder = new ODFlacDecoder(fileName); + auto decoder = std::make_movable(fileName); */ /* bool success = decoder->Init(); if (!success) { - delete decoder; return NULL; } */ diff --git a/src/ondemand/ODDecodeFlacTask.h b/src/ondemand/ODDecodeFlacTask.h index 53060ff14..4081e63d2 100644 --- a/src/ondemand/ODDecodeFlacTask.h +++ b/src/ondemand/ODDecodeFlacTask.h @@ -51,7 +51,7 @@ class ODDecodeFlacTask final : public ODDecodeTask virtual ~ODDecodeFlacTask(); - std::unique_ptr Clone() const override; + movable_ptr Clone() const override; ///Creates an ODFileDecoder that decodes a file of filetype the subclass handles. ODFileDecoder* CreateFileDecoder(const wxString & fileName) override; @@ -119,7 +119,7 @@ public: private: friend class FLACImportFileHandle; sampleFormat mFormat; - ODFLACFile *mFile; + std::unique_ptr mFile; ODLock mFlacFileLock;//for mFile; wxFFile mHandle; unsigned long mSampleRate; diff --git a/src/ondemand/ODManager.cpp b/src/ondemand/ODManager.cpp index 92c4bfedd..578c6e6eb 100644 --- a/src/ondemand/ODManager.cpp +++ b/src/ondemand/ODManager.cpp @@ -32,7 +32,7 @@ static bool gPause=false; //to be loaded in and used with Pause/Resume before OD /// a flag that is set if we have loaded some OD blockfiles from PCM. static bool sHasLoadedOD=false; -ODManager* ODManager::pMan=NULL; +std::unique_ptr ODManager::pMan{}; //init the accessor function pointer - use the first time version of the interface fetcher //first we need to typedef the function pointer type because the compiler doesn't support it in the raw typedef ODManager* (*pfodman)(); @@ -59,18 +59,36 @@ ODManager::ODManager() mPause = gPause; //must set up the queue condition - mQueueNotEmptyCond = new ODCondition(&mQueueNotEmptyCondLock); + mQueueNotEmptyCond = std::make_unique(&mQueueNotEmptyCondLock); } //private destructor - DELETE with static method Quit() ODManager::~ODManager() { + mTerminateMutex.Lock(); + mTerminate = true; + mTerminateMutex.Unlock(); + + //This while loop waits for ODTasks to finish and the DELETE removes all tasks from the Queue. + //This function is called from the main audacity event thread, so there should not be more requests for pMan + mTerminatedMutex.Lock(); + while (!mTerminated) + { + mTerminatedMutex.Unlock(); + wxThread::Sleep(200); + + //signal the queue not empty condition since the ODMan thread will wait on the queue condition + mQueueNotEmptyCondLock.Lock(); + mQueueNotEmptyCond->Signal(); + mQueueNotEmptyCondLock.Unlock(); + + mTerminatedMutex.Lock(); + } + mTerminatedMutex.Unlock(); + //get rid of all the queues. The queues get rid of the tasks, so we don't worry abut them. //nothing else should be running on OD related threads at this point, so we don't lock. - for(unsigned int i=0;i &&mtask, bool lockMutex) { + auto task = mtask.get(); ODWaveTrackTaskQueue* queue = NULL; if(lockMutex) @@ -135,13 +154,13 @@ void ODManager::AddNewTask(ODTask* task, bool lockMutex) //search for a task containing the lead track. wavetrack removal is threadsafe and bound to the mQueuesMutex //note that GetWaveTrack is not threadsafe, but we are assuming task is not running on a different thread yet. if(mQueues[i]->ContainsWaveTrack(task->GetWaveTrack(0))) - queue=mQueues[i]; + queue = mQueues[i].get(); } if(queue) { //Add it to the existing queue but keep the lock since this reference can be deleted. - queue->AddTask(task); + queue->AddTask(std::move(mtask)); if(lockMutex) mQueuesMutex.Unlock(); } @@ -149,9 +168,9 @@ void ODManager::AddNewTask(ODTask* task, bool lockMutex) { //Make a NEW one, add it to the local track queue, and to the immediate running task list, //since this task is definitely at the head - queue = new ODWaveTrackTaskQueue(); - queue->AddTask(task); - mQueues.push_back(queue); + auto newqueue = make_movable(); + newqueue->AddTask(std::move(mtask)); + mQueues.push_back(std::move(newqueue)); if(lockMutex) mQueuesMutex.Unlock(); @@ -165,7 +184,7 @@ ODManager* ODManager::InstanceFirstTime() gODInitedMutex.Lock(); if(!pMan) { - pMan = new ODManager(); + pMan.reset(safenew ODManager()); pMan->Init(); gManagerCreated = true; } @@ -174,13 +193,13 @@ ODManager* ODManager::InstanceFirstTime() //change the accessor function to use the quicker method. Instance = &(ODManager::InstanceNormal); - return pMan; + return pMan.get(); } //faster method of instance fetching once init is done ODManager* ODManager::InstanceNormal() { - return pMan; + return pMan.get(); } bool ODManager::IsInstanceCreated() @@ -342,25 +361,7 @@ void ODManager::Quit() { if(IsInstanceCreated()) { - pMan->mTerminateMutex.Lock(); - pMan->mTerminate = true; - pMan->mTerminateMutex.Unlock(); - - //This while loop waits for ODTasks to finish and the DELETE removes all tasks from the Queue. - //This function is called from the main audacity event thread, so there should not be more requests for pMan - pMan->mTerminatedMutex.Lock(); - while(!pMan->mTerminated) - { - pMan->mTerminatedMutex.Unlock(); - wxThread::Sleep(200); - - //signal the queue not empty condition since the ODMan thread will wait on the queue condition - pMan->mQueueNotEmptyCond->Signal(); - - pMan->mTerminatedMutex.Lock(); - } - pMan->mTerminatedMutex.Unlock(); - delete pMan; + pMan.reset(); } } @@ -396,7 +397,7 @@ void ODManager::MakeWaveTrackIndependent(WaveTrack* track) { if(mQueues[i]->ContainsWaveTrack(track)) { - owner = mQueues[i]; + owner = mQueues[i].get(); break; } } @@ -425,11 +426,11 @@ bool ODManager::MakeWaveTrackDependent(WaveTrack* dependentTrack,WaveTrack* mast { if(mQueues[i]->ContainsWaveTrack(masterTrack)) { - masterQueue = mQueues[i]; + masterQueue = mQueues[i].get(); } else if(mQueues[i]->ContainsWaveTrack(dependentTrack)) { - dependentQueue = mQueues[i]; + dependentQueue = mQueues[i].get(); dependentIndex = i; } @@ -450,7 +451,6 @@ bool ODManager::MakeWaveTrackDependent(WaveTrack* dependentTrack,WaveTrack* mast //finally remove the dependent track mQueues.erase(mQueues.begin()+dependentIndex); mQueuesMutex.Unlock(); - delete dependentQueue; //note this locks the ODManager's current task queue. return true; } @@ -484,8 +484,7 @@ void ODManager::UpdateQueues() { //we need to release the lock on the queue vector before using the task vector's lock or we deadlock //so get a temp. - ODWaveTrackTaskQueue* queue; - queue = mQueues[i]; + ODWaveTrackTaskQueue* queue = mQueues[i].get(); AddTask(queue->GetFrontTask()); } @@ -494,7 +493,6 @@ void ODManager::UpdateQueues() //if the queue is empty DELETE it. if(mQueues[i]->IsEmpty()) { - delete mQueues[i]; mQueues.erase(mQueues.begin()+i); i--; } diff --git a/src/ondemand/ODManager.h b/src/ondemand/ODManager.h index ee367633f..4113723f7 100644 --- a/src/ondemand/ODManager.h +++ b/src/ondemand/ODManager.h @@ -62,7 +62,7 @@ class ODManager final void DecrementCurrentThreads(); ///Adds a wavetrack, creates a queue member. - void AddNewTask(ODTask* task, bool lockMutex=true); + void AddNewTask(movable_ptr &&mtask, bool lockMutex=true); ///Wakes the queue loop up by signalling its condition variable. void SignalTaskQueueLoop(); @@ -125,7 +125,8 @@ class ODManager final //private constructor - Singleton. ODManager(); //private constructor - DELETE with static method Quit() - virtual ~ODManager(); + friend std::default_delete < ODManager > ; + ~ODManager(); ///Launches a thread for the manager and starts accepting Tasks. void Init(); @@ -136,10 +137,10 @@ class ODManager final void UpdateQueues(); //instance - static ODManager* pMan; + static std::unique_ptr pMan; //List of tracks and their active and inactive tasks. - std::vector mQueues; + std::vector> mQueues; ODLock mQueuesMutex; //List of current Task to do. @@ -169,7 +170,7 @@ class ODManager final //for the queue not empty comdition ODLock mQueueNotEmptyCondLock; - ODCondition* mQueueNotEmptyCond; + std::unique_ptr mQueueNotEmptyCond; #ifdef __WXMAC__ diff --git a/src/ondemand/ODTask.h b/src/ondemand/ODTask.h index 2f8399f23..847e080b1 100644 --- a/src/ondemand/ODTask.h +++ b/src/ondemand/ODTask.h @@ -55,7 +55,7 @@ class ODTask /* not final */ virtual ~ODTask(){}; //clones everything except information about the tracks. - virtual std::unique_ptr Clone() const = 0; + virtual movable_ptr Clone() const = 0; ///Subclasses should override to return respective type. virtual unsigned int GetODType(){return eODNone;} diff --git a/src/ondemand/ODWaveTrackTaskQueue.cpp b/src/ondemand/ODWaveTrackTaskQueue.cpp index 10f3041e9..5b934f305 100644 --- a/src/ondemand/ODWaveTrackTaskQueue.cpp +++ b/src/ondemand/ODWaveTrackTaskQueue.cpp @@ -16,6 +16,7 @@ tasks associated with a WaveTrack. *//*******************************************************************/ +#include "../Audacity.h" #include "ODWaveTrackTaskQueue.h" #include "ODTask.h" #include "ODManager.h" @@ -31,8 +32,8 @@ ODWaveTrackTaskQueue::~ODWaveTrackTaskQueue() { mTasks[i]->TerminateAndBlock();//blocks if active. //small chance we may have rea-added the task back into the queue from a diff thread. - so remove it if we have. - ODManager::Instance()->RemoveTaskIfInQueue(mTasks[i]); - delete mTasks[i]; + ODManager::Instance()->RemoveTaskIfInQueue(mTasks[i].get()); + mTasks[i].reset(); } } @@ -100,10 +101,12 @@ void ODWaveTrackTaskQueue::AddWaveTrack(WaveTrack* track) mTracksMutex.Unlock(); } -void ODWaveTrackTaskQueue::AddTask(ODTask* task) +void ODWaveTrackTaskQueue::AddTask(movable_ptr &&mtask) { + ODTask *task = mtask.get(); + mTasksMutex.Lock(); - mTasks.push_back(task); + mTasks.push_back(std::move(mtask)); mTasksMutex.Unlock(); //take all of the tracks in the task. @@ -172,7 +175,7 @@ void ODWaveTrackTaskQueue::MakeWaveTrackIndependent(WaveTrack* track) mTasksMutex.Unlock(); //AddNewTask locks the m_queuesMutex which is already locked by ODManager::MakeWaveTrackIndependent, //so we pass a boolean flag telling it not to lock again. - ODManager::Instance()->AddNewTask(task.release(), false); + ODManager::Instance()->AddNewTask(std::move(task), false); mTasksMutex.Lock(); } mTasksMutex.Unlock(); @@ -256,7 +259,7 @@ ODTask* ODWaveTrackTaskQueue::GetTask(size_t x) ODTask* ret = NULL; mTasksMutex.Lock(); if (x < mTasks.size()) - ret = mTasks[x]; + ret = mTasks[x].get(); mTasksMutex.Unlock(); return ret; } @@ -304,7 +307,6 @@ void ODWaveTrackTaskQueue::RemoveFrontTask() if(mTasks.size()) { //wait for the task to stop running. - delete mTasks[0]; mTasks.erase(mTasks.begin()); } mTasksMutex.Unlock(); @@ -317,7 +319,7 @@ ODTask* ODWaveTrackTaskQueue::GetFrontTask() if(mTasks.size()) { mTasksMutex.Unlock(); - return mTasks[0]; + return mTasks[0].get(); } mTasksMutex.Unlock(); return NULL; diff --git a/src/ondemand/ODWaveTrackTaskQueue.h b/src/ondemand/ODWaveTrackTaskQueue.h index 175df1ca4..6d274da4b 100644 --- a/src/ondemand/ODWaveTrackTaskQueue.h +++ b/src/ondemand/ODWaveTrackTaskQueue.h @@ -22,6 +22,7 @@ tasks associated with a WaveTrack. #ifndef __AUDACITY_ODWAVETRACKTASKQUEUE__ #define __AUDACITY_ODWAVETRACKTASKQUEUE__ +#include "../MemoryX.h" #include #include "ODTaskThread.h" #include @@ -71,7 +72,7 @@ class ODWaveTrackTaskQueue final int GetNumWaveTracks(); ///Add a task to the queue. - void AddTask(ODTask* task); + void AddTask(movable_ptr &&mtask); //returns true if either tracks or tasks are empty bool IsEmpty(); @@ -105,7 +106,7 @@ class ODWaveTrackTaskQueue final ODLock mTracksMutex; ///the list of tasks associated with the tracks. This class owns these tasks. - std::vector mTasks; + std::vector> mTasks; ODLock mTasksMutex; };