Skip to content

Commit

Permalink
Merge pull request #4615 from daschuer/soundsource-file-type
Browse files Browse the repository at this point in the history
 lp1445885: Fix handling of files with wrong suffix
  • Loading branch information
uklotzde authored Jan 25, 2022
2 parents 75af4e2 + 8a9b24b commit b0a7c9e
Show file tree
Hide file tree
Showing 41 changed files with 533 additions and 316 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@
* SoundSourceMP3: Log recoverable errors as info instead of warning [#4365](https://github.com/mixxxdj/mixxx/pull/4365)
* Fix type detection of AIFF files [#4364](https://github.com/mixxxdj/mixxx/pull/4364)
* AAC encoder: Fixed a memory leak [#4386](https://github.com/mixxxdj/mixxx/pull/4386) [#4408](https://github.com/mixxxdj/mixxx/pull/4408)
* Improve robustness of file type detection by considering the actual MIME type of the content. [lp:1445885](https://bugs.launchpad.net/mixxx/+bug/1445885) [#4356](https://github.com/mixxxdj/mixxx/pull/4356) [#4357](https://github.com/mixxxdj/mixxx/pull/4357)

### Audio Engine

Expand Down
6 changes: 6 additions & 0 deletions res/linux/org.mixxx.Mixxx.metainfo.xml
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,12 @@
#4386
#4408
</li>
<li>
Improve robustness of file type detection by considering the actual MIME type of the content.
lp:1445885
#4356
#4357
</li>
</ul>
<p>
Audio Engine
Expand Down
11 changes: 6 additions & 5 deletions src/coreservices.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -399,16 +399,17 @@ void CoreServices::initialize(QApplication* pApp) {
QSet<QString>::fromList(prev_plugins_list);
#endif

const QList<QString> curr_plugins_list = SoundSourceProxy::getSupportedFileExtensions();
QSet<QString> curr_plugins =
const QList<QString> supportedFileSuffixes = SoundSourceProxy::getSupportedFileSuffixes();
auto curr_plugins =
#if QT_VERSION >= QT_VERSION_CHECK(5, 14, 0)
QSet<QString>(curr_plugins_list.begin(), curr_plugins_list.end());
QSet<QString>(supportedFileSuffixes.begin(), supportedFileSuffixes.end());
#else
QSet<QString>::fromList(curr_plugins_list);
QSet<QString>::fromList(supportedFileSuffixes);
#endif

rescan = rescan || (prev_plugins != curr_plugins);
pConfig->set(ConfigKey("[Library]", "SupportedFileExtensions"), curr_plugins_list.join(","));
pConfig->set(ConfigKey("[Library]", "SupportedFileExtensions"),
supportedFileSuffixes.join(","));

// Scan the library directory. Do this after the skinloader has
// loaded a skin, see Bug #1047435
Expand Down
14 changes: 10 additions & 4 deletions src/sources/metadatasourcetaglib.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,14 +86,20 @@ class AiffFile : public TagLib::RIFF::AIFF::File {

std::pair<MetadataSourceTagLib::ImportResult, QDateTime>
MetadataSourceTagLib::afterImport(ImportResult importResult) const {
return std::make_pair(importResult,
MetadataSource::getFileSynchronizedAt(QFile(m_fileName)));
const auto sourceSynchronizedAt =
MetadataSource::getFileSynchronizedAt(QFile(m_fileName));
DEBUG_ASSERT(sourceSynchronizedAt.isValid() ||
importResult != ImportResult::Succeeded);
return std::make_pair(importResult, sourceSynchronizedAt);
}

std::pair<MetadataSourceTagLib::ExportResult, QDateTime>
MetadataSourceTagLib::afterExport(ExportResult exportResult) const {
return std::make_pair(exportResult,
MetadataSource::getFileSynchronizedAt(QFile(m_fileName)));
const auto sourceSynchronizedAt =
MetadataSource::getFileSynchronizedAt(QFile(m_fileName));
DEBUG_ASSERT(sourceSynchronizedAt.isValid() ||
exportResult != ExportResult::Succeeded);
return std::make_pair(exportResult, sourceSynchronizedAt);
}

std::pair<MetadataSource::ImportResult, QDateTime>
Expand Down
81 changes: 38 additions & 43 deletions src/sources/soundsource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <QMimeDatabase>
#include <QMimeType>

#include "sources/soundsourceproxy.h"
#include "util/logger.h"

namespace mixxx {
Expand All @@ -21,20 +22,6 @@ inline QUrl validateLocalFileUrl(QUrl url) {
return url;
}

inline QString fileTypeFromSuffix(const QString& suffix) {
const QString fileType = suffix.toLower().trimmed();
if (fileType.isEmpty()) {
// Always return a default-constructed, null string instead
// of an empty string which might either be null or "".
return QString{};
}
// Map shortened suffix "aif" to "aiff" for disambiguation
if (fileType == QStringLiteral("aif")) {
return QStringLiteral("aiff");
}
return fileType;
}

} // anonymous namespace

//static
Expand All @@ -45,45 +32,53 @@ QString SoundSource::getTypeFromUrl(const QUrl& url) {

//static
QString SoundSource::getTypeFromFile(const QFileInfo& fileInfo) {
const QString fileSuffix = fileInfo.suffix();
const QString fileType = fileTypeFromSuffix(fileSuffix);
DEBUG_ASSERT(!fileType.isEmpty() || fileType == QString{});
const QMimeType mimeType = QMimeDatabase().mimeTypeForFile(
const QString fileSuffix = fileInfo.suffix().toLower().trimmed();

if (fileSuffix == QLatin1String("opus")) {
// Bypass the insufficient mime type lookup from content for opus files
// Files with "opus" suffix are of mime type "audio/x-opus+ogg" or "audio/opus".
// In case of "audio/x-opus+ogg" only the container format "audio/ogg"
// is detected, which will be decoded with the SoundSourceOggVobis()
// but we want SoundSourceOpus(). "audio/opus" files without ogg
// container are detected as "text/plain". They are not yet supported by Mixxx.
return fileSuffix;
}
if (fileSuffix == QLatin1String("flac")) {
// Bypass the insufficient mime type lookup from content for FLAC files.
// Legacy FLAC files may contain an ID3 tag (written by ExactAudioCopy and
// others) that causes these files to be identified as "audio/mpeg" instead
// of "audio/flac". Most decoders and TagLib are able to ignore and skip
// the non-standard ID3 data.
// https://mixxx.zulipchat.com/#narrow/stream/109171-development/topic/mimetype.20sometimes.20wrong
return fileSuffix;
}

QMimeType mimeType = QMimeDatabase().mimeTypeForFile(
fileInfo, QMimeDatabase::MatchContent);
// According to the documentation mimeTypeForFile always returns a valid
// type, using the generic type application/octet-stream as a fallback.
// This might also occur for missing files as seen on Qt 5.12.
if (!mimeType.isValid() ||
mimeType.name() == QStringLiteral("application/octet-stream")) {
qWarning()
<< "Unknown MIME type for file" << fileInfo.filePath();
return fileType;
}
const QString preferredSuffix = mimeType.preferredSuffix();
if (preferredSuffix.isEmpty()) {
DEBUG_ASSERT(mimeType.suffixes().isEmpty());
qInfo()
<< "MIME type" << mimeType
<< "has no preferred suffix";
return fileType;
}
const QString preferredFileType = fileTypeFromSuffix(preferredSuffix);
if (fileType == preferredFileType || mimeType.suffixes().contains(fileSuffix)) {
return fileType;
if (!mimeType.isValid() || mimeType.isDefault()) {
qInfo() << "Unable to detect MIME type from file" << fileInfo.filePath();
mimeType = QMimeDatabase().mimeTypeForFile(
fileInfo, QMimeDatabase::MatchExtension);
if (!mimeType.isValid() || mimeType.isDefault()) {
return fileSuffix;
}
}
const QString fileType = SoundSourceProxy::getFileTypeByMimeType(mimeType);
if (fileType.isEmpty()) {
qWarning() << "No file type registered for MIME type" << mimeType;
return fileSuffix;
}
if (fileType != fileSuffix && !mimeType.suffixes().contains(fileSuffix)) {
qWarning()
<< "Using type" << preferredFileType
<< "according to the detected MIME type" << mimeType
<< "of file" << fileInfo.filePath();
} else {
qWarning()
<< "Using type" << preferredFileType
<< "instead of" << fileType
<< "Using type" << fileType
<< "instead of" << fileSuffix
<< "according to the detected MIME type" << mimeType
<< "of file" << fileInfo.filePath();
}
return preferredFileType;
return fileType;
}

SoundSource::SoundSource(const QUrl& url, const QString& type)
Expand Down
6 changes: 3 additions & 3 deletions src/sources/soundsourcecoreaudio.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ constexpr SINT kMp3MaxSeekPrefetchFrames =
const QString SoundSourceProviderCoreAudio::kDisplayName = QStringLiteral("Apple Core Audio");

//static
const QStringList SoundSourceProviderCoreAudio::kSupportedFileExtensions = {
const QStringList SoundSourceProviderCoreAudio::kSupportedFileTypes = {
QStringLiteral("aac"),
QStringLiteral("m4a"),
QStringLiteral("mp4"),
Expand All @@ -41,8 +41,8 @@ const QStringList SoundSourceProviderCoreAudio::kSupportedFileExtensions = {
};

SoundSourceProviderPriority SoundSourceProviderCoreAudio::getPriorityHint(
const QString& supportedFileExtension) const {
Q_UNUSED(supportedFileExtension)
const QString& supportedFileType) const {
Q_UNUSED(supportedFileType)
// On macOS SoundSourceCoreAudio is the preferred decoder for all
// supported audio formats.
return SoundSourceProviderPriority::Higher;
Expand Down
8 changes: 4 additions & 4 deletions src/sources/soundsourcecoreaudio.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,18 +55,18 @@ class SoundSourceCoreAudio
class SoundSourceProviderCoreAudio : public SoundSourceProvider {
public:
static const QString kDisplayName;
static const QStringList kSupportedFileExtensions;
static const QStringList kSupportedFileTypes;

QString getDisplayName() const override {
return kDisplayName;
}

QStringList getSupportedFileExtensions() const override {
return kSupportedFileExtensions;
QStringList getSupportedFileTypes() const override {
return kSupportedFileTypes;
}

SoundSourceProviderPriority getPriorityHint(
const QString& supportedFileExtension) const override;
const QString& supportedFileType) const override;

SoundSourcePointer newSoundSource(const QUrl& url) override {
return newSoundSourceFromUrl<SoundSourceCoreAudio>(url);
Expand Down
21 changes: 10 additions & 11 deletions src/sources/soundsourceffmpeg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ SoundSourceProviderFFmpeg::SoundSourceProviderFFmpeg() {
std::call_once(initFFmpegLibFlag, initFFmpegLib);
}

QStringList SoundSourceProviderFFmpeg::getSupportedFileExtensions() const {
QStringList SoundSourceProviderFFmpeg::getSupportedFileTypes() const {
QStringList list;
QStringList disabledInputFormats;

Expand All @@ -365,27 +365,26 @@ QStringList SoundSourceProviderFFmpeg::getSupportedFileExtensions() const {
list.append("aac");
continue;
} else if (!strcmp(pavInputFormat->name, "aiff")) {
list.append("aif");
list.append("aiff");
continue;
} else if (!strcmp(pavInputFormat->name, "mp3")) {
list.append("mp3");
continue;
} else if (!strcmp(pavInputFormat->name, "mp4")) {
} else if (!strcmp(pavInputFormat->name, "mp4") ||
!strcmp(pavInputFormat->name, "m4v")) {
list.append("mp4");
continue;
} else if (!strcmp(pavInputFormat->name, "m4v")) {
list.append("m4v");
continue;
} else if (!strcmp(pavInputFormat->name, "mov,mp4,m4a,3gp,3g2,mj2")) {
} else if (!strcmp(pavInputFormat->name,
"mov,mp4,m4a,3gp,3g2,mj2")) {
list.append("mov");
list.append("mp4");
list.append("m4a");
list.append("3gp");
list.append("3g2");
list.append("mj2");
continue;
} else if (!strcmp(pavInputFormat->name, "opus") || !strcmp(pavInputFormat->name, "libopus")) {
} else if (!strcmp(pavInputFormat->name, "opus") ||
!strcmp(pavInputFormat->name, "libopus")) {
list.append("opus");
continue;
} else if (!strcmp(pavInputFormat->name, "wav")) {
Expand Down Expand Up @@ -453,10 +452,10 @@ QStringList SoundSourceProviderFFmpeg::getSupportedFileExtensions() const {
}

SoundSourceProviderPriority SoundSourceProviderFFmpeg::getPriorityHint(
const QString& supportedFileExtension) const {
Q_UNUSED(supportedFileExtension)
const QString& supportedFileType) const {
Q_UNUSED(supportedFileType)
// TODO: Increase priority to Default or even Higher for all
// supported and tested file extension?
// supported and tested file types?
// Currently it is only used as a fallback after all other
// SoundSources failed to open a file or are otherwise unavailable.
return SoundSourceProviderPriority::Lowest;
Expand Down
4 changes: 2 additions & 2 deletions src/sources/soundsourceffmpeg.h
Original file line number Diff line number Diff line change
Expand Up @@ -197,10 +197,10 @@ class SoundSourceProviderFFmpeg : public SoundSourceProvider {
return kDisplayName;
}

QStringList getSupportedFileExtensions() const override;
QStringList getSupportedFileTypes() const override;

SoundSourceProviderPriority getPriorityHint(
const QString& supportedFileExtension) const override;
const QString& supportedFileType) const override;

SoundSourcePointer newSoundSource(const QUrl& url) override {
return newSoundSourceFromUrl<SoundSourceFFmpeg>(url);
Expand Down
6 changes: 3 additions & 3 deletions src/sources/soundsourceflac.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,13 @@ constexpr SINT kBitsPerSampleDefault = 0;
const QString SoundSourceProviderFLAC::kDisplayName = QStringLiteral("Xiph.org libFLAC");

//static
const QStringList SoundSourceProviderFLAC::kSupportedFileExtensions = {
const QStringList SoundSourceProviderFLAC::kSupportedFileTypes = {
QStringLiteral("flac"),
};

SoundSourceProviderPriority SoundSourceProviderFLAC::getPriorityHint(
const QString& supportedFileExtension) const {
Q_UNUSED(supportedFileExtension)
const QString& supportedFileType) const {
Q_UNUSED(supportedFileType)
// This reference decoder is supposed to produce more accurate
// and reliable results than any other DEFAULT provider.
return SoundSourceProviderPriority::Higher;
Expand Down
8 changes: 4 additions & 4 deletions src/sources/soundsourceflac.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,18 +59,18 @@ class SoundSourceFLAC final : public SoundSource {
class SoundSourceProviderFLAC : public SoundSourceProvider {
public:
static const QString kDisplayName;
static const QStringList kSupportedFileExtensions;
static const QStringList kSupportedFileTypes;

QString getDisplayName() const override {
return kDisplayName;
}

QStringList getSupportedFileExtensions() const override {
return kSupportedFileExtensions;
QStringList getSupportedFileTypes() const override {
return kSupportedFileTypes;
}

SoundSourceProviderPriority getPriorityHint(
const QString& supportedFileExtension) const override;
const QString& supportedFileType) const override;

SoundSourcePointer newSoundSource(const QUrl& url) override {
return newSoundSourceFromUrl<SoundSourceFLAC>(url);
Expand Down
6 changes: 3 additions & 3 deletions src/sources/soundsourcem4a.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -189,14 +189,14 @@ inline bool startsWithADTSHeader(
const QString SoundSourceProviderM4A::kDisplayName = QStringLiteral("Nero FAAD2");

//static
const QStringList SoundSourceProviderM4A::kSupportedFileExtensions = {
const QStringList SoundSourceProviderM4A::kSupportedFileTypes = {
QStringLiteral("m4a"),
QStringLiteral("mp4"),
};

QStringList SoundSourceProviderM4A::getSupportedFileExtensions() const {
QStringList SoundSourceProviderM4A::getSupportedFileTypes() const {
if (faad2::LibLoader::Instance()->isLoaded()) {
return kSupportedFileExtensions;
return kSupportedFileTypes;
} else {
return QStringList(); // none available
}
Expand Down
4 changes: 2 additions & 2 deletions src/sources/soundsourcem4a.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,13 @@ class SoundSourceM4A : public SoundSource {
class SoundSourceProviderM4A : public SoundSourceProvider {
public:
static const QString kDisplayName;
static const QStringList kSupportedFileExtensions;
static const QStringList kSupportedFileTypes;

QString getDisplayName() const override {
return kDisplayName;
}

QStringList getSupportedFileExtensions() const override;
QStringList getSupportedFileTypes() const override;

SoundSourcePointer newSoundSource(const QUrl& url) override;
};
Expand Down
6 changes: 3 additions & 3 deletions src/sources/soundsourcemediafoundation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,16 @@ const QString SoundSourceProviderMediaFoundation::kDisplayName =
QStringLiteral("Microsoft Media Foundation");

//static
const QStringList SoundSourceProviderMediaFoundation::kSupportedFileExtensions = {
const QStringList SoundSourceProviderMediaFoundation::kSupportedFileTypes = {
QStringLiteral("aac"),
QStringLiteral("m4a"),
QStringLiteral("m4v"),
QStringLiteral("mp4"),
};

SoundSourceProviderPriority SoundSourceProviderMediaFoundation::getPriorityHint(
const QString& supportedFileExtension) const {
Q_UNUSED(supportedFileExtension)
const QString& supportedFileType) const {
Q_UNUSED(supportedFileType)
// On Windows SoundSourceMediaFoundation is the preferred decoder for all
// supported audio formats.
return SoundSourceProviderPriority::Higher;
Expand Down
Loading

0 comments on commit b0a7c9e

Please sign in to comment.