spotifyd icon indicating copy to clipboard operation
spotifyd copied to clipboard

Added alternative to disable changing volume

Open Icelk opened this issue 4 years ago • 9 comments

I added an alternative to "softvol" and "alsa"; "none". If remotely changing volume, or resetting it on restart is undesired, this is a fix.

Icelk avatar Jan 07 '21 11:01 Icelk

Is this a good solution or should I investigate if we can disable remote changing of volume? Maybe change the mixer in librespot to an Option<Mixer>? This just ignores input. Perhaps the best way to move forward is to implement this and later change the librespot API?

Icelk avatar Jan 24 '21 09:01 Icelk

What do you @JojiiOfficial @robinvd @SirWindfield think about this?

Icelk avatar Feb 19 '21 17:02 Icelk

Does anybody know why the lint check is not successful?

Icelk avatar Feb 19 '21 18:02 Icelk

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jul 06 '21 13:07 stale[bot]

Still waiting for response.

Icelk avatar Jul 09 '21 09:07 Icelk

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Nov 21 '21 06:11 stale[bot]

Still waiting for response.

Icelk avatar Nov 23 '21 15:11 Icelk

Looks like all the checks are passing now.

slondr avatar Jan 14 '22 02:01 slondr

I’m a bit confused now. I recently saw in spotify-tui the mobile clients have volume disabled. They advertised it to the Spotify API, and spotify-tui recognises it.

Should we merge this and then use the same struct for disabling it properly (if it’s possible)? If that’s the case, I think librespot needs to support the feature.

Icelk avatar Jan 14 '22 05:01 Icelk

I recently saw in spotify-tui the mobile clients have volume disabled. They advertised it to the Spotify API, and spotify-tui recognises it.

Should we merge this and then use the same struct for disabling it properly (if it’s possible)? If that’s the case, I think librespot needs to support the feature.

@Icelk While looking over the code today, I found by chance the following lines:

https://github.com/Spotifyd/spotifyd/blob/fbcbeca6562f02f53ab5e9771f7d8c2a3c9ae3cf/src/setup.rs#L69-L80

I was wondering, why this struct exists, since the actual volume control is done by the mixer, which has nothing to do with this setting. Apparently, this exists to signal to Spotify clients the type of volume control this device implements. And, most importantly, it has three possible states: Log, Linear and Fixed. I think the latter could be quite interesting to us.

If you're still interested after all this time, this PR has been lying around, I suspect that it might be possible to turn off the client volume control via this setting! If not, that's completely understandable and okay too, of course!

eladyn avatar Mar 06 '23 18:03 eladyn

Thanks for the consideration! You were completely correct.

I rewrote this PR to be up to date with the latest master, and implemented your idea. Thanks for the tip!

Icelk avatar Mar 06 '23 21:03 Icelk

This PR now doesn't seem to have any issues. Can I improve anything @eladyn ?

Icelk avatar Mar 06 '23 21:03 Icelk

Wow! I wouldn't have expected such a fast response after such a long time. Thank you for the effort.

No problem :)

Just some comments on the code that might remove some duplicate code. The rest looks fine!

Just pushed some fixes.

Thanks for your swift responses!

Icelk avatar Mar 08 '23 13:03 Icelk

Thanks!

Icelk avatar Mar 08 '23 18:03 Icelk