cli icon indicating copy to clipboard operation
cli copied to clipboard

[Lyrics Plus] save song delays

Open EricAndrechek opened this issue 3 years ago • 5 comments

📝 Provide a description of the new feature

Ideally, when the user clicks +/- to adjust the delay here: image this delay value would be saved. Then, when the song plays again in the future, even if the device has restarted or Spotify has been closed and opened again, the delay the user had custom set could be re-used again.

If you wanted to take it a step further, it would be pretty cool if there was some database that these delays users set could be saved to, and then a median value or something would be applied by default to adjust for slight timing inaccuracies. Or on a simpler note, maybe save these delays in a JSON file somewhere so that they can easily be exported and shared or synced across devices. This is a bit extreme and extra, but would be cool!

➕ Additional Information

My thought is, that in the CACHE variable, or, ideally in localStorage with CONFIG, the CONFIG.visual.delay value could be saved for each song. Then, instead of resetting the delay to 0 here, it would instead query the cache/config to see if there was a saved delay value for that song's track URI as the key and use that delay, falling back on 0. This would mean that users would be able to set delays for songs as they wished and that those configured delays would save when they played the song again.

I think that by only storing the URI and delay (and not saving it or deleting the saved entry if the delay is 0, obviously), it wouldn't take up too much space, and using URIs as keys would keep it speedy. Alternatively, instead of storing the delay values in the existing config localStorage, we could make a separate localStorage item to store delays and add a button to the settings to export said localStorage item as a JSON file.

I am not super familiar with React and hence don't fully feel comfortable making this change myself via a pull request, but if the maintainers agree with this idea and think it would be accepted as a merge, I could potentially look into helping implement it.

EricAndrechek avatar Jul 29 '22 17:07 EricAndrechek

~~Duplicate of #1765~~

rxri avatar Jul 29 '22 17:07 rxri

Duplicate of #1765

I interpreted that as a delay that would apply for all songs, not specific to each song. I understood that issue as having a use-case for say if there was a large latency in your Bluetooth audio or something that you needed to correct for. I think this issue is pretty significantly different from that.

EricAndrechek avatar Jul 29 '22 17:07 EricAndrechek

Yeah, and that's not what this issue is trying to accomplish. Instead of some weird global offset delay like that this would be a unique offset for each individual song.

EricAndrechek avatar Jul 29 '22 17:07 EricAndrechek

Yes, I noticed - that's why I deleted my comment. My bad. I don't see an actual benefit of making separate delay for every song. However, I will re-open it for other people in our team to check for benefits of this, and they will decide if it's good idea or not. Sorry for inconvenience.

rxri avatar Jul 29 '22 17:07 rxri

For me the use case is when I'm listening to songs the lyrics being off will bug me, so I'll spend some time adjusting the delay to line it up. This takes time, especially when I'm doing it for hundreds of songs. And every time I play the song again I have to recalibrate the delay. It bothers me to have it off and so fixing it every time is my current solution which seems unnecessarily tedious. Storing these values would mean I don't have to keep changing those values every time I hear the same song again. Not everyone may use this, but I don't think it negatively impacts those who don't.

Thanks for your support and understanding!

EricAndrechek avatar Jul 29 '22 17:07 EricAndrechek