beetbox / pyacoustid

Python bindings for Chromaprint acoustic fingerprinting and the Acoustid Web service
MIT License
330 stars 66 forks source link

to_buffer helper added for python 3 support #51

Closed victusfate closed 5 years ago

victusfate commented 5 years ago

I wasn't able to test with python 2, but I did test with python 3

sampsyo commented 5 years ago

Thanks for getting this started! The thing to check is whether the new to_buffer function works for all the allowed inputs. Namely, does a tobytes method exist on all of the BUFFER_TYPES types, buffer, memoryview, and bytearray? If not, we'll have to handle all of those one way or another.

victusfate commented 5 years ago

oh yeah, whoops!

memoryview is the data format I'm currently passing in. I'll check bytearray next, I don't think python3 w/ chromaprint supports buffer type from this line https://github.com/beetbox/pyacoustid/blob/master/chromaprint.py#L12

victusfate commented 5 years ago

ok updated the tobytes() call (which only memoryview has) with a bytes constructor call which appears to work for both memoryview and bytearray

messel@Marks-MacBook-Pro:~$ python
Python 3.7.2 (default, Sep 19 2019, 14:58:19) 
[Clang 10.0.1 (clang-1001.0.46.4)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> b = bytearray(range(20))
>>> b
bytearray(b'\x00\x01\x02\x03\x04\x05\x06\x07\x08\t\n\x0b\x0c\r\x0e\x0f\x10\x11\x12\x13')
>>> bytes(b)
b'\x00\x01\x02\x03\x04\x05\x06\x07\x08\t\n\x0b\x0c\r\x0e\x0f\x10\x11\x12\x13'
>>> m = memoryview(b'\x00\x01\x02\x03\x04\x05\x06\x07\x08\t\n\x0b\x0c\r\x0e\x0f\x10\x11\x12\x13')
>>> m
<memory at 0x103f724c8>
>>> m.tobytes()
b'\x00\x01\x02\x03\x04\x05\x06\x07\x08\t\n\x0b\x0c\r\x0e\x0f\x10\x11\x12\x13'

I tested passing bytearray objects to the chromaprint.feed function by manually constructing one from a memoryview object, ie bytearray(memoryview) so it looks good.

sampsyo commented 5 years ago

Thanks!! I committed a slightly different version that just uses bytes or str directly, via a BYTES_TYPE object. Let me know if this works for you too!

victusfate commented 5 years ago

I'll try it out, thanks

ok looked at the change log, should work fine. will test in a few.

victusfate commented 5 years ago

tried it out, worked w/ py3

victusfate commented 5 years ago

oh quick newbie python question in my requirements.txt I can reference your version with the fix git+https://github.com/beetbox/pyacoustid.git@33c0203aacddbdee3d3c6ceccb70f7305415a579

is there some way I can reference the v1.2.0 even though there's no release yet? without the direct github reference I got this error

Collecting pyacoustid==1.2.0 (from -r requirements.txt (line 9))
  ERROR: Could not find a version that satisfies the requirement pyacoustid==1.2.0 (from -r requirements.txt (line 9)) (from versions: 0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 1.0.0, 1.1.0, 1.1.2, 1.1.3, 1.1.4, 1.1.5, 1.1.6, 1.1.7)
ERROR: No matching distribution found for pyacoustid==1.2.0 (from -r requirements.txt (line 9))
sampsyo commented 5 years ago

That should probably work, although I'm not a pip expert.