asivery / webminidisc

Upload your Music to NetMD and HiMD MiniDisc devices thanks to WebUSB and WASM
GNU General Public License v2.0
122 stars 28 forks source link

Song recognition not working on version 1.5.0 #63

Closed joncco closed 2 weeks ago

joncco commented 1 month ago

I have been using Web Minidisc Pro with my MZ-N910. Under version 1.4.2, some tracks get reconized, some crash the whole program. When I switched to 1.5.0, even those tracks previously recognized in 1.4.2, now show up as "not recognized".

I have tried both the Electron version and the online version, I could not get 1.5.0 to recognize any track that I have tried, including those that worked in 1.4.2. I have tried this in windows (using edge for the online version) and linux, results are the same.

Rolling back to 1.4.2 (electron version) restores the ability.
Screenshot from 2024-10-01 16-00-49

previous screenshot of 1.4.2, following is of 1.5.0 after attempting to recognize the tracks:

Screenshot from 2024-10-01 16-05-12

vcolombo commented 1 month ago

I've observed similar behavior. Previously, the only way I was able to get song recognition to work was to use webmd.pro as that was still at 1.4.2.

asivery commented 1 month ago

Thank you for reporting this issue, I'll try to get it resolved soon. I did make some changes to the song recognition library, but during testing, these changes improved the reliability - I didn't expect this to happen.

dctrotz commented 1 month ago

I am glad it's not just me. I thought I was going crazy or was just stupid. I tried two computers and purchased a new input device trying to get this to work.

asivery commented 4 weeks ago

@dctrotz @vcolombo @joncco I think it should be fixed now. Can you please check if it works on the testing version? If it does, I'll push the update.

joncco commented 3 weeks ago

Found 2 problems (which may not be related)

  1. Full width title does not come out correctly: (this one is how it should look) fullwidthcorrect (this one is how it looks like in testing) fullwidthtesting

  2. I cannot click on recognize (grayed out) recognizegrayed

It also does not seem to note that i already have a userscript installed. (do I need to modify the URL in the userscript?)

asivery commented 3 weeks ago

Thanks for mentioning the full width issues, I thought they had already been fixed, as for the userscript, yes - you need to update the URL.

joncco commented 3 weeks ago

So, I have been testing this the past few days, and some tracks do get recognised, some do not. To make sure these are not caused by changes to the recognition engine, I have re-tested with 1.4.2 as well. Something has indeed changed, track 29 used to be recognized but now not. recognise142

Testing version now recognize some of the tracks (but some tracks are offset by one) testingnew

I should probably test this with something whose tracks I actually know. Will do that and post another update.

asivery commented 2 weeks ago

@joncco I've fixed full width support, but I cannot reproduce the indices being shifted back by one... that seems like a really weird issue. Can you check if this isn't just a problem with this disc being titled incorrectly?

Edit: Finally managed to reproduce it. Fixed in a6ae3058d481f4b49a56ff7d45a7e08a17ad9ab9

joncco commented 2 weeks ago

Confirmed, both full width support and track indices shifted back by one are both fixed. Because it did not happen all the time, I also had a hard time reproducing the offset issue and I'm glad that you were able to reproduce it.

asivery commented 2 weeks ago

@joncco In that case, do you think this is ready to merge into the main version?

dctrotz commented 2 weeks ago

@asivery - sorry about the delay in getting back to you. Your fix has resolved my issues. Thank you!

David

joncco commented 2 weeks ago

I have been testing the "testing" version against 1.4.2, (with 20 or so MDs) and they are more or less the same. (by more-or-less, 1.4.2 tends to crash randomly). This should be ready to merge back into main. Thanks for timely resolution.

asivery commented 2 weeks ago

Perfect! Thank you for testing @joncco, @dctrotz !