Skip to content

Conversation

@exislow
Copy link
Contributor

@exislow exislow commented Dec 16, 2024

No description provided.

Copy link
Collaborator

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.

Copy link
Contributor Author

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

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:
Copy link
Collaborator

Choose a reason for hiding this comment

The 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...

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

@tehkillerbee
Copy link
Collaborator

Thanks for the PR. Everything LGTM. Merging...

@tehkillerbee tehkillerbee merged commit 270d8ae into EbbLabs:master Dec 18, 2024
5 checks passed
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.

2 participants