search

beetbox / pyacoustid

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

Fix compare_fingerprint errors #77

Closed albertopasqualetto closed 1 year ago

albertopasqualetto commented 1 year ago

dll fix is not so good, but it works.

sampsyo commented 1 year ago

Hi there! Could you provide a little more context for these changes? What problems were you seeing?

albertopasqualetto commented 1 year ago

@sampsyo for the changes in acoustid.py max function was wrong in python, and for the [1] is because a is a tuple and the decode fingerprint only requires the byte array.

For the changes in chromaprint.py, i am on windows 11 and chromaprint.dll is not an output of the chromaprint build and install (all the functions are in libchromaprint.dll), also without the winmode=0 the dll is not found, i do not know the real reason (i think for some security reason), but I found that piece of code and that works.

Edit: i added some comments.

albertopasqualetto commented 1 year ago

Up?

sampsyo commented 1 year ago

Hello! Since there are several unrelated changes in here, it's going to be important to understand one at a time. In general, while these changes might have made the library work on your setup, we need to proceed super carefully in each case to make sure that we don't break the library for others on different setups—and some of these, such as switching to WinDLL, could have implications that we're not anticipating.

One way to move this forward might be to split this PR into several smaller ones so we can discuss them in more detail. The changes to fingerprint comparison, i.e., the follow-up to #75, could be one unit—we can make absolutely sure we're getting the data types right at each point. And then the Windows-related library loading changes could be considered separately.

Thanks for your patience! Just want to make sure we proceed with care.

albertopasqualetto commented 1 year ago

Ok, I created 2 different PRs, one for fingerprint comparison (#78 ) and one for Windows import problems (#79). I am closing this PR. Thanks for your interest.