mpogue2 / SquareDesk

Fully-featured music player and sequence designer, designed for square dance callers
10 stars 4 forks source link

Bug: Tempo wrong when playlist songs are double-clicked after playlist loaded #686

Open mpogue2 opened 2 years ago

mpogue2 commented 2 years ago

I might be imagining this one, but it seems to me that the tempo is frequently NOT what I set it to. I never set it to 128, yet I'm discovering that some songs are at 128 at the dance.

I am suspecting that listening to a song of the same name in Vocals will override the tempo setting for the one in the SInging folder. But, that's just a guess. It's annoying that I need to check this at the dance.

That is, if I am not imagining it.

mpogue2 commented 2 years ago

Here's more info on what appears to be happening:

i) When loading a playlist, 0.9.10 will also automatically load the first song. 0.9.5 did not. Since this is different between 0.9.10 and 0.9.5, it's arguably a bug (I didn't change this intentionally).

ii) There's a bug that I think already existed in 9.5, but it was kinda "covered up" by the wacky way that playlists were handled (with current.csv). The bug is this: If you load a playlist, everything loads correctly, except then the TEMPO of the very first song in the list will change itself (!!) to the last saved tempo (not the tempo saved to the playlist, the PRE-playlist tempo), after a second or two.

This is SUPER confusing, and it took me 1/2 hour to even understand what was going on.
Sometimes this also happens to other songs, too, but ONLY when they are double-clicked (or in the case of the first song, this happens automatically due to bug i) above.

This appears to ONLY affect TEMPO, but not PITCH or anything else about the song. Don't ask me why.

mpogue2 commented 2 years ago

This is not really an M1-specific bug, since it exists in 0.9.5 as well (well, the ii bug above is also in 0.9.5).

mpogue2 commented 2 years ago

Fixed by fe921c8654e8441ad4c63dc25eae1248a1e74e84. This was exacerbated by the new "async load" that is used by the audio_decoder. Now there is a targetTempo/Pitch, and we set to that at the very end of the load after double click.

mpogue2 commented 2 years ago

Also now marks the playlist modified, if tempo or pitch are changed on a song that is from a loaded playlist.

mpogue2 commented 2 years ago

Couple minor tweaks: b0f3e63875914e732f14896d9cd3286b34ecb1d1

mpogue2 commented 2 years ago

I think this is fixed now. Reopen, if we see this again.

mpogue2 commented 1 year ago

Reopening, because I'm seeing a completely incorrect tempo in V1.0.2 . Not sure this is the same bug, because the new symptom is that either a) the tempo is percent, and the incorrect tempo is 120% (the max), or b) the tempo is BPM, and the incorrect tempo is just way off from the correct value (e.g. 110 BPM).

mpogue2 commented 1 year ago

Tweaked the tempo code a bit more. It now accepts 125 ± 15BPM as acceptable. This allows Sandstorm to be detected correctly now. And, it looks at the audio from 30-40 sec now, which allows Halloween Make My Day to be detected correctly now. Fixed by 359476f79a71f0b39d8a2087d07959e13b8e7367 .