Implement parameters for volume shell event

[Kucheek]

Is your feature request related to a problem? Please describe.

I'm using hifiberry sound card which comes with integrated volume control. Sound quality-wise, it is preferred to use it instead of software volume control which reduces signal-to-noise ratio. As I learned from a brief search, Java sound API cannot directly access ALSA mixer controls which are used to manage volume implemented in sound card, so it provides a gain setting which is a software solution.

Describe the solution you'd like

To use integrated volume control of sound card, I believe onVolumeChanged shell event #329 combined with a bypassSinkVolume setting #317 could be a good way. Amixer provides command line interface to control the volume, e.g.:

amixer set 'Digital' 5%- - lower the volume by 5%
amixer set 'Digital' 5%+ - raise the volume by 5%
amixer set 'Digital' 60% - set the volume to 60%

What I'm missing is either:

• Existing VolumeChanged shell event will be replaced with some VolumeLowered and VolumeRaised events accepting a parameter calculated as 100/volumeSteps so that I could call "amixer set 'Digital' $1%-" or "amixer set 'Digital' $1%+" respectively.
• To be able to pass a parameter into the shell command configured for VolumeChanged event. This parameter will be having an integer, 0-100 value set by librespot-java converted from and matching a current volume setting so that I could call "amixer set 'Digital' $1%"

Option 1 (the relative volume changes) is preferred to avoid sudden volume jumps when triggering volume change from librespot-java because it does not read the current ALSA control volume level. On my machine I am using other applications which modify volume using alsa mixer (squeezelite for instance) and they can leave the setting eg. at desired quiet level of 20% and when I switch to librespot-java which I previously left for example at volume 40000/65536, I don't want to be deafened by jumping to somewhere around 60% by setting exact value.

Describe alternatives you've considered
I did not think of any so far. I could always use gain volume control but it collides with volume setting done by other apps that change alsa mixer volume.

Additional context
none

[devgianlu]

• What about environment variables instead of arguments? It looks much cleaner as I can pass in a bunch of stuff and you can pick only what you need.
• Having the volume adjusted by increments or decrements could lead to librespot-java and the system being out of sync. Can we find another solution?

I have already implemented the environment variables thing by adding VOLUME to onVolumeChanged as the current volume from 0 to 100.

[Kucheek]

Sure, environment variables are OK, I guess I overthought it a bit. Will try using VOLUME and I see that onVolumeChanged is fired upon established connection, so I think I'm safe with appropriate initialVolume value as a starting point.

[Kucheek]

I've built based on your commit, but no success so far. In my config file, I have the entry configured as follows:

onVolumeChanged = "amixer set 'Digital' $VOLUME%"

I see related log entries:

2021-05-27 15:52:13,912 TRACE ShellEvents:52 - Executed shell command: amixer set 'Digital' $VOLUME% -> 0

I might be using the env variable the wrong way. When I configure onVolumeChanged = "amixer set 'Digital' 5%+", I am able to raise the volume from librespot-java just fine.

[devgianlu]

The environment variable is set correctly (tested on Windows). You can try applying this patch to have the output of the command printed out:

Index: player/src/main/java/xyz/gianlu/librespot/player/ShellEvents.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/player/src/main/java/xyz/gianlu/librespot/player/ShellEvents.java b/player/src/main/java/xyz/gianlu/librespot/player/ShellEvents.java
--- a/player/src/main/java/xyz/gianlu/librespot/player/ShellEvents.java	(revision 90a53e64f27305ced9b38abdee1569a932702a5a)
+++ b/player/src/main/java/xyz/gianlu/librespot/player/ShellEvents.java	(date 1622217616821)
@@ -25,7 +25,9 @@
 import xyz.gianlu.librespot.core.Session;
 import xyz.gianlu.librespot.metadata.PlayableId;
 
+import java.io.BufferedReader;
 import java.io.IOException;
+import java.io.InputStreamReader;
 
 /**
  * @author devgianlu
@@ -40,6 +42,14 @@
         this.conf = conf;
     }
 
+    public static void printResults(Process process) throws IOException {
+        BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream()));
+        String line = "";
+        while ((line = reader.readLine()) != null) {
+            System.out.println(line);
+        }
+    }
+
     private void exec(String command, String... envp) {
         if (!this.conf.enabled)
             return;
@@ -48,8 +58,11 @@
             return;
 
         try {
-            int exitCode = runtime.exec(command.trim(), envp).waitFor();
+            Process p =  runtime.exec(command.trim(), envp);
+            int exitCode =p.waitFor();
             LOGGER.trace("Executed shell command: {} -> {}", command, exitCode);
+            
+            printResults(p);
         } catch (IOException | InterruptedException ex) {
             LOGGER.error("Failed executing command: {}", command, ex);
         }

Even if ShellEvents:52 - Executed shell command: amixer set 'Digital' $VOLUME% -> 0 is already telling you the command exited with 0.

[Kucheek]

Thanks, I've applied the patch but from the output and printed result everything seemed like the env variable is not filled properly with value on my linux machine. With onVolumeChanged = "echo $VOLUME", I was getting log entries:

Executed shell command: echo $VOLUME -> 0
$VOLUME

I tried on Windows as you pointed out that you have it working there just fine and I was getting actual volume values properly. So I started to suspect the root cause is *nix related and did some research. Found this article which explained the issue for me. Appears the exec() method does not act as bash so it does not resolve the env variable notation and takes $VOLUME as plaintext. So as I understood, instead using bash command directly, one has to execute bash itself with the command as a parameter. So I tried to configure onVolumeChanged = "/bin/bash -c \"echo $VOLUME\"" but error code "1" was returned. I played around with the quotes escaping, tried few variants but none worked.

In the end I modified your code a bit and got it working this way:

    private void exec(String command, String... envp) {
        if (!this.conf.enabled)
            return;

        if (command == null || command.trim().isEmpty())
            return;

        try {
            String[] cmd = {"/bin/bash", "-c", command.trim()};
            Process p =  runtime.exec(cmd, envp);
            int exitCode =p.waitFor();
            LOGGER.trace("Executed shell command: {} -> {}", command, exitCode);
            
            printResults(p);
        } catch (IOException | InterruptedException ex) {
            LOGGER.error("Failed executing command: {}", command, ex);
        }
    }

With the above code, I am able to use onVolumeChanged = "amixer set Digital $VOLUME%" and manage the master ALSA volume from within librespot-java. But I guess this would again ruin the functionality for win users so I leave it up to you to decide how to manage it. Probably some OS recognition? Or might be the bash call can be properly included directly in the onVolumeChanged command only I was not able to do so.

[devgianlu]

Thank you for the debugging and great explanation! I have added a configuration option to execute the commands with bash -c (see shell.executeWithBash).