Skip to content
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

Field comparison problems #2942

Closed
twrightsman opened this issue Jun 2, 2018 · 13 comments
Closed

Field comparison problems #2942

twrightsman opened this issue Jun 2, 2018 · 13 comments
Labels
needinfo We need more details or follow-up from the filer before this can be tagged "bug" or "feature." stale

Comments

@twrightsman
Copy link
Contributor

Problem

Generalized issue for the problem reported in #2929. beets has problems comparing certain fields like BPM for the commands write and update.
So far it seems that the comparison problems are with at least three fields:

  1. BPM (bpm: 96 -> 96.0649795532)
  2. bitrate
  3. acoustid_fingerprint (the strings are the same but beets still reports them as different)
acoustid_fingerprint: AQADtHnCTRp0NE6UONB-XDiDPkZDDicu-FAX5UG_XAjz5TjqoMnxHR_uE-EZNIwY46hHiNlxR5xwo18O7YePXguOo5n2QleOPggpB_1w6_iHUM5h7tBxBvmOB85UdB9-hNdhH3qP79CbnfhhHVZ09DpClmiOHz_6GEbuQxePeBcO_8cZIc2RbOuFEk10fMKZ46jWB43yIa6QvHiPUse3sIO1E5WYoT8Nf-iF_nC-HOFC2ajuwXx4nIddo1cO19mgK0bul3gK8zuuXPge4QrSKy9eCbqPRoyOTtlR-NGhncF35Nlx9If_4kqQQ0d-_Ee9BfGPXdBv1GFplPKhC3-R90pgRsejFFeFvIOWHJHo4celfMKbw2eOpztiv_iR60fDB_1x9AyHD_7h8riMKxH8B4e5XKh04wmF5kcvPGzRJy3iZYNe5BzhGk8unBbhcMzxTDgaKsWVYf6R6yiZB9zSEsyu4GJm9Md3eNex45wOfTr6JDgTHZ-OS0aPX4L2o9XhHtoTnAlWxI9wKUuT4GHRMCZCNgr04VImPNscPB8uZVnx9dCiSJQQSccX_MF5jbh4CX0YosG7C4h9QTf-Do2LSskUH5HJGE2oI3yiHGdFo1J2SMmY4zh-PFcGXdFxS0FoH8xT4clU7HzRH020ZMF5_EfuJHgkP9AP_egeBK_h41wQPjjUnEM-R3iSS6jRrDjeZDruRE1wHnfG4kdz6XiCfId2ujh-Bp1GEo2T4xSa_NAZ49ZgHWWPMjd8RehDhGdwJrHgFpUoxVLwH1OOZFQ0I8eFH8wPT5SLlBKNZEoO3AxxRaCeChcTHVesY08YI1lUcgjH4EVzC-2kDEcfCc1D5E0O7UST4y-0l8fpwsePH82hSw-aI1RG5bizDrmO5xGabULtHOqFx0eYp-i447wQRhf6VNAupPvw49lx4hSD6FGPw2OP66DWoZcS9IRnCa3EGpURRjv-Ci8VND16BdxV_FeQuwr0HHmKVkWzp_ge5OKn4Dn0H3lb_AfzcEeYC4dkJcvRwy7Ed0LJZEdOobQ6YruJlseTGZcjHc2DSpRE4Utwx0J-aFUYpIzoGX2Oh0M3B_909E1x-XiH6biKWD2YjVCfBJeGg9fRLyJ8iuiPK8uFB-rO4kzSo78O48gjFuV6_DjiJ4W-GfGPh8dF4gys6niU4pXSIM88QcWXLIfOG2EfbKN0HM-Saqh5nJ7x_Si_oQmdHP_x7EdO3IauC-lY41GS4z7-CFZ54RVGZUHaI3nUET-4ucQrBV9m9Id3I4ukI5kH89iipTnqzSSOi0nwVSmuIvwhMj_uEyKD3_jgsEIr7YUuksHZBnln5INrXIuU4EuOz2kS6CSJ-3gD7Y3gWceToCRx6MscXEek_EFlHY-WGtOyB02OkBKJp0mhqg9SHz_-w8yHR8ghZltyI7KON4YntBMuotka5wj_4Cc05UKYSCeuC9fi4OLhOiOeIN-hSLSL5hly9HAoBn1waUdf-MgPXbORHmc2wY_SDb3x4-LQ5wgT59DjII9iPIdvnIpx4zpSOaKR9CmaHj2PVzf2Hb7xDEesG5qXI082pNaO6_iPkc6hvwOeQ8-PMF-MXbjgNDw-h8d3IX6QkMGefriUo8HhH-WH-4g3aIsmpNEbDdOjHPxx9LsRXZGK6xAfhFYW7JqPRw8a9DpOJREJ_njzY3pcfGgqHneH40HTSRXOXUj7QC58E7mOyseePPhxGVd-_PjCDj8YHU40yvgO5hLCv2hKndDHBPWOJuKFH19wbTLaZ8cJ_SEuksh3w0f4BeoT5Jlwsfh2vHnQE83IHM-CPFCrHHk2EfeMHHLWI8yPe-gz48fYmDiHpsrWI9wTGn9FnIL2H212CfpzVKcD_1APP0EeFs5Gbpge_Cq-DMQbEtVGPM0F70f0BPlhx8SP9IsYzOSFXzwk-fhMIt0xrod3IhfEOGvQJctS5HyCWhojPAma43oQO3MCXVeQK0HTow8-LsZXPDreM3iW41YoNKjxG2dzHFufI8_CQdMYZIkmBU_w48Imj8WDJD7CC03U489x7TiafDH6I68UaOFynJKC_xAt4UM-irgvfMeP3Uf-QswLr7jQaDouQz9yVM3RxMUz_Gh-hHKioM8hVj3CH-dRUSec6EHZGbiPm8Ed_JmRXEeunLicJigc56gyGS87HM9RhMQPPYI-xeiHtPzxw9aCypeGsPugRQ_yrsHMo2GcozdOEWEeQTNV5OvxE7-wPTsuHs-Fj0OY64d2CdcR5tFxinhzOJbRJ0TQUEfJIzxf3IExLTnOnLh6PDnSKkFiBj2a5cTTMsIbhhHcD5_wIXoOLWOCUIuOo-sFP8elGcaV49uP9DmOP4ZDHedy5BL841KeDM8DX7i4I5S60MR1XNCSVujSKDgeVFp05Dju5fDDCWU-zD_I-aicIlL-4xT0HD38Z1CZ8AgXffixV-hvMBGj48ujHUPzqaiVBx89_OiFMD_0I053TFwP5werE--QvtDeIc-Fe0eVo3kmPNuR9dCPMFXioafB-biUhbhwoc-FU1uEHt2P5riWIGuD5MfUOIevEr2Ocwl-Y7IyODqPPqiHLHkPfYqQ5xGmXHpQ38ih54jbY1KiC_4y4gcTVh0eGgGFAEQJpYwAAhiEjAACUCAUQcQgIABgABFAjEBINAZEIIIIS4QTQBhAGUBEGkgUQIZRpAAwGEjFBBgEWiIBEEIggAAwgBBhgBIACIMIQYgIQoQwAghGiCKBIODMJ0AYAAAXQEGABGDCWWkYAcAAIYxgBgngFGACAUapQogcoZBQwgimDGPAJSUUQAAA6gBB2gggDWFAcsUIQQgYAolyDChCDBCOAEMIEBAgIgwwQghEvCkCAGIAUQoQSKgRCAkFFIJEAgqMNcwIiAAQDBAPhCNCiIAIIcIA4hgEQjgDlOGQIOAAR0AZwzByAAHBqFMAOGgNdMgAAIBQjABgHCKEAEWcgUIxYAwChgAKEVDaACCEMUYJZISQQggAADHCECoEEAgYRQxgwkgBRAAIEOU0tsKgYAhQCCEFEDAAqAKEBcIYSAywlAlFHCJAOEWIMQQRAZgSgiNiCKAAIWCBUk4YBZwwSAQABgNAEUGcIEQQgIAAwCrBmBXIIGEMksZIiIhgwClAFQJEIASAQQgiRQgCBhnABBUGMQCAEQooqJRDCARmnCCEEIYMFQYKRBgCBggqhANCIyKAEZIgAkBCDglBBFEIEMEMJsAYYYBgghCgleBOEIaENYQQwZAQJBgmECJAACMIE1IBIMBxAABKgACCAKMQcUIKaYgCAhIHCBGEEAIAIABIBAAhjigABGDCGS4EMMIJJqBRwAEBiCGCMGgUMMYAA5zQwCjvqKFKCAOQIggIAgwAgBgABFJKCSWIYcZBABgTFDkgAZFGKYAMEEAASAxBChGNrCFSCOKIIQIBRYBgBhEIhFLCAACARAgYICBwlgDACEACCoEEkM4rYxRCzEAhBEEAMGOMUUQQIgARBBkxGXMiCAAAAMAAqgwmQADDBDEQAIGMUUwJyAQSwAALlCJAWGQEMEhIYoggTAGBEFEOAUQRMAIQgpQAAmjjjBWUMA -> AQADtHnCTRp0NE6UONB-XDiDPkZDDicu-FAX5UG_XAjz5TjqoMnxHR_uE-EZNIwY46hHiNlxR5xwo18O7YePXguOo5n2QleOPggpB_1w6_iHUM5h7tBxBvmOB85UdB9-hNdhH3qP79CbnfhhHVZ09DpClmiOHz_6GEbuQxePeBcO_8cZIc2RbOuFEk10fMKZ46jWB43yIa6QvHiPUse3sIO1E5WYoT8Nf-iF_nC-HOFC2ajuwXx4nIddo1cO19mgK0bul3gK8zuuXPge4QrSKy9eCbqPRoyOTtlR-NGhncF35Nlx9If_4kqQQ0d-_Ee9BfGPXdBv1GFplPKhC3-R90pgRsejFFeFvIOWHJHo4celfMKbw2eOpztiv_iR60fDB_1x9AyHD_7h8riMKxH8B4e5XKh04wmF5kcvPGzRJy3iZYNe5BzhGk8unBbhcMzxTDgaKsWVYf6R6yiZB9zSEsyu4GJm9Md3eNex45wOfTr6JDgTHZ-OS0aPX4L2o9XhHtoTnAlWxI9wKUuT4GHRMCZCNgr04VImPNscPB8uZVnx9dCiSJQQSccX_MF5jbh4CX0YosG7C4h9QTf-Do2LSskUH5HJGE2oI3yiHGdFo1J2SMmY4zh-PFcGXdFxS0FoH8xT4clU7HzRH020ZMF5_EfuJHgkP9AP_egeBK_h41wQPjjUnEM-R3iSS6jRrDjeZDruRE1wHnfG4kdz6XiCfId2ujh-Bp1GEo2T4xSa_NAZ49ZgHWWPMjd8RehDhGdwJrHgFpUoxVLwH1OOZFQ0I8eFH8wPT5SLlBKNZEoO3AxxRaCeChcTHVesY08YI1lUcgjH4EVzC-2kDEcfCc1D5E0O7UST4y-0l8fpwsePH82hSw-aI1RG5bizDrmO5xGabULtHOqFx0eYp-i447wQRhf6VNAupPvw49lx4hSD6FGPw2OP66DWoZcS9IRnCa3EGpURRjv-Ci8VND16BdxV_FeQuwr0HHmKVkWzp_ge5OKn4Dn0H3lb_AfzcEeYC4dkJcvRwy7Ed0LJZEdOobQ6YruJlseTGZcjHc2DSpRE4Utwx0J-aFUYpIzoGX2Oh0M3B_909E1x-XiH6biKWD2YjVCfBJeGg9fRLyJ8iuiPK8uFB-rO4kzSo78O48gjFuV6_DjiJ4W-GfGPh8dF4gys6niU4pXSIM88QcWXLIfOG2EfbKN0HM-Saqh5nJ7x_Si_oQmdHP_x7EdO3IauC-lY41GS4z7-CFZ54RVGZUHaI3nUET-4ucQrBV9m9Id3I4ukI5kH89iipTnqzSSOi0nwVSmuIvwhMj_uEyKD3_jgsEIr7YUuksHZBnln5INrXIuU4EuOz2kS6CSJ-3gD7Y3gWceToCRx6MscXEek_EFlHY-WGtOyB02OkBKJp0mhqg9SHz_-w8yHR8ghZltyI7KON4YntBMuotka5wj_4Cc05UKYSCeuC9fi4OLhOiOeIN-hSLSL5hly9HAoBn1waUdf-MgPXbORHmc2wY_SDb3x4-LQ5wgT59DjII9iPIdvnIpx4zpSOaKR9CmaHj2PVzf2Hb7xDEesG5qXI082pNaO6_iPkc6hvwOeQ8-PMF-MXbjgNDw-h8d3IX6QkMGefriUo8HhH-WH-4g3aIsmpNEbDdOjHPxx9LsRXZGK6xAfhFYW7JqPRw8a9DpOJREJ_njzY3pcfGgqHneH40HTSRXOXUj7QC58E7mOyseePPhxGVd-_PjCDj8YHU40yvgO5hLCv2hKndDHBPWOJuKFH19wbTLaZ8cJ_SEuksh3w0f4BeoT5Jlwsfh2vHnQE83IHM-CPFCrHHk2EfeMHHLWI8yPe-gz48fYmDiHpsrWI9wTGn9FnIL2H212CfpzVKcD_1APP0EeFs5Gbpge_Cq-DMQbEtVGPM0F70f0BPlhx8SP9IsYzOSFXzwk-fhMIt0xrod3IhfEOGvQJctS5HyCWhojPAma43oQO3MCXVeQK0HTow8-LsZXPDreM3iW41YoNKjxG2dzHFufI8_CQdMYZIkmBU_w48Imj8WDJD7CC03U489x7TiafDH6I68UaOFynJKC_xAt4UM-irgvfMeP3Uf-QswLr7jQaDouQz9yVM3RxMUz_Gh-hHKioM8hVj3CH-dRUSec6EHZGbiPm8Ed_JmRXEeunLicJigc56gyGS87HM9RhMQPPYI-xeiHtPzxw9aCypeGsPugRQ_yrsHMo2GcozdOEWEeQTNV5OvxE7-wPTsuHs-Fj0OY64d2CdcR5tFxinhzOJbRJ0TQUEfJIzxf3IExLTnOnLh6PDnSKkFiBj2a5cTTMsIbhhHcD5_wIXoOLWOCUIuOo-sFP8elGcaV49uP9DmOP4ZDHedy5BL841KeDM8DX7i4I5S60MR1XNCSVujSKDgeVFp05Dju5fDDCWU-zD_I-aicIlL-4xT0HD38Z1CZ8AgXffixV-hvMBGj48ujHUPzqaiVBx89_OiFMD_0I053TFwP5werE--QvtDeIc-Fe0eVo3kmPNuR9dCPMFXioafB-biUhbhwoc-FU1uEHt2P5riWIGuD5MfUOIevEr2Ocwl-Y7IyODqPPqiHLHkPfYqQ5xGmXHpQ38ih54jbY1KiC_4y4gcTVh0eGgGFAEQJpYwAAhiEjAACUCAUQcQgIABgABFAjEBINAZEIIIIS4QTQBhAGUBEGkgUQIZRpAAwGEjFBBgEWiIBEEIggAAwgBBhgBIACIMIQYgIQoQwAghGiCKBIODMJ0AYAAAXQEGABGDCWWkYAcAAIYxgBgngFGACAUapQogcoZBQwgimDGPAJSUUQAAA6gBB2gggDWFAcsUIQQgYAolyDChCDBCOAEMIEBAgIgwwQghEvCkCAGIAUQoQSKgRCAkFFIJEAgqMNcwIiAAQDBAPhCNCiIAIIcIA4hgEQjgDlOGQIOAAR0AZwzByAAHBqFMAOGgNdMgAAIBQjABgHCKEAEWcgUIxYAwChgAKEVDaACCEMUYJZISQQggAADHCECoEEAgYRQxgwkgBRAAIEOU0tsKgYAhQCCEFEDAAqAKEBcIYSAywlAlFHCJAOEWIMQQRAZgSgiNiCKAAIWCBUk4YBZwwSAQABgNAEUGcIEQQgIAAwCrBmBXIIGEMksZIiIhgwClAFQJEIASAQQgiRQgCBhnABBUGMQCAEQooqJRDCARmnCCEEIYMFQYKRBgCBggqhANCIyKAEZIgAkBCDglBBFEIEMEMJsAYYYBgghCgleBOEIaENYQQwZAQJBgmECJAACMIE1IBIMBxAABKgACCAKMQcUIKaYgCAhIHCBGEEAIAIABIBAAhjigABGDCGS4EMMIJJqBRwAEBiCGCMGgUMMYAA5zQwCjvqKFKCAOQIggIAgwAgBgABFJKCSWIYcZBABgTFDkgAZFGKYAMEEAASAxBChGNrCFSCOKIIQIBRYBgBhEIhFLCAACARAgYICBwlgDACEACCoEEkM4rYxRCzEAhBEEAMGOMUUQQIgARBBkxGXMiCAAAAMAAqgwmQADDBDEQAIGMUUwJyAQSwAALlCJAWGQEMEhIYoggTAGBEFEOAUQRMAIQgpQAAmjjjBWUMA

Setup

  • OS: Ubuntu 18.04
  • Python version: 3.6.5
  • beets version: 1.4.7
@sampsyo
Copy link
Member

sampsyo commented Jun 2, 2018

Hi! Are you interested in diving in to see if you can propose potential solutions?

@sampsyo sampsyo added the needinfo We need more details or follow-up from the filer before this can be tagged "bug" or "feature." label Jun 2, 2018
@twrightsman
Copy link
Contributor Author

twrightsman commented Jun 3, 2018

At least for acoustid_fingerprint, it seems that pyacoustid returns the fingerprint as a byte string and this is directly stored into a library (when it should be converted to a string). When beet write or beet update tries to compare the library metadata with the file metadata, it compares a byte string (from the library) to a string (from the file), which yields false even though the strings are technically the same.

Even though the acoustid_fingerprint column is declared as TEXT, it seems sqlite is loosely-typed and BLOBs can still be stored there. I'll add a commit that converts the byte strings over to strings soon.

@twrightsman
Copy link
Contributor Author

For bpm, the bpm field is sometimes being stored as a float in the library.

sqlite> SELECT DISTINCT typeof(bpm) FROM items;
integer
real

The AcousticBrainz plugin is not properly converting the retrieved floating-point bpm into an integer. (Maybe the defined type of bpm for beets should be float?)

acousticbrainz: attribute bpm of [...] set to 133.899032593

@twrightsman
Copy link
Contributor Author

For bitrate, it seems that the file metadata is not to the same precision as the library metadata. So a bitrate of 256000 will fail the comparison against 256138 but will display as 256kbps -> 256kbps.

Again, I suspect this is due to the AcousticBrainz plugin not casting values from the database results to the proper types/precisions.

@twrightsman
Copy link
Contributor Author

@sampsyo It seems that these problems could be prevented by casting values to the appropriate type on assignment to a model. I can try to flesh out the dbcore.types module with actual normalize functions (they seem un-implemented at the moment) if that's something you think would best address this issue?

@sampsyo
Copy link
Member

sampsyo commented Jun 7, 2018

Yes; thanks for looking into it—enhancing the normalization for at least some types would probably be a good idea. It would be important to be very careful with this, though, since changes to these types can have pervasive effects on every operation in beets. Maybe we can start with one or two types and thoroughly test things out.

For the fingerprint in particular, it’s not 100% clear to me whether the right type is a string or a binary blob.

@minusf
Copy link

minusf commented Jul 4, 2018

Found this bugreport while trying to list my library using bitrates. My files are mostly ogg so I guess these are nominal bitrates, but still, very useful...

$ beet list bitrate:192
$
$ beet list -f \$bitrate|head
192kbps
192kbps
192kbps
192kbps
192kbps
192kbps
192kbps
192kbps
256kbps
256kbps
$ beet list bitrate:192kpbs
invalid query: 'bitrate:192kpbs': '192kpbs' is not an int or a float

@sampsyo
Copy link
Member

sampsyo commented Jul 4, 2018

@minusf This is related but a bit different. You probably want bitrate:192000 or bitrate:192000..193000 or similar.

@minusf
Copy link

minusf commented Jul 5, 2018

this works, excellent. perhaps would be nice addition to the querying section of the docs :}

@sampsyo
Copy link
Member

sampsyo commented Jul 5, 2018

Sure! If you can think of a good place to put it, please consider sending a pull request.

Holzhaus added a commit to Holzhaus/beets that referenced this issue Dec 19, 2018
Since pyacoustid returns the fingerprint as bytes (and thus causes the
database to store a bytes/BLOB object), but the tag value is a string,
the acoustid_fingerprint tag always causes file change when using beet's
"write" command, even if the actual value didn't change.

Issue beetbox#2942 describes the problem.

This commit fixes that issue for newly imported/fingerprinted files. However,
you still need to change the type of all acoustid_fingerprint fields
that are already present in the database:

    $ sqlite3 beets.db
    SQLite version 3.26.0 2018-12-01 12:34:55
    Enter ".help" for usage hints.
    sqlite> UPDATE items SET acoustid_fingerprint = CAST(acoustid_fingerprint AS TEXT);
Holzhaus added a commit to Holzhaus/beets that referenced this issue Dec 19, 2018
Since pyacoustid returns the fingerprint as bytes (and thus causes the
database to store a bytes/BLOB object), but the tag value is a string,
the acoustid_fingerprint tag always causes file change when using beet's
"write" command, even if the actual value didn't change.

Issue beetbox#2942 describes the problem.

This commit fixes that issue for newly imported/fingerprinted files. However,
you still need to change the type of all acoustid_fingerprint fields
that are already present in the database:

    $ sqlite3 beets.db
    SQLite version 3.26.0 2018-12-01 12:34:55
    Enter ".help" for usage hints.
    sqlite> UPDATE items SET acoustid_fingerprint = CAST(acoustid_fingerprint AS TEXT);
@rachmadaniHaryono
Copy link
Contributor

hi, i just use the latest commit, which contain #3097 but it still register this diff error on both bpm and pyacoustid_fingerprint.

a temporary solution i made is this branch https://github.com/rachmadaniHaryono/beets/tree/feature/skip-bpm-fingerprint-error-diff

which skip changes based on that condition.

also not mention in this discussion, does bpm have some rounded function?

i still have this change on beet write

石田燿子 - Sailor Moon S Sdtk. - 愛の戦士
  bpm: 145 -> 144.997955322
See-Saw -  - あんなに一緒だったのに
  bpm: 123 -> 122.99520874
時東ぁみ - Single - せんちめんたる じぇねれ〜しょん
  bpm: 142 -> 141.999679565

@stale
Copy link

stale bot commented Jul 12, 2020

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jul 12, 2020
@stale stale bot closed this as completed Jul 19, 2020
@Phil305
Copy link

Phil305 commented Feb 20, 2021

I actually have this same issue when I get BPMs from the AcousticBrainz Plugin on beets version 1.4.9 running Python version 3.9.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needinfo We need more details or follow-up from the filer before this can be tagged "bug" or "feature." stale
Projects
None yet
Development

No branches or pull requests

5 participants