snapcast icon indicating copy to clipboard operation
snapcast copied to clipboard

Make the default volume configurable

Open mgoltzsche opened this issue 3 years ago • 10 comments

@badaix First of all thanks for providing and maintaining this awesome piece of software!

Is your feature request related to a problem? Please describe. When connecting a new snapclient, its volume is set to 100%. However this can be very annoying and even damage speakers when audio is already playing at that point. To give an example I am experimenting with a snapcast setup where a snapclient is run on a Raspberry Pi 3 with a Hifiberry Amp2 using the client option --mixer hardware:Digital. Setting the volume to 100% is very loud obviously. Eventually it would even kill the old/cheap speakers I am using for testing purposes if I'd keep playing audio at that level (not sure if they got worse already after a couple of those accidents). Also I think my neighbors are angry with me now :wink:.

This relates to #735, particularly to https://github.com/badaix/snapcast/issues/735#issuecomment-757352479. cc @lucianm.

Describe the solution you'd like To be safe, I'd like to be able to configure the default volume for new clients to eg. 30% to avoid such accidents. I think it would be best to be able to make it configurable within the client. Though it would also be fine if it would be configurable on the server if that's easier to implement.

Describe alternatives you've considered Stay focussed when connecting a new client, make sure no audio is playing, decrease the new client's volume before starting to play music.

mgoltzsche avatar Sep 12 '21 22:09 mgoltzsche

I have a similar use case for this feature request. My USB sound card does not have a hardware volume control so if snapserver is sending audio when my machine starts, the audio plays at 100% until homeassistant starts and turns the volume in snapcast down. I'd thus like to set the default (or initial volume) to 0% or muted.

stuart12 avatar Nov 08 '21 01:11 stuart12

Is anyone working on that already? If not, I might have a look at the source if I would be able to implement this - at least with a little help from my friends :smile:. Actually I won't have time before February/March 2022, but I would love to have this feature...

nis65 avatar Dec 26 '21 15:12 nis65

@mgoltzsche @badaix I was thinking about where such a feature should be implemented and would be happy to receive your opinion:

From a users point of view I think it would be nice to set the initial snapclient volume as a snapclient parameter. With this, I could have individual defaults depending on whether I am deploying to a raspi with an AMP where the snapclient volume is the last volume control in the chain (e.g. initial snapclient volume 10%) and a snapclient with a line out that is connected to an amp with hardware volume control (e.g. initial snapclient volume 100%).

But from a technical point of view the snapclient volume is set/defined on the server (is that correct?). So it is simpler (and also probably more suitable to the snapcast architecture) just to have one global "initial volume" on the snapserver that is assigned to each snapclient on initial connect.

nis65 avatar Dec 28 '21 15:12 nis65

Tried to understand the source / protocol

  • Setting on the client side would mean I would have to extend the snapcast binary protocol (add a value to the hello message). this does not look like a job for me.
  • Setting on server side seems to happen in one (or both) of the two places:
    • https://github.com/badaix/snapcast/blob/master/server/config.hpp#L61
    • https://github.com/badaix/snapcast/blob/master/server/config.hpp#L128 It's a very long time ago that I programmed c++, so I may be completely off... Next step is to build up my compile/test pipeline...

nis65 avatar Dec 28 '21 21:12 nis65

Update: Finally had some the time to compile snapcast on my raspi. For a quick test, I manually replaced the snapserver binary in /usr/bin/with the one compiled by myself and everything still worked. Now the fun can begin...

nis65 avatar Jun 20 '22 06:06 nis65

And YES, I have a working solution :smile:

I'll have to properly prepare a merge request now and then hope that it'll get merged sometime.

Have a preview here, it's less than 10 lines added. You'll have to compile from source to test it. I did it on a Raspberry4 (to prevent me from having to play with crosscompile), a full recompile of everything takes a bit more than 30 minutes, you can try to compile snapserver only if you want.

Please contact me directly if you want me to get a copy of my snapserver Raspberry binary.

nis65 avatar Jun 25 '22 13:06 nis65

Awesome! Looks good to me.

mgoltzsche avatar Jun 25 '22 14:06 mgoltzsche

@badaix @mgoltzsche

As the change is now merged into develop, what happens to the issue here? Will/can you @mgoltzsche close it (as you opened it) or will @badaix have to do this?

And should this happen now or when the feature is promoted from develop to master only?

nis65 avatar Jul 04 '22 08:07 nis65

My usual process is to mark it as "next release", once it is in develop and to close it when it's merged into master and releases. So I will not forget to mention it in the release notes and anyone wanting a similar feature will see it and know that it will be part of the next release

badaix avatar Jul 04 '22 16:07 badaix

Perfect.So I'll await the next release 🙂 .

nis65 avatar Jul 04 '22 21:07 nis65

Fixed in snapcast v0.27.0

badaix avatar Feb 05 '23 12:02 badaix