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

Bypass mime type lookup from content for .flac files #73

Merged
merged 1 commit into from
Jan 24, 2022
Merged

Bypass mime type lookup from content for .flac files #73

merged 1 commit into from
Jan 24, 2022

Conversation

uklotzde
Copy link

No description provided.

Copy link

@ywwg ywwg left a comment

Choose a reason for hiding this comment

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

This seems good to me -- it seems very unlikely that a ".flac" file would not contain flac data (it's such an uncommon format as it is!)

@ywwg
Copy link

ywwg commented Jan 24, 2022

(leaving unmerged in case @daschuer wants to take a look)

@coveralls
Copy link

Pull Request Test Coverage Report for Build 1740834226

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 29.007%

Totals Coverage Status
Change from base Build 1735970147: 0.02%
Covered Lines: 20678
Relevant Lines: 71286

💛 - Coveralls

@daschuer
Copy link
Owner

I am afraid all media types can have an ID3 header. How about falling back to the "file type by extention" approach for to all ID3 header files?

That should be a safer route, that is easy to tweak in case.

A full solution would be to lookup the mime type behind the ID3 header.

We may also limit the extention based approach to soundsources known to be able to skip an ID3 header.

This is not a request on this PR, I am just asking for your opinions.

@uklotzde
Copy link
Author

uklotzde commented Jan 24, 2022

Let's defer extended workarounds until bug reports arrive, step by step. I consider everything else over-engineering at this point.

@daschuer daschuer merged commit 212489b into daschuer:soundsource-file-type Jan 24, 2022
@uklotzde uklotzde deleted the soundsource-file-type branch January 31, 2022 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants