From 32b9065dba36e0061adbaf4721ef159e362c398f Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Tue, 8 May 2018 10:47:45 +0200 Subject: [PATCH 1/3] Fix decoding of improperly encoded FLAC files --- src/sources/soundsourceflac.cpp | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/sources/soundsourceflac.cpp b/src/sources/soundsourceflac.cpp index 01dfc00149..3d68e05ce7 100644 --- a/src/sources/soundsourceflac.cpp +++ b/src/sources/soundsourceflac.cpp @@ -345,6 +345,25 @@ FLAC__bool SoundSourceFLAC::flacEOF() { return m_file.atEnd(); } +inline +FLAC__int32 adjustDecodedSample(FLAC__int32 decodedSample, SINT bitsPerSample) { + // Workaround for improperly encoded FLAC files that may contain + // garbage in the most significant, unused bits of decoded samples. + // Required at least for libFLAC 1.3.2. This workaround might become + // obsolete once libFLAC is taking care of these issues internally. + // https://bugs.launchpad.net/mixxx/+bug/1769717 + // https://hydrogenaud.io/index.php/topic,61792.msg559045.html#msg559045 + FLAC__int32 signBit = static_cast(1) << (bitsPerSample - 1); + FLAC__int32 bitMask = (static_cast(1) << bitsPerSample) - 1; // == (signBit << 1) - 1 + FLAC__int32 maskedSample = decodedSample & bitMask; + if (maskedSample & signBit) { + // Sign extension for negative values + return maskedSample | ~bitMask; + } else { + return maskedSample; + } +} + FLAC__StreamDecoderWriteStatus SoundSourceFLAC::flacWrite( const FLAC__Frame* frame, const FLAC__int32* const buffer[]) { const SINT numChannels = frame->header.channels; @@ -391,15 +410,15 @@ FLAC__StreamDecoderWriteStatus SoundSourceFLAC::flacWrite( case 1: { // optimized code for 1 channel (mono) for (SINT i = 0; i < numWritableFrames; ++i) { - *pSampleBuffer++ = buffer[0][i] * m_sampleScaleFactor; + *pSampleBuffer++ = adjustDecodedSample(buffer[0][i], m_bitsPerSample) * m_sampleScaleFactor; } break; } case 2: { // optimized code for 2 channels (stereo) for (SINT i = 0; i < numWritableFrames; ++i) { - *pSampleBuffer++ = buffer[0][i] * m_sampleScaleFactor; - *pSampleBuffer++ = buffer[1][i] * m_sampleScaleFactor; + *pSampleBuffer++ = adjustDecodedSample(buffer[0][i], m_bitsPerSample) * m_sampleScaleFactor; + *pSampleBuffer++ = adjustDecodedSample(buffer[1][i], m_bitsPerSample) * m_sampleScaleFactor; } break; } @@ -407,7 +426,7 @@ FLAC__StreamDecoderWriteStatus SoundSourceFLAC::flacWrite( // generic code for multiple channels for (SINT i = 0; i < numWritableFrames; ++i) { for (SINT j = 0; j < channelCount(); ++j) { - *pSampleBuffer++ = buffer[j][i] * m_sampleScaleFactor; + *pSampleBuffer++ = adjustDecodedSample(buffer[j][i], m_bitsPerSample) * m_sampleScaleFactor; } } } From 79f7048a44dced6eccc61555bf02fdb5be02c560 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Wed, 2 May 2018 08:22:17 +0200 Subject: [PATCH 2/3] Delete unused member function --- src/library/dao/settingsdao.cpp | 4 ---- src/library/dao/settingsdao.h | 6 ++---- src/test/schemamanager_test.cpp | 2 -- 3 files changed, 2 insertions(+), 10 deletions(-) diff --git a/src/library/dao/settingsdao.cpp b/src/library/dao/settingsdao.cpp index 2e7f8cb7d1..5c56a98426 100644 --- a/src/library/dao/settingsdao.cpp +++ b/src/library/dao/settingsdao.cpp @@ -5,13 +5,9 @@ SettingsDAO::SettingsDAO(const QSqlDatabase& db) : m_db(db) { -} -SettingsDAO::~SettingsDAO() { -} -void SettingsDAO::initialize() { } QString SettingsDAO::getValue(const QString& name, QString defaultValue) const { diff --git a/src/library/dao/settingsdao.h b/src/library/dao/settingsdao.h index cd0d66bf3f..bcb95acd91 100644 --- a/src/library/dao/settingsdao.h +++ b/src/library/dao/settingsdao.h @@ -12,12 +12,10 @@ // All library-specific preferences go in the library settings table -class SettingsDAO : public QObject { +class SettingsDAO final : public QObject { public: SettingsDAO(const QSqlDatabase& db); - virtual ~SettingsDAO(); - - virtual void initialize(); + ~SettingsDAO() override = default; QString getValue(const QString& name, QString defaultValue = QString()) const; bool setValue(const QString& name, const QVariant& value); diff --git a/src/test/schemamanager_test.cpp b/src/test/schemamanager_test.cpp index bd24f79478..25d9e77fc5 100644 --- a/src/test/schemamanager_test.cpp +++ b/src/test/schemamanager_test.cpp @@ -64,7 +64,6 @@ TEST_F(SchemaManagerTest, BackwardsCompatibleVersion) { EXPECT_EQ(SchemaManager::Result::UpgradeSucceeded, result); SettingsDAO settings(dbConnection()); - settings.initialize(); // Pretend the database version is one past the required version but // min_compatible is the required version. @@ -90,7 +89,6 @@ TEST_F(SchemaManagerTest, BackwardsIncompatibleVersion) { EXPECT_EQ(SchemaManager::Result::UpgradeSucceeded, result); SettingsDAO settings(dbConnection()); - settings.initialize(); // Pretend the database version is one past the required version and // min_compatible is one past the required version. From 71934a9b6fee154de7de0be1a667df0a857a0aa8 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Wed, 2 May 2018 08:23:38 +0200 Subject: [PATCH 3/3] Try to fix crash during database migration in macOS tests --- src/library/dao/settingsdao.cpp | 37 ++++++++++++++++++++++++++++--------- src/library/dao/settingsdao.h | 2 +- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/src/library/dao/settingsdao.cpp b/src/library/dao/settingsdao.cpp index 5c56a98426..148409ab22 100644 --- a/src/library/dao/settingsdao.cpp +++ b/src/library/dao/settingsdao.cpp @@ -3,24 +3,43 @@ #include "library/dao/settingsdao.h" -SettingsDAO::SettingsDAO(const QSqlDatabase& db) - : m_db(db) { +#include "util/logger.h" +#include "util/assert.h" +namespace { +mixxx::Logger kLogger("SettingsDAO"); +} // anonymous namespace + +SettingsDAO::SettingsDAO(const QSqlDatabase& db) + : m_db(db) { } QString SettingsDAO::getValue(const QString& name, QString defaultValue) const { QSqlQuery query(m_db); - query.prepare("SELECT value FROM settings WHERE name = :name"); - query.bindValue(":name", name); - - QString value = defaultValue; - if (query.exec() && query.first()) { - value = query.value(query.record().indexOf("value")).toString(); + if (query.prepare("SELECT value FROM settings WHERE name = :name")) { + query.bindValue(":name", name); + if (query.exec() && query.first()) { + QVariant value = query.value(query.record().indexOf("value")); + VERIFY_OR_DEBUG_ASSERT(value.isValid()) { + kLogger.warning() << "Invalid value:" << value; + } else { + return value.toString(); + } + } + } else { + // Prepare is expected to fail for a fresh database + // when the schema is still empty! + kLogger.debug() + << "Failed to prepare query:" + << "Returning default value" + << defaultValue + << "for" + << name; } - return value; + return defaultValue; } bool SettingsDAO::setValue(const QString& name, const QVariant& value) { diff --git a/src/library/dao/settingsdao.h b/src/library/dao/settingsdao.h index bcb95acd91..91ae8bb401 100644 --- a/src/library/dao/settingsdao.h +++ b/src/library/dao/settingsdao.h @@ -14,7 +14,7 @@ // All library-specific preferences go in the library settings table class SettingsDAO final : public QObject { public: - SettingsDAO(const QSqlDatabase& db); + explicit SettingsDAO(const QSqlDatabase& db); ~SettingsDAO() override = default; QString getValue(const QString& name, QString defaultValue = QString()) const;