Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Analysis Fixes #1289

Merged
merged 9 commits into from
Jun 22, 2017
36 changes: 28 additions & 8 deletions src/analyzer/analyzerqueue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@
#include "analyzer/analyzergain.h"
#include "analyzer/analyzerebur128.h"
#include "analyzer/analyzerwaveform.h"
#include "library/dao/analysisdao.h"
#include "mixer/playerinfo.h"
#include "sources/soundsourceproxy.h"
#include "track/track.h"
#include "util/compatibility.h"
#include "util/db/dbconnectionpooler.h"
#include "util/db/dbconnectionpooled.h"
#include "util/event.h"
#include "util/timer.h"
#include "util/trace.h"
Expand Down Expand Up @@ -51,7 +53,8 @@ AnalyzerQueue::AnalyzerQueue(
m_queue_size(0) {

if (mode != Mode::WithoutWaveform) {
m_pAnalyzers.push_back(std::make_unique<AnalyzerWaveform>(pConfig));
m_pAnalysisDao = std::make_unique<AnalysisDao>(pConfig);
m_pAnalyzers.push_back(std::make_unique<AnalyzerWaveform>(m_pAnalysisDao.get()));
}
m_pAnalyzers.push_back(std::make_unique<AnalyzerGain>(pConfig));
m_pAnalyzers.push_back(std::make_unique<AnalyzerEbur128>(pConfig));
Expand Down Expand Up @@ -197,7 +200,7 @@ bool AnalyzerQueue::doAnalysis(TrackPointer tio, mixxx::AudioSourcePointer pAudi

const SINT framesRead =
pAudioSource->readSampleFramesStereo(
kAnalysisFramesPerBlock,
framesToRead,
&m_sampleBuffer);
DEBUG_ASSERT(framesRead <= framesToRead);
frameIndex += framesRead;
Expand Down Expand Up @@ -299,12 +302,22 @@ void AnalyzerQueue::run() {
}

void AnalyzerQueue::execThread() {
const mixxx::DbConnectionPooler dbConnection(m_pDbConnectionPool);
if (!dbConnection) {
kLogger.warning()
<< "Failed to open database connection for analyzer queue";
kLogger.debug() << "Exiting thread";
return;
// The thread-local database connection for waveform anylsis must not
// be closed before returning from this function. Therefore the
// DbConnectionPooler is defined at the outher function scope,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo here. Did you mean "other" or "outer"?

// independent of whether a database connection is opened or not.
mixxx::DbConnectionPooler dbConnectionPooler;
// m_pAnalysisDao remains null if no analyzer needs database access.
// Currently only waveform analyses makes use of it.
if (m_pAnalysisDao) {
dbConnectionPooler = mixxx::DbConnectionPooler(m_pDbConnectionPool);
if (!dbConnectionPooler) {
kLogger.warning()
<< "Failed to open database connection for analyzer queue thread";
return;
}
// Obtain and use the newly created database connection within this thread
m_pAnalysisDao->initialize(mixxx::DbConnectionPooled(m_pDbConnectionPool));
}

m_progressInfo.current_track.reset();
Expand Down Expand Up @@ -380,6 +393,13 @@ void AnalyzerQueue::execThread() {
}
emptyCheck();
}

if (m_pAnalysisDao) {
// Invalidate reference to the thread-local database connection
// that will be closed soon. Not necessary, just in case ;)
m_pAnalysisDao->initialize(QSqlDatabase());
}

emit(queueEmpty()); // emit in case of exit;
}

Expand Down
3 changes: 3 additions & 0 deletions src/analyzer/analyzerqueue.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "util/memory.h"

class Analyzer;
class AnalysisDao;

class AnalyzerQueue : public QThread {
Q_OBJECT
Expand Down Expand Up @@ -60,6 +61,8 @@ class AnalyzerQueue : public QThread {

mixxx::DbConnectionPoolPtr m_pDbConnectionPool;

std::unique_ptr<AnalysisDao> m_pAnalysisDao;

typedef std::unique_ptr<Analyzer> AnalyzerPtr;
std::vector<AnalyzerPtr> m_pAnalyzers;

Expand Down
13 changes: 7 additions & 6 deletions src/analyzer/analyzerwaveform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,15 @@ mixxx::Logger kLogger("AnalyzerWaveform");
} // anonymous

AnalyzerWaveform::AnalyzerWaveform(
const UserSettingsPointer& pConfig) :
m_analysisDao(pConfig),
AnalysisDao* pAnalysisDao) :
m_pAnalysisDao(pAnalysisDao),
m_skipProcessing(false),
m_waveformData(nullptr),
m_waveformSummaryData(nullptr),
m_stride(0, 0),
m_currentStride(0),
m_currentSummaryStride(0) {
DEBUG_ASSERT(m_pAnalysisDao); // mandatory
m_filter[0] = 0;
m_filter[1] = 0;
m_filter[2] = 0;
Expand Down Expand Up @@ -102,7 +103,7 @@ bool AnalyzerWaveform::isDisabledOrLoadStoredSuccess(TrackPointer tio) const {

if (trackId.isValid() && (missingWaveform || missingWavesummary)) {
QList<AnalysisDao::AnalysisInfo> analyses =
m_analysisDao.getAnalysesForTrack(trackId);
m_pAnalysisDao->getAnalysesForTrack(trackId);

QListIterator<AnalysisDao::AnalysisInfo> it(analyses);
while (it.hasNext()) {
Expand All @@ -117,7 +118,7 @@ bool AnalyzerWaveform::isDisabledOrLoadStoredSuccess(TrackPointer tio) const {
missingWaveform = false;
} else if (vc != WaveformFactory::VC_KEEP) {
// remove all other Analysis except that one we should keep
m_analysisDao.deleteAnalysis(analysis.analysisId);
m_pAnalysisDao->deleteAnalysis(analysis.analysisId);
}
} if (analysis.type == AnalysisDao::TYPE_WAVESUMMARY) {
vc = WaveformFactory::waveformSummaryVersionToVersionClass(analysis.version);
Expand All @@ -127,7 +128,7 @@ bool AnalyzerWaveform::isDisabledOrLoadStoredSuccess(TrackPointer tio) const {
missingWavesummary = false;
} else if (vc != WaveformFactory::VC_KEEP) {
// remove all other Analysis except that one we should keep
m_analysisDao.deleteAnalysis(analysis.analysisId);
m_pAnalysisDao->deleteAnalysis(analysis.analysisId);
}
}
}
Expand Down Expand Up @@ -309,7 +310,7 @@ void AnalyzerWaveform::finalize(TrackPointer tio) {
// waveforms (i.e. if the config setting was disabled in a previous scan)
// and then it is not called. The other analyzers have signals which control
// the update of their data.
m_analysisDao.saveTrackAnalyses(*tio);
m_pAnalysisDao->saveTrackAnalyses(*tio);

kLogger.debug() << "Waveform generation for track" << tio->getId() << "done"
<< m_timer.elapsed().debugSecondsWithUnit();
Expand Down
6 changes: 3 additions & 3 deletions src/analyzer/analyzerwaveform.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@

#include "analyzer/analyzer.h"
#include "waveform/waveform.h"
#include "library/dao/analysisdao.h"
#include "util/math.h"
#include "util/performancetimer.h"

//NOTS vrince some test to segment sound, to apply color in the waveform
//#define TEST_HEAT_MAP

class EngineFilterIIRBase;
class AnalysisDao;

inline CSAMPLE scaleSignal(CSAMPLE invalue, FilterIndex index = FilterCount) {
if (invalue == 0.0) {
Expand Down Expand Up @@ -137,7 +137,7 @@ struct WaveformStride {
class AnalyzerWaveform : public Analyzer {
public:
explicit AnalyzerWaveform(
const UserSettingsPointer& pConfig);
AnalysisDao* pAnalysisDao);
~AnalyzerWaveform() override;

bool initialize(TrackPointer tio, int sampleRate, int totalSamples) override;
Expand All @@ -154,7 +154,7 @@ class AnalyzerWaveform : public Analyzer {
void destroyFilters();
void storeIfGreater(float* pDest, float source);

mutable AnalysisDao m_analysisDao;
AnalysisDao* m_pAnalysisDao;

bool m_skipProcessing;

Expand Down
4 changes: 3 additions & 1 deletion src/test/analyserwaveformtest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ namespace {
class AnalyzerWaveformTest: public MixxxTest {
protected:
AnalyzerWaveformTest()
: aw(config()),
: analysisDao(config()),
aw(&analysisDao),
bigbuf(nullptr),
canaryBigBuf(nullptr) {
}
Expand Down Expand Up @@ -49,6 +50,7 @@ class AnalyzerWaveformTest: public MixxxTest {
}

protected:
AnalysisDao analysisDao;
AnalyzerWaveform aw;
TrackPointer tio;
CSAMPLE* bigbuf;
Expand Down
11 changes: 10 additions & 1 deletion src/util/db/dbconnectionpooler.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,19 @@ class DbConnectionPooler final {
return static_cast<bool>(m_pDbConnectionPool);
}

#if !defined(_MSC_VER) || _MSC_VER > 1900
DbConnectionPooler& operator=(DbConnectionPooler&&) = default;
#else
// Workaround for Visual Studio 2015 (and before)
DbConnectionPooler& operator=(DbConnectionPooler&& other) {
m_pDbConnectionPool = std::move(other.m_pDbConnectionPool);
return *this;
}
#endif

private:
DbConnectionPooler(const DbConnectionPooler&) = delete;
DbConnectionPooler& operator=(const DbConnectionPooler&) = delete;
DbConnectionPooler& operator=(DbConnectionPooler&&) = delete;

// Prevent heap allocation
static void * operator new(std::size_t);
Expand Down