Conversation
|
Package maintainer for Arch Linux here, if you do not fix it now, you're just kicking the can down the road. As there are already issues popping up and there will be more as Python 3.1{3,4} become more widespread. Just my 2 cents. |
|
How does this affect matching fingerprints that have already been uploaded to AcousticBrainz? |
|
It shouldnt matter too much, they will be different ofc. But comparing should be done with a distance measure anyways and results should be closer to the true distribution now. We should test this tho not sure how the implementation looks on their side. Effectively only the last few bytes changed (because that's how the algorithm works). Also only for long songs (bigger 120s). The only issue I see is beets here: I'm not sure how and where we do fingerpint comparisons but if they are equality checks and not a distance based with a cutoff, that will introduce issues. As a side note, the fingerprints are still different to the ones generated by the cli fpcalc tool. But the average error in my local testing seem to be lower i.e. they are closer by distance. As another note the fingerprints will also differ slightly if you use another spectrum extraction backend. So inconsistencies are always an expected thing. |
dc1af3b to
98bbd65
Compare
we use to ingest/feed in fingerprinting can influence the generated fingerprints. This fixes the issue by only consuming the expected samples.
98bbd65 to
01a5af0
Compare
|
@snejus I have just seen that this is still open. Rebased it and added a test for that verifies the expected behavior. How do we feel about merging this now? |
e47fa5b to
15f8ae2
Compare
|
Would you mind testing this in beets? |
|
We use the proper function to compare the fingerprints https://github.com/beetbox/beets/blob/4e08403df445e13a49916f3d1390dcc55ab66e5b/beetsplug/chroma.py#L329. |
|
I'm just concerned about the previously calculated fingerprints that had already been uploaded online. I'd like to check that a new fingerprint calculated for the same track (say 6 min long) is still matching the old one. |
|
What do you mean by "different"? At the byte level fingerprint of course they will differ. From a fingerprint matching perspective, though, they are considered the same. With the changes, our fingerprints are now more consistent, but they may still be different on a byte level for some users. This will happen if the block sizes didn’t align perfectly with Importantly, "different" here is not a problem. The algorithm is designed to handle small differences songs are never entirely identical at the sample level, and minor variations (like a single bit flipped due to sampling or compression) are expected. If you compute the distance/match between the pre-fix and post-fix fingerprints, it will be very small though it will almost never be exactly 0. Example:
|
|
Thanks for the explanation, this makes sense to me! Good to go |
|
@semohr we should release it to unlock beetbox/beets#6267 right? |
|
Go ahead. Im was just trying to rebase that PR and remove the refactor. Was done quite some time ago 😬 |
|
See #94 |
This PR adds testing targets for version 3.14, enabling us to verify compatibility with Python 3.14. closes #6232 I would also like to see this addition included here as it will introduce issues for 3.14 users if not beetbox/pyacoustid#90 <img width="1223" height="534" alt="image" src="https://github.com/user-attachments/assets/befa6204-2daf-4234-bf5a-247971eda23e" />
Depending on python version and hardware the
io.DEFAULT_BUFFER_SIZEvaries. We use this value to determine the length of blocks we ingest/feed to the chromaprint library. This can influence the generated fingerprints if the block sizes do not align with the number of max samples (which they hardly ever do).Decision needed:
How do we want to approach this? The fix can be considered a bugfix but also breaking change. The fingerprints generated previously (using too much data from the last block) will not match the ones generated after the change. In my opinion this is a bug although fixing the bug might introduce issues for some users.
closes #89