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

droid-sink: use UI volume for voice volume #119

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

peat-psuwit
Copy link

set_voice_volume() accepts value from 0.0 to 1.0 of unspecified scale. Previously, we were using pa_sw_volume_to_linear() as an input for this function, but it turns out that the volume wasn't gradual (the voice call remained silent until ~50% UI volume).

From reading Android code, it seems like the input of this function scales linearly with the UI value. Thus, it's better to feed the UI- based value to this function. Even though I still can't find if PA's volume (cubic) is the same scale as set_voice_volume() (and thus the dB value in PA could be incorrect), at least from my brief testing it already sounds better.

@Thaodan Thaodan requested a review from jusa February 27, 2023 19:04
Copy link
Contributor

@jusa jusa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add following to the commit body:

[droid-sink] Use UI volume for voice volume. Fixes JB#60853

@peat-psuwit peat-psuwit force-pushed the for-upstream/fp4-voice-volume branch from f9269b0 to bdbb85c Compare June 7, 2023 09:53
@peat-psuwit
Copy link
Author

Done, and rebased. Please take another look.

@jusa
Copy link
Contributor

jusa commented Jun 7, 2023

Done, and rebased. Please take another look.

Sorry I probably wasn't clear enough, I meant to add it to the body of message, so the commit message would be

droid-sink: Use UI volume for voice volume.

`set_voice_volume()` accepts value from 0.0 to 1.0 of unspecified
scale. Previously, we were using `pa_sw_volume_to_linear()` as an input
for this function, but it turns out that the volume wasn't gradual (the
voice call remained silent until ~50% UI volume).

From reading Android code, it seems like the input of this function
scales linearly with the UI value. Thus, it's better to feed the UI-
based value to this function. Even though I still can't find if PA's
volume (cubic) is the same scale as `set_voice_volume()` (and thus the
dB value in PA could be incorrect), at least from my brief testing it
already sounds better.

[droid-sink] Use UI volume for voice volume. Fixes JB#60853

`set_voice_volume()` accepts value from 0.0 to 1.0 of unspecified
scale. Previously, we were using `pa_sw_volume_to_linear()` as an input
for this function, but it turns out that the volume wasn't gradual (the
voice call remained silent until ~50% UI volume).

From reading Android code, it seems like the input of this function
scales linearly with the UI value. Thus, it's better to feed the UI-
based value to this function. Even though I still can't find if PA's
volume (cubic) is the same scale as `set_voice_volume()` (and thus the
dB value in PA could be incorrect), at least from my brief testing it
already sounds better.

[droid-sink] Use UI volume for voice volume. Fixes JB#60853
@peat-psuwit peat-psuwit force-pushed the for-upstream/fp4-voice-volume branch from bdbb85c to 5a12ab1 Compare June 7, 2023 10:17
@peat-psuwit
Copy link
Author

Oh Ok. Does this look good now?

jusa
jusa previously approved these changes Jun 7, 2023
@jusa jusa dismissed their stale review June 7, 2023 11:13

I need to look a bit more into this first.

@jusa
Copy link
Contributor

jusa commented Jun 7, 2023

Heh, it's been some time since I was last working with pulseaudio things so I didn't think through the first time. :)

So here goes, the default volume steps for voice call are really not that great and pretty much are legacy dating from Jolla1 times. So they don't really map at all for anything recent. They can (and thus should) be defined for each adaptation. For reference you could check for example how Xperia 10 III has been set up.

https://github.com/mer-hybris/droid-config-sony-lena/blob/master/sparse/var/lib/nemo-pulseaudio-parameters/algs/mainvolume

Since different devices may have different volume curves for voice afaik I'm not sure if some better generic defaults would be ok, at least we should increase the steps to more than 6 (which again is legacy from Jolla1 times, where there were actually only 6 different volume levels for voicecalls in the Jolla1 hal implementation).

Anyways, I think it should be possible to figure out usable and good steps using the mainvolume configuration, let me know if you have any issues there.

@peat-psuwit
Copy link
Author

Err... not sure I follow you here.

This patch originates from Ubuntu Touch, which doesn't have 'MainVolume' module to define each step's value. We assume that Pulseaudio's volume slider will do the sane thing and causes the volume to gradually increase (perceptually).

If, however, Pulseaudio needs another table to define the sane steps, then my feeling is that PA or its modules are doing something wrong. In this case, I believe the wrong doing is in passing a "linear" volume to set_voice_volume(), because it's likely the wrong scale.

You'll notice that, in the example file you linked, the differences of each step for call is different, but the differences between each step for media is the same. However, if I understand the code correctly, this is supposed to control the same PA software volume. This makes me feel like the 'MainVolume' table is a workaround for the broken scale translation.


But Ok, I figured out that if you do merge this, then it'll be a breaking change in SailfishOS that every port will have to fix. So... not sure how to proceed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants