jonnyjd / python-discid Goto Github PK
View Code? Open in Web Editor NEWlibdiscid python binding
Home Page: https://python-discid.readthedocs.org/
License: GNU Lesser General Public License v3.0
libdiscid python binding
Home Page: https://python-discid.readthedocs.org/
License: GNU Lesser General Public License v3.0
It would be nice if the user was asked to re-enter username and password if musicbrainzngs throws an AuthenticationError.
In general this binding works like libdiscid works, which has a documented API at http://jonnyjd.github.com/libdiscid/.
However, that one is probably confusing for a python developer and things work a bit different in our binding.
So a library without documentation doesn't help at all.
That is only partly true:
import discid
help(discid)
is somewhat of a documentation. Though we have no docstrings yet and many private members are shown.
Requested in JonnyJD/musicbrainz-isrcsubmit#42
That obviously would make use of DiscId.put()
, but we might even implement gathering the data from the CUE files in the discid module.
That would be the first function that is not just forwarded to libdiscid.
Hi, could you make the doc build with more recent versions of sphinx?
Running Sphinx v1.3.5
making output directory...
Extension error:
Could not import extension ext.data_doc (exception: cannot import name 'safe_repr')
Apparently safe_repr
was replaced by object_description
but I'm not even sure the data_doc extension
is needed (I tried building with a fixed extension and without the extension and the doc seems identical).
Hi @JonnyJD!
I'm currently packaging python-discid for Arch Linux. Could you please add the public key for 68304838CC686607
to your account so that the tags show up as verified? Thanks! :)
On macOS when loading libdiscid and libdiscid.dylib is both available system wide (e.g. in /usr/local/) and local to the application, the system wide install is preferred.
However, especially when running from a bundled app it might be preferable to use the bundled version to be sure to get exactly the expected version.
Having the local version being preferred seems to have been the intention of #16, however it does not currently work this way.
Picard is affected by this, see https://tickets.metabrainz.org/browse/PICARD-1653
On current master (601811c) Cygwin (Python 2.6.8) fails the examples with:
$ ./examples.py
Traceback (most recent call last):
File "./examples.py", line 6, in <module>
import discid
ImportError: dynamic module does not define init function (initdiscid)
That one should already go in the base API, because this is what we would need to submit DiscIds, obviously.
Packaging 0.2.0 for ubuntu breaks because the tests gave a syntax error:
python3.2 setup.py test -vv
running test
Traceback (most recent call last):
File "setup.py", line 64, in <module>
"Topic :: Software Development :: Libraries :: Python Modules"
File "/usr/lib/python3.2/distutils/core.py", line 148, in setup
dist.run_commands()
File "/usr/lib/python3.2/distutils/dist.py", line 917, in run_commands
self.run_command(cmd)
File "/usr/lib/python3.2/distutils/dist.py", line 936, in run_command
cmd_obj.run()
File "setup.py", line 36, in run
suite = unittest.defaultTestLoader.loadTestsFromNames(self.names)
File "/usr/lib/python3.2/unittest/loader.py", line 137, in loadTestsFromNames
suites = [self.loadTestsFromName(name, module) for name in names]
File "/usr/lib/python3.2/unittest/loader.py", line 137, in <listcomp>
suites = [self.loadTestsFromName(name, module) for name in names]
File "/usr/lib/python3.2/unittest/loader.py", line 96, in loadTestsFromName
module = __import__('.'.join(parts_copy))
File "/tmp/buildd/python-discid-0.2.0/test_discid.py", line 19
self.assertTrue(type(discid._decode(b"test")) is type(u"test"))
^
SyntaxError: invalid syntax
I would consider not to have a DEFAULT_DEVICE constant but rather do an actual function call. Even though the default device is at the moment mostly a constant in libdiscid, it might be something which changes on runtime.
I am not entirely sure how OSX handles the devices, but isn't it the case there that the devices change on runtime when the users inserts / removes a disc? At least https://github.com/metabrainz/libdiscid/blob/master/src/disc_darwin.c#L155 suggest this and Picard has issues with it. Even on other systems that could happen when the user attaches a portable drive, even though dynamic detection of the device is currently not supported by libdiscid on those platforms.
Hi!
From a report in https://tickets.metabrainz.org/browse/MBS-10050 - it seems that python-discid generated an invalid TOC. Thought I'd report it here in case it's an unknown issue.
The current Picard build (1.3) doesn't work with discid support when packaged with py2app 0.9.
See:
https://gist.github.com/mayhem/e8c3172feca887cd7c2d
http://chatlogs.musicbrainz.org/musicbrainz-devel/2014/2014-10/2014-10-20.html#T22-24-22-939717
The stacktrace is:
File "discid/libdiscid.pyo", line 93, in _open_library
10/21/14 12:23:15.925 AM [0x0-0x645645].org.musicbrainz.picard: File "ctypes/__init__.pyo", line 443, in LoadLibrary
10/21/14 12:23:15.925 AM [0x0-0x645645].org.musicbrainz.picard: File "ctypes/__init__.pyo", line 365, in __init__
10/21/14 12:23:15.925 AM [0x0-0x645645].org.musicbrainz.picard: OSError: dlopen(libdiscid.0.dylib, 6): image not found
While the dylib is in /Applications/MusicBrainz Picard.app/Contents/Frameworks/libdiscid.0.dylib
Functions like test_which, has_program and gather_isrcs never close devnull. Thus the leak file descriptors. This shouldn't be a problem since isrcsubmit does not open that many file descriptors. However, it would be cleaner to just surround the appropriate places with something like with open(os.devnull, "w") as devnull:
. (There's no need to add from __future__ import with_statement
since Python 2.5 isn't supported anyway.)
This causes issues, as e.g. Picard might end up using a version of discid.dll not shiped with Picard.
In my case I had an older discid.dll in C:\bin (which is in my path), and in the Picard installation folder is the newer discid.dll version 0.6.1 shipped with the installer.
Is this a general ctypes issue or can this be influenced? Especially on Windows a local version should be preferred.
import discid
help(discid)
Lists a lot of stuff we don't actually want to expose.
The problem is our from ctypes import *
I would suggest not to expose discid_get_webservice_url.
I consider discid_get_webservice_url broken by design. We can't remove it from libdiscid, but there is no need to expose it in any bindings :)
Just for clarification:
discid_get_webservice_url adds a dependency on an external service (MB ws/1) which could change at any time. For me it is unclear how libdiscid should deal with changes to the webservice (hi there ws/2). E.g. you can't just point discid_get_webservice_url to the ws/2 URL since this would break the API for any software actually using discid_get_webservice_url in a meaningful way (that is using the URL to load the XML and parse it).
One could just deprecate discid_get_webservice_url and add discid_get_webservice_url2, but that method will again have the same issues in the future.
Introduced with 601811c
We don't set prototypes and just cast the returned value to the correct one.
This worked well for everything, up until get_default_device:
On 64 bit Linux c_device.value
gives a Segmentation Fault.
On 32 bit Windows this works fine.
This is somewhat related to #30, but different in my eyes.
DEFAULT_DEVICE or get_default_device() (depending on outcome of #30) is about the actual default and should stay as a feature due to haveing discid_get_default_device() returning a single string in libdiscid.
get_devices()
would get a list of currently (or in general) available devices, which probably should include the one default.
This function is basically implemented in https://github.com/musicbrainz/picard/blob/master/picard/util/cdrom.py (as a list of currently available devices) for Windows and Linux.
An implementation for Mac OS X is partly implemented in isrcsubmit:
https://github.com/JonnyJD/musicbrainz-isrcsubmit/blob/bdb8efc2a6819d791dce198879bde34846c95d07/isrcsubmit.py#L301 (using drutil)
Part of this is implemented in libdiscid for Mac (for default_device) and proposed for Windows:
metabrainz/libdiscid#20.
Hi! When packaging 1.2.0 for Arch Linux I used pytest to run the test suite (i.e. pytest -v
). The way of running tests via setuptools (i.e. python setup.py test
) is soon going to be deprecated hence I'm rather switching earlier than later ;-)
Unfortunately a few of the tests seem to have defaults set that can not be met (also because they assume that a cdrom drive is available. We run packaging tools in pristine systemd-nspawn containers which is why there will never be a cdrom drive though.
============================= test session starts ==============================
platform linux -- Python 3.8.5, pytest-6.0.1, py-1.9.0, pluggy-0.13.1 -- /usr/bin/python
cachedir: .pytest_cache
rootdir: /build/python-discid/src/python-discid-1.2.0
collecting ... collected 16 items
test_discid.py::TestModulePrivate::test_decode PASSED [ 6%]
test_discid.py::TestModulePrivate::test_encode PASSED [ 12%]
test_discid.py::TestModulePrivate::test_encoding PASSED [ 18%]
test_discid.py::TestModule::test_default_device PASSED [ 25%]
test_discid.py::TestModule::test_device_encoding PASSED [ 31%]
test_discid.py::TestModule::test_features PASSED [ 37%]
test_discid.py::TestModule::test_features_implemented PASSED [ 43%]
test_discid.py::TestModule::test_invalid_device PASSED [ 50%]
test_discid.py::TestModule::test_put_fail PASSED [ 56%]
test_discid.py::TestModule::test_put_success PASSED [ 62%]
test_discid.py::TestModule::test_version_string PASSED [ 68%]
test_discid.py::TestDisc::test_default_device PASSED [ 75%]
test_discid.py::TestDisc::test_features PASSED [ 81%]
test_discid.py::TestDisc::test_read_features FAILED [ 87%]
test_discid.py::TestDisc::test_read_put FAILED [ 93%]
test_discid.py::TestDisc::test_read_simple FAILED [100%]
=================================== FAILURES ===================================
_________________________ TestDisc.test_read_features __________________________
self = <test_discid.TestDisc testMethod=test_read_features>
def test_read_features(self):
> disc = discid.read(features=["mcn", "isrc"]) # read from default drive
test_discid.py:178:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
discid/disc.py:55: in read
disc.read(device, features)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
self = <discid.disc.Disc object at 0x6bc3e9d7f970>, device = None
features = ['mcn', 'isrc']
def read(self, device=None, features=[]):
"""Reads the TOC from the device given as string
The user is supposed to use :func:`discid.read`.
"""
if "read" not in FEATURES:
raise NotImplementedError("discid_read not implemented on platform")
# only use features implemented on this platform and in this module
self._requested_features = list(set(features) & set(FEATURES)
& set(FEATURES_IMPLEMENTED))
# create the bitmask for libdiscid
c_features = 0
for feature in features:
c_features += _FEATURE_MAPPING.get(feature, 0)
# device = None will use the default device (internally)
try:
result = _LIB.discid_read_sparse(self._handle, _encode(device),
c_features) == 1
except AttributeError:
result = _LIB.discid_read(self._handle, _encode(device)) == 1
self._success = result
if not self._success:
> raise DiscError(self._get_error_msg())
E discid.disc.DiscError: cannot open device `/dev/cdrom'
discid/disc.py:152: DiscError
____________________________ TestDisc.test_read_put ____________________________
self = <test_discid.TestDisc testMethod=test_read_put>
def test_read_put(self):
# a read followed with a put, which should clear the features
> disc = discid.read(features=["mcn", "isrc"]) # read from default drive
test_discid.py:196:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
discid/disc.py:55: in read
disc.read(device, features)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
self = <discid.disc.Disc object at 0x6bc3e9d0c700>, device = None
features = ['mcn', 'isrc']
def read(self, device=None, features=[]):
"""Reads the TOC from the device given as string
The user is supposed to use :func:`discid.read`.
"""
if "read" not in FEATURES:
raise NotImplementedError("discid_read not implemented on platform")
# only use features implemented on this platform and in this module
self._requested_features = list(set(features) & set(FEATURES)
& set(FEATURES_IMPLEMENTED))
# create the bitmask for libdiscid
c_features = 0
for feature in features:
c_features += _FEATURE_MAPPING.get(feature, 0)
# device = None will use the default device (internally)
try:
result = _LIB.discid_read_sparse(self._handle, _encode(device),
c_features) == 1
except AttributeError:
result = _LIB.discid_read(self._handle, _encode(device)) == 1
self._success = result
if not self._success:
> raise DiscError(self._get_error_msg())
E discid.disc.DiscError: cannot open device `/dev/cdrom'
discid/disc.py:152: DiscError
__________________________ TestDisc.test_read_simple ___________________________
self = <test_discid.TestDisc testMethod=test_read_simple>
def test_read_simple(self):
> disc = discid.read() # read from default drive
test_discid.py:123:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
discid/disc.py:55: in read
disc.read(device, features)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
self = <discid.disc.Disc object at 0x6bc3e9cd2640>, device = None, features = []
def read(self, device=None, features=[]):
"""Reads the TOC from the device given as string
The user is supposed to use :func:`discid.read`.
"""
if "read" not in FEATURES:
raise NotImplementedError("discid_read not implemented on platform")
# only use features implemented on this platform and in this module
self._requested_features = list(set(features) & set(FEATURES)
& set(FEATURES_IMPLEMENTED))
# create the bitmask for libdiscid
c_features = 0
for feature in features:
c_features += _FEATURE_MAPPING.get(feature, 0)
# device = None will use the default device (internally)
try:
result = _LIB.discid_read_sparse(self._handle, _encode(device),
c_features) == 1
except AttributeError:
result = _LIB.discid_read(self._handle, _encode(device)) == 1
self._success = result
if not self._success:
> raise DiscError(self._get_error_msg())
E discid.disc.DiscError: cannot open device `/dev/cdrom'
discid/disc.py:152: DiscError
=========================== short test summary info ============================
FAILED test_discid.py::TestDisc::test_read_features - discid.disc.DiscError: ...
FAILED test_discid.py::TestDisc::test_read_put - discid.disc.DiscError: canno...
FAILED test_discid.py::TestDisc::test_read_simple - discid.disc.DiscError: ca...
========================= 3 failed, 13 passed in 0.13s =========================
I will disable these tests for now, as they can never be met in a packaging environment. Maybe they can be changed so that they are skipped if no cdrom drive is available?
http://tickets.musicbrainz.org/browse/LIB-29 is quite new and probably still takes some time.
However, if we include that together with the ISRC API addition, then we might circumvent performance problems later on.
So everybody who only wants the id can use read()
now and we will make simple reads faster when LIB-29 is implemented.
Later we can introduce an optional mode
option where you have to tell if you want ISRCs and the "ID users" don't have to change anything.
Right now the DiscError
exception is being called for any issue with reading the disc, e.g., including there not being a disc drive at all or the user not having permission to access it. It would be nice if at least these two (no drive, no permissions) could be handled by their own exceptions.
The libdiscid API is documented here: http://jonnyjd.github.com/libdiscid/discid_8h.html
In our binding these are complete:
read()
) libdiscid 0.5.0On Windows and Darwin finding libdiscid in the current folder works fine, but on Linux/Unix we have to use ./libdiscid.*
Problem found on a sunos5
machine (sys.platform) without root access (no system-wide install) and verified on linux
.
We have to make sure that the general detection still works, of course.
On Windows XP I got this error in isrsubmit when no libdiscid library can be found:
C:\MinGW\msys\1.0\home\JonnyJD\isrcsubmit>isrcsubmit.bat
Traceback (most recent call last):
File "C:\MinGW\msys\1.0\home\JonnyJD\isrcsubmit\isrcsubmit.py", line 42, in <m
odule>
import discid
File "C:\MinGW\msys\1.0\home\JonnyJD\isrcsubmit\discid.py", line 103, in <modu
le>
_LIB = _open_library(_LIB_NAME)
File "C:\MinGW\msys\1.0\home\JonnyJD\isrcsubmit\discid.py", line 100, in _open
_library
return ctypes.cdll.LoadLibrary(lib_name)
File "C:\Python27\lib\ctypes\__init__.py", line 443, in LoadLibrary
return self._dlltype(name)
File "C:\Python27\lib\ctypes\__init__.py", line 365, in __init__
self._handle = _dlopen(self._name, mode)
WindowsError: [Error 126] Das angegebene Modul wurde nicht gefunden
I couldn't find a quick way to change the language the translation would be like:
The given module was not found
.
No indication on what module or what name was tried.
So this is not helping at all to find the problem, unless you already know what the problem is.
Fun thing is: it doesn't actually tell more than an ImportError
I would have thrown before the change in #15 (2626879).
EDIT:
That was with Python 2.7.3. With Python 3.3.0 I get the same, but WindowsError is called OSError.
there are a couple of ISRC backends in gatherIsrcs()
in isrcsubmit:
https://github.com/JonnyJD/musicbrainz-isrcsubmit/blob/e00afb14ee65ead6e15badab69f6699912de262a/isrcsubmit.py#L584
libdiscid can natively (without other tools) read ISRCs on Darwin and Windows. However, it can't on Linux.
Including above mentioned backends for linux might be an option.
(the best option would be to implement ISRC reading in libdiscid for linux, though)
When a disc id is set once, Libdiscid will not recalculate it, even if read()
or put()
are called again.
See: https://github.com/metabrainz/libdiscid/blob/master/src/disc.c
So we have to tell that in the documentation or make this use case impossible.
Ideas:
__init__
read/put()
We possibly should change that behavior upstream.
The decision to use track_offsets[1]
to get the offset for the first track was based on discid_get_track_offset(disc, 1)
.
This is kind of an 1:1 translation to the libdiscid API, but when using an array tracks
we have this:
tracks[0] corresponds to track_offset[1].
Well, the problem is there anyways, since tracks[0] does have the track number 1.
The question rather is:
What is the least suprise?
for track_number in range(1, len(tracks)-1):
print(track_offset[track_number])
or
for track_number in range(1, len(tracks)-1):
print(track_offset[track_number-1])
Should the track_offsets be accessed by track_number or by "track_index"?
The decision in the libdiscid API and in this module currently was, that you should index by track_number and having track_*[0] to be something different a way to achieve this.
A related issue is #23 (Track objects?). That doesn't solve the index problem though.
On Darwin the default_device is no (platform dependent) constant, but we actively search for a suitable device on the system.
This will break when there is no drive (or maybe also when there is no disc?).
Tested on a virtualbox machine. That might also be some change on my end, since I updated virtualbox/kernel and many other things in between. I also can't get isrcsubmit to work and that did work before..
However, we should probably remove the default_device test on darwin.
We have MusicBrainz colors now, but we don't have a version list (on the left) anymore.
You can still change between documented versions in the lower right corner, but that isn't as obvious.
There are lots of things to test that only work when a cd drive is available or we even need an audio disc in the drive.
These tests are quite valuable, but should not be done when python setup.py test
is issued, since that is supposed to be run on installation/build.
It can still reside in test_discid.py
so we don't have even more files.
There is a decision to be made what we do when a feature is not available.
We probably shouldn't fail, but skip tests and issue some type of warning.
Not sure how the support for that is in unittest. We probably have to code a skipped_list and display it after the tests.
The use case would be a newer local libdiscid where an old libdiscid is installed system-wide.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.