Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pipewire: Use decibel-to-linear translation from libaudcore. Closes: #1218 #146

Conversation

radioactiveman
Copy link
Member

No description provided.

@radioactiveman
Copy link
Member Author

radioactiveman commented Dec 17, 2023

@jlindgren90: Did I understand/implement this correctly? The initial bug report description confused me because it says we should use a linear scale (which we do already).

The output plugin saves the volume level (as before) as integer (0 to 100). PipeWire expects the volume as value between 0.0 and 1.0.

Result:
50% leads to 0.1 now (before 0.5)
85% leads to 0.5 now (before 0.85)

And would it be possible to add the helper method to libaudcore for 4.3.2? Or is it not allowed because we have to increase the libversion?

Thanks.

@jlindgren90
Copy link
Member

Thanks, this looks right to me. A sanity check would be just to listen to it as you increase/decrease the volume: every 10% change should subjectively sound like about the "same" relative volume change. I don't have pipewire installed, so if you're happy with how it sounds, please go ahead and merge.

It occurs to me that it may be possible to make use of the existing translation in libaudcore via the audio_amplify function, without adding a new helper. I think this should work and be equivalent to what you have currently (not tested):

diff --git a/src/pipewire/pipewire.cc b/src/pipewire/pipewire.cc
index c2838359e..1a0159502 100644
--- a/src/pipewire/pipewire.cc
+++ b/src/pipewire/pipewire.cc
@@ -138,16 +138,13 @@ void PipeWireOutput::set_volume(StereoVolume v)
 
     float values[m_channels];
 
-    if (m_channels == 2)
-    {
-        values[0] = v.left / 100.0f;
-        values[1] = v.right / 100.0f;
-    }
-    else
-    {
-        for (unsigned int i = 0; i < m_channels; i++)
-            values[i] = aud::max(v.left, v.right) / 100.0f;
-    }
+    // Re-use libaudcore's decibel-to-linear translation by passing
+    // full-scale PCM values (1.0) to audio_amplify(). This also
+    // translates the StereoVolume to an arbitrary number of channels.
+    for (unsigned int i = 0; i < m_channels; i++)
+        values[i] = 1.0f;
+
+    audio_amplify(values, m_channels, 1, v);
 
     pw_thread_loop_lock(m_loop);
     pw_stream_set_control(m_stream, SPA_PROP_channelVolumes, m_channels, values, nullptr);

radioactiveman and others added 2 commits December 18, 2023 18:10
…#1218

Avoid also variable length arrays, officially not supported in C++.

Co-authored-by: John Lindgren <john@jlindgren.net>
They are not officially supported in C++.
@radioactiveman radioactiveman changed the title pipewire: Use logarithmic scale for volume level. Closes: #1218 pipewire: Use decibel-to-linear translation from libaudcore. Closes: #1218 Dec 18, 2023
@jlindgren90
Copy link
Member

LGTM

@radioactiveman
Copy link
Member Author

@jlindgren90: Your suggestion works as expected, thanks. 👍

@radioactiveman radioactiveman merged commit 9154d19 into audacious-media-player:master Dec 18, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants