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

Soundsource filetype detection fix #4602

Merged
merged 2 commits into from
Jan 6, 2022

Conversation

Holzhaus
Copy link
Member

@Holzhaus Holzhaus commented Jan 5, 2022

Continuation of #4600 for the main branch.

Proposed changelog entry

- Fix file type detection when file has wrong file extension by determining the MIME type from content #4602

@Holzhaus Holzhaus added sources minor bug changelog This PR should be included in the changelog labels Jan 5, 2022
@Holzhaus Holzhaus added this to the 2.4.0 milestone Jan 5, 2022
@Holzhaus Holzhaus requested a review from uklotzde January 5, 2022 22:09
If a file is named `foo.wav` but actually contains MP3 data,
`SoundSource::getTypeFromFile` should return "mp3", not "wav".
This behavior is expected and already tested in
`SoundSourceProxyTest.getTypeFromFile`, but due to a bug in our test
file creation code, the test operated on a wrong file name and passed
although the behavior was broken and the function would just return
"wav" in the example above.

The reason for this is that QMimeDatabase only looks at the file name when
the file suffix is misleading, and thus cannot detect that the file is
actually an MP3 file:

> The default matching algorithm looks at both the file name and the
> file contents, if necessary. The file extension has priority over the
> contents, but the contents will be used if the file extension is
> unknown, or matches multiple MIME types.
>
> Source: https://doc.qt.io/qt-5/qmimedatabase.html#mimeTypeForFile

This commit fixes `SoundSource::getTypeFromFile` to work as expected by using
the file *content* instead of the file name for determining the file
type.
This reverts commit 6690063 because the
underlying issue (i.e. SoundSource::getTypeFromFile() not detecting the
correct file type if the file has a known but misleading file extension)
has been fixed in commit c376317.
Hence, the check can be re-enabled.
@Holzhaus Holzhaus force-pushed the soundsource-filetype-detection-fix branch from 80b53a6 to 17a4b16 Compare January 6, 2022 10:24
@Holzhaus Holzhaus marked this pull request as ready for review January 6, 2022 10:25
Copy link
Contributor

@uklotzde uklotzde left a comment

Choose a reason for hiding this comment

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

Thank you for sorting this out by fixing my mistakes! LGTM

@uklotzde uklotzde merged commit bdf0978 into mixxxdj:main Jan 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog This PR should be included in the changelog code quality minor bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants