-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
lp1445885: Fix handling of files with wrong suffix #4615
Changes from 21 commits
71f45b5
4853a3a
699a490
74d7632
703fbfb
00d4f9a
eabfcca
5937340
5bfa42c
e40fab9
adf9faf
5b500ee
49df712
f57cf30
db15d6f
8fb2185
cb52de8
9b2518e
f63a72b
d3b504d
2a44a94
3ed94d0
babfce5
2f77c85
212489b
8a9b24b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
#include <QMimeDatabase> | ||
#include <QMimeType> | ||
|
||
#include "sources/soundsourceproxy.h" | ||
#include "util/logger.h" | ||
|
||
namespace mixxx { | ||
|
@@ -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 | ||
|
@@ -45,45 +32,40 @@ 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 == "opus") { | ||
// opus has mime type "audio/ogg" which will be decoded with the SoundSourceOggVobis() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is because "ogg" is a container and "vorbis" is the compression algorithm. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the OGG container can include Speex, Vorbis, Opus, FLAC, OggPCM.
|
||
// but we want SoundSourceOpus() | ||
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) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nit-pick: QLatin1String("opus")?
QString provides many QLatin1String overloads as I only recently noticed.