Implement synchronisation between ALSA and Spotify volume

devgianlu / go-librespot

Yet another open source Spotify client, written in Go.

GNU General Public License v3.0

52 stars 7 forks source link

Implement synchronisation between ALSA and Spotify volume #40

Closed tooxo closed 3 weeks ago

tooxo commented 2 months ago

This is a first POC for implementing the feature I mentioned in #36. I've never written any GO before, so it would be nice if someone could give me some feedback on where I could make improvements.

Features:

[x] Everytime the ALSA volume is changed, the volume in spotify also changes.
[x] Sync the volume from ALSA to Spotify on connect or startup
[x] Make the behaviour configurable
- [x] ~~(The synchronization should only be enabled, if externalVolume is true)~~
- [x] The ALSA control name needs to be configurable (currently hardcoded as master)
- [x] Mixer functionality should be opt-in instead of opt-out
- [ ] ...
[ ] Documentation explaining the feature

tooxo commented 2 months ago

@devgianlu I implemented the feature how I imagined it when I wrote the issue a few days ago. It would be great if you took a look at it if this works with your project. I have been testing my code for the last few days and didn't find any issues yet.

If you have questions, please ask or "review" the corresponding code parts.

Edit: While working on your code I noticed that you seem to split up your configuration structs to pass every field of the struct into a function. Is there a reason behind this? For me it seems like this just adds extra steps, when one wants to add a new configuration variable.

devgianlu commented 1 month ago

While working on your code I noticed that you seem to split up your configuration structs to pass every field of the struct into a function. Is there a reason behind this? For me it seems like this just adds extra steps, when one wants to add a new configuration variable.

Yes, that is not very wise or clean, but I think it should be sorted out in another PR.

tooxo commented 4 weeks ago

I think it is now ready from my side, let me know what you think.

devgianlu commented 3 weeks ago

After this I think we're good to go, feel free to reword/reorganize the commits or I'll just squash this PR.

tooxo commented 3 weeks ago

After this I think we're good to go, feel free to reword/reorganize the commits or I'll just squash this PR.

Just, squash it, should be alright.