-
Notifications
You must be signed in to change notification settings - Fork 111
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
✨ Extended file extension detection to properly detect M4A files. Fixes tamland/python-tidal#309 #310
✨ Extended file extension detection to properly detect M4A files. Fixes tamland/python-tidal#309 #310
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -620,7 +620,11 @@ def get_file_extension(stream_url: str, stream_codec: Optional[str] = None) -> s | |
if AudioExtensions.FLAC in stream_url: | ||
# If the file extension within the URL is '*.flac', this is simply a FLAC file. | ||
result: str = AudioExtensions.FLAC | ||
elif AudioExtensions.MP4 in stream_url: | ||
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. Looks like this shouldn't break any tests but perhaps a good idea to check anyways. Also, it would be a good idea to extend the existing tests to check that M4A extension does indeed return Codec.MP4A... 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. This is an example, which returns a "m4a" extension: https://tidal.com/browse/track/65119559 Tests are always a good idea. I can do some testing work, as soon as I have finished my personal issues with my project. 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. Added you a test for M4A file extension. To be honest: Someone could also add a lot more tests, but this takes a lot of time. Tell me, if I can help you with something else. Otherwise I would be happy about a merge and hotfix release. |
||
elif ( | ||
AudioExtensions.MP4 in stream_url | ||
or AudioExtensions.M4A in stream_url | ||
or stream_codec == Codec.MP4A | ||
): | ||
# MPEG-4 is simply a container format for different audio / video encoded lines, like FLAC, AAC, M4A etc. | ||
# '*.m4a' is usually used as file extension, if the container contains only audio lines | ||
# See https://en.wikipedia.org/wiki/MP4_file_format | ||
|
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.
I assume changes to this file was accidentally committed. Please remove it from this MR, then I will update it separately.
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.
Thanks, my bad. I have reverted the changes