-
Notifications
You must be signed in to change notification settings - Fork 91
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
switch CDDB implementation to freedb.py from python-audio-tools #276
Conversation
License: GPL-2.0+
The removed functions depend on other classes of python-audio-tools, but aren't needed here.
The last release of cddb-py was 15 years ago. The freedb code taken from python-audio-tools speaks protocol version 6 (Unicode) and is compatible to both Python 2 and 3.
I suspect it'd be a better long term solution to package freedb.py as its own library (can be used by more than whipper, commits to sync with upstream won't have to show up in our commit history, etc), but barring that this approach is acceptable. Merging is not contingent on a positive answer, but were people having issues with cddb-py not supporting protocol version 6? |
I don't know of any issues with the old implementation besides its unavailability from pip. |
I commented on upstream, asking about them packaging this for PyPI: tuffy/python-audio-tools#33 (comment) |
@mtdcr Is there any reason the module couldn't work if imported as |
IIRC, xmcd_metadata returns a stripped-down version of the response formatted in a way specific to audiotools. The module also identifies itself as "audiotools" to cddb servers, which wouldn't be polite if used by whipper. I'm not arguing against distribution of audiotools via pypi, but I guess whipper's use of freedb.py is not a good argument for it either. freedb.py is just a small part of audiotools. To let freedb.py be useful for whipper or other projects, I believe that audiotools would have to solve both problems mentioned above, e.g. by creating a second interface to get access to the raw response and by allowing to set a custom application name and version. |
Source: https://github.com/tuffy/python-audio-tools/blob/master/audiotools/freedb.py (GPL-2.0+)
The last release of cddb-py was 15 years ago. The freedb code taken from python-audio-tools speaks protocol version 6 (Unicode) and is compatible to both Python 2 and 3.
The first motivation to get rid of cddb-py was its unavailability in pip. Python-audio-tools isn't there either, and its freedb code can't be used as an independent library, but as the source code looks small and clean and comes with a matching license, I guess it's sane to just take the code from there and maintain it inside whipper.