From 1e5b3e7f5349b2996c02e59936c270bc9e9e4197 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sun, 26 Apr 2015 15:22:15 +0200 Subject: [PATCH 1/4] Fix Coverity warning for SoundSourceSndFile *** CID 61970: Uninitialized members (UNINIT_CTOR) /src/sources/soundsourcesndfile.cpp: 17 in Mixxx::SoundSourceSndFile::SoundSourceSndFile(QUrl)() 11 return list; 12 } 13 14 SoundSourceSndFile::SoundSourceSndFile(QUrl url) 15 : SoundSource(url), 16 m_pSndFile(NULL) { >>> CID 61970: Uninitialized members (UNINIT_CTOR) >>> Non-static class member field "m_sfInfo.seekable" is not initialized in this constructor nor in any functions that it calls. 17 } 18 19 SoundSourceSndFile::~SoundSourceSndFile() { 20 close(); 21 } --- src/sources/soundsourcesndfile.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sources/soundsourcesndfile.cpp b/src/sources/soundsourcesndfile.cpp index efa04cdfb2e7..c185e48ac2bc 100644 --- a/src/sources/soundsourcesndfile.cpp +++ b/src/sources/soundsourcesndfile.cpp @@ -14,6 +14,7 @@ QList SoundSourceSndFile::supportedFileExtensions() { SoundSourceSndFile::SoundSourceSndFile(QUrl url) : SoundSource(url), m_pSndFile(NULL) { + memset(&m_sfInfo, 0, sizeof(m_sfInfo)); } SoundSourceSndFile::~SoundSourceSndFile() { @@ -22,7 +23,6 @@ SoundSourceSndFile::~SoundSourceSndFile() { Result SoundSourceSndFile::tryOpen(const AudioSourceConfig& /*audioSrcCfg*/) { DEBUG_ASSERT(!m_pSndFile); - memset(&m_sfInfo, 0, sizeof(m_sfInfo)); #ifdef __WINDOWS__ // Pointer valid until string changed const QString fileName(getLocalFileName()); From 6c68b61930e1e5c471271530e7d4d7b534c6efaf Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sun, 26 Apr 2015 15:27:45 +0200 Subject: [PATCH 2/4] Fix Coverity warning for SoundSourceMp3 *** CID 61968: Uninitialized members (UNINIT_CTOR) /src/sources/soundsourcemp3.cpp: 163 in Mixxx::SoundSourceMp3::SoundSourceMp3(QUrl)() 157 m_fileSize(0), 158 m_pFileData(NULL), 159 m_avgSeekFrameCount(0), 160 m_curFrameIndex(kFrameIndexMin), 161 m_madSynthCount(0) { 162 m_seekFrameList.reserve(kSeekFrameListCapacity); >>> CID 61968: Uninitialized members (UNINIT_CTOR) >>> Non-static class member field "m_madSynth.pcm" is not initialized in this constructor nor in any functions that it calls. 163 } 164 165 SoundSourceMp3::~SoundSourceMp3() { 166 close(); 167 } --- src/sources/soundsourcemp3.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/sources/soundsourcemp3.cpp b/src/sources/soundsourcemp3.cpp index 3b37c9e51735..e32000105f89 100644 --- a/src/sources/soundsourcemp3.cpp +++ b/src/sources/soundsourcemp3.cpp @@ -164,6 +164,9 @@ SoundSourceMp3::SoundSourceMp3(QUrl url) m_curFrameIndex(kFrameIndexMin), m_madSynthCount(0) { m_seekFrameList.reserve(kSeekFrameListCapacity); + mad_stream_init(&m_madStream); + mad_frame_init(&m_madFrame); + mad_synth_init(&m_madSynth); } SoundSourceMp3::~SoundSourceMp3() { @@ -185,12 +188,9 @@ Result SoundSourceMp3::tryOpen(const AudioSourceConfig& /*audioSrcCfg*/) { m_pFileData = m_file.map(0, m_fileSize); // Transfer it to the mad stream-buffer: - mad_stream_init(&m_madStream); mad_stream_options(&m_madStream, MAD_OPTION_IGNORECRC); mad_stream_buffer(&m_madStream, m_pFileData, m_fileSize); DEBUG_ASSERT(m_pFileData == m_madStream.this_frame); - mad_frame_init(&m_madFrame); - mad_synth_init(&m_madSynth); DEBUG_ASSERT(m_seekFrameList.empty()); m_avgSeekFrameCount = 0; @@ -370,10 +370,11 @@ Result SoundSourceMp3::tryOpen(const AudioSourceConfig& /*audioSrcCfg*/) { } void SoundSourceMp3::close() { + mad_synth_finish(&m_madSynth); + mad_frame_finish(&m_madFrame); + mad_stream_finish(&m_madStream); + if (m_pFileData) { - mad_synth_finish(&m_madSynth); - mad_frame_finish(&m_madFrame); - mad_stream_finish(&m_madStream); m_file.unmap(m_pFileData); m_pFileData = NULL; } From 813c8d53fa2a066bfec0a220445635cf99494307 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sun, 26 Apr 2015 17:59:51 +0200 Subject: [PATCH 3/4] Eliminate unneeded member variable in SoundSourceSndFile --- src/sources/soundsourcesndfile.cpp | 12 ++++++------ src/sources/soundsourcesndfile.h | 1 - 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/sources/soundsourcesndfile.cpp b/src/sources/soundsourcesndfile.cpp index c185e48ac2bc..50170b2a113c 100644 --- a/src/sources/soundsourcesndfile.cpp +++ b/src/sources/soundsourcesndfile.cpp @@ -14,7 +14,6 @@ QList SoundSourceSndFile::supportedFileExtensions() { SoundSourceSndFile::SoundSourceSndFile(QUrl url) : SoundSource(url), m_pSndFile(NULL) { - memset(&m_sfInfo, 0, sizeof(m_sfInfo)); } SoundSourceSndFile::~SoundSourceSndFile() { @@ -29,8 +28,9 @@ Result SoundSourceSndFile::tryOpen(const AudioSourceConfig& /*audioSrcCfg*/) { LPCWSTR lpcwFilename = (LPCWSTR) fileName.utf16(); m_pSndFile = sf_wchar_open(lpcwFilename, SFM_READ, &m_sfInfo); #else - m_pSndFile = sf_open(getLocalFileNameBytes().constData(), SFM_READ, - &m_sfInfo); + SF_INFO sfInfo; + memset(&sfInfo, 0, sizeof(sfInfo)); + m_pSndFile = sf_open(getLocalFileNameBytes().constData(), SFM_READ, &sfInfo); #endif if (!m_pSndFile) { // sf_format_check is only for writes @@ -45,9 +45,9 @@ Result SoundSourceSndFile::tryOpen(const AudioSourceConfig& /*audioSrcCfg*/) { return ERR; } - setChannelCount(m_sfInfo.channels); - setFrameRate(m_sfInfo.samplerate); - setFrameCount(m_sfInfo.frames); + setChannelCount(sfInfo.channels); + setFrameRate(sfInfo.samplerate); + setFrameCount(sfInfo.frames); return OK; } diff --git a/src/sources/soundsourcesndfile.h b/src/sources/soundsourcesndfile.h index 81e8824fe78d..cec8a6156150 100644 --- a/src/sources/soundsourcesndfile.h +++ b/src/sources/soundsourcesndfile.h @@ -31,7 +31,6 @@ class SoundSourceSndFile: public Mixxx::SoundSource { Result tryOpen(const AudioSourceConfig& audioSrcCfg) /*override*/; SNDFILE* m_pSndFile; - SF_INFO m_sfInfo; }; } // namespace Mixxx From 6df18030e39abf4218c9858c228743e4abc3b084 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sun, 26 Apr 2015 18:06:00 +0200 Subject: [PATCH 4/4] Symmetric invocation of init()/finish() functions in SoundSourceMp3 --- src/sources/soundsourcemp3.cpp | 24 +++++++++++++++++++----- src/sources/soundsourcemp3.h | 7 +++++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/sources/soundsourcemp3.cpp b/src/sources/soundsourcemp3.cpp index e32000105f89..7b6aca310400 100644 --- a/src/sources/soundsourcemp3.cpp +++ b/src/sources/soundsourcemp3.cpp @@ -164,13 +164,25 @@ SoundSourceMp3::SoundSourceMp3(QUrl url) m_curFrameIndex(kFrameIndexMin), m_madSynthCount(0) { m_seekFrameList.reserve(kSeekFrameListCapacity); + initDecoding(); +} + +SoundSourceMp3::~SoundSourceMp3() { + close(); + finishDecoding(); +} + +void SoundSourceMp3::initDecoding() { mad_stream_init(&m_madStream); mad_frame_init(&m_madFrame); mad_synth_init(&m_madSynth); } -SoundSourceMp3::~SoundSourceMp3() { - close(); +void SoundSourceMp3::finishDecoding() { + m_madSynthCount = 0; + mad_synth_finish(&m_madSynth); + mad_frame_finish(&m_madFrame); + mad_stream_finish(&m_madStream); } Result SoundSourceMp3::tryOpen(const AudioSourceConfig& /*audioSrcCfg*/) { @@ -370,9 +382,7 @@ Result SoundSourceMp3::tryOpen(const AudioSourceConfig& /*audioSrcCfg*/) { } void SoundSourceMp3::close() { - mad_synth_finish(&m_madSynth); - mad_frame_finish(&m_madFrame); - mad_stream_finish(&m_madStream); + finishDecoding(); if (m_pFileData) { m_file.unmap(m_pFileData); @@ -382,6 +392,10 @@ void SoundSourceMp3::close() { m_file.close(); m_seekFrameList.clear(); + + // Re-init the decoder, because the SoundSource might be reopened and + // the destructor calls finishDecoding() after close(). + initDecoding(); } SINT SoundSourceMp3::restartDecoding( diff --git a/src/sources/soundsourcemp3.h b/src/sources/soundsourcemp3.h index d29616f8981a..9eb2fbd70430 100644 --- a/src/sources/soundsourcemp3.h +++ b/src/sources/soundsourcemp3.h @@ -66,7 +66,14 @@ class SoundSourceMp3: public SoundSource { SINT m_curFrameIndex; + // NOTE(uklotzde): Each invocation of initDecoding() must be + // followed by an invocation of finishDecoding(). In between + // 2 matching invocations restartDecoding() might invoked any + // number of times, but only if the files has been opened + // successfully. + void initDecoding(); SINT restartDecoding(const SeekFrameType& seekFrame); + void finishDecoding(); // MAD decoder mad_stream m_madStream;