Re: [pulseaudio-discuss] pactl percentage bug fix
On Sat, 2022-06-04 at 23:05 -0700, Sean Greenslade wrote: > On Sun, Jun 05, 2022 at 01:51:55AM +0100, Patrick May wrote: > > On 04/06/2022 08:48, Sean Greenslade wrote: > > > > > While debugging, I did notice another issue that I was uncertain of how > > > to solve. The detection of absolute values vs. relative values is based > > > on the presence of a plus or minus at the start of the value. This is > > > fine for everything except decibels, since absolute decibel values will > > > often be negative numbers. The way the code sits now, it's impossible to > > > pass an absolute negative decibel value. Probably not a huge deal, but > > > it seemed worth pointing out. > > > > I tested it with > > > > pactl -- set-sink-volume 0 0db > > > > and it sets the volume to 100%. So the ability to specify absolute decibel > > values is only useful if you like blasting stuff at greater than or equal to > > 100% volume > > As I said, it's a bug. When I said it's probably not a huge deal, I > meant in the sense that I doubt many people have ever tried to use the > decibel versions of this command, seeing as this bug makes the absolute > decibel inputs nearly useless. > > My reason for bringing it up was to try and solicit opinions on how best > to fix it. The current method of assuming a "-" always means relative is > not good, but I can't think of a clean way to fix it. Less clean options > that jump to mind: > - Add a new command, e.g. "set-sink-decibels-absolute -3dB" > - Add a new suffix variant, e.g. "set-sink-volume -3dBAbs" > (note that "dBA" is not acceptable, since that has a specific meaning > with regards to sound measurement) > - Have a prefix for absolute numbers, e.g. "set-sink-volume =-3dB" > > What do other people think? There probably aren't any nice-looking solutions to this... My suggestion would be to use pactl set-sink-volume --absolute/--relative sinkname -3dB -- Tanu
Re: [pulseaudio-discuss] pactl percentage bug fix
On Sun, Jun 05, 2022 at 01:51:55AM +0100, Patrick May wrote: > On 04/06/2022 08:48, Sean Greenslade wrote: > > > While debugging, I did notice another issue that I was uncertain of how > > to solve. The detection of absolute values vs. relative values is based > > on the presence of a plus or minus at the start of the value. This is > > fine for everything except decibels, since absolute decibel values will > > often be negative numbers. The way the code sits now, it's impossible to > > pass an absolute negative decibel value. Probably not a huge deal, but > > it seemed worth pointing out. > > I tested it with > > pactl -- set-sink-volume 0 0db > > and it sets the volume to 100%. So the ability to specify absolute decibel > values is only useful if you like blasting stuff at greater than or equal to > 100% volume As I said, it's a bug. When I said it's probably not a huge deal, I meant in the sense that I doubt many people have ever tried to use the decibel versions of this command, seeing as this bug makes the absolute decibel inputs nearly useless. My reason for bringing it up was to try and solicit opinions on how best to fix it. The current method of assuming a "-" always means relative is not good, but I can't think of a clean way to fix it. Less clean options that jump to mind: - Add a new command, e.g. "set-sink-decibels-absolute -3dB" - Add a new suffix variant, e.g. "set-sink-volume -3dBAbs" (note that "dBA" is not acceptable, since that has a specific meaning with regards to sound measurement) - Have a prefix for absolute numbers, e.g. "set-sink-volume =-3dB" What do other people think? --Sean
Re: [pulseaudio-discuss] pactl percentage bug fix
I tested it with pactl -- set-sink-volume 0 0db and it sets the volume to 100%. So the ability to specify absolute decibel values is only useful if you like blasting stuff at greater than or equal to 100% volume On 04/06/2022 08:48, Sean Greenslade wrote: While debugging, I did notice another issue that I was uncertain of how to solve. The detection of absolute values vs. relative values is based on the presence of a plus or minus at the start of the value. This is fine for everything except decibels, since absolute decibel values will often be negative numbers. The way the code sits now, it's impossible to pass an absolute negative decibel value. Probably not a huge deal, but it seemed worth pointing out. --Sean
Re: [pulseaudio-discuss] pactl percentage bug fix
On Fri, Jun 03, 2022 at 06:19:20PM +0100, Patrick May wrote: > Here is the problem: > > https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/blob/master/src/utils/pactl.c > > In src/utils/pactl.c: > > Between line 2530 and line 2535, in function parse_volume: > > These two 'if' statements will BOTH execute if the input string contains > both a '.' and ends with a '%'. > > This results in vol_flags having VOL_PERCENT and VOL_LINEAR or'd in and set, > which is equivalent in effect to '*vol_flags |= VOL_DECIBEL' > (because VOL_PERCENT is 1 and VOL_LINEAR is 2, and VOL_DECIBEL is 3. Refer > to line 76) > So from then on the function is erroneously acting as if the input string > was a decibel value, instead of a percentage. > > Suggested fix: > Change line 2532 from this > if (pa_endswith(vs, "%")) { > To this: > else if (pa_endswith(vs, "%")) { > > I apologise for my rudeness earlier but this was not the first time > pulseaudio bugs have BLASTED EXTREMELY LOUD NOISE AT 100% THROUGH MY > HEADPHONES and I was very upset. > > I don't have a git account or any of the stuff necessary to actually submit > a patch so I would appreciate if someone or pulseaudio dev would take note > of this and make the necessary change I dug into this issue and found a couple of different bugs in this code. I've submitted a merge request with my fixes: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/merge_requests/716 While debugging, I did notice another issue that I was uncertain of how to solve. The detection of absolute values vs. relative values is based on the presence of a plus or minus at the start of the value. This is fine for everything except decibels, since absolute decibel values will often be negative numbers. The way the code sits now, it's impossible to pass an absolute negative decibel value. Probably not a huge deal, but it seemed worth pointing out. --Sean
[pulseaudio-discuss] pactl percentage bug fix
Here is the problem: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/blob/master/src/utils/pactl.c In src/utils/pactl.c: Between line 2530 and line 2535, in function parse_volume: These two 'if' statements will BOTH execute if the input string contains both a '.' and ends with a '%'. This results in vol_flags having VOL_PERCENT and VOL_LINEAR or'd in and set, which is equivalent in effect to '*vol_flags |= VOL_DECIBEL' (because VOL_PERCENT is 1 and VOL_LINEAR is 2, and VOL_DECIBEL is 3. Refer to line 76) So from then on the function is erroneously acting as if the input string was a decibel value, instead of a percentage. Suggested fix: Change line 2532 from this if (pa_endswith(vs, "%")) { To this: else if (pa_endswith(vs, "%")) { I apologise for my rudeness earlier but this was not the first time pulseaudio bugs have BLASTED EXTREMELY LOUD NOISE AT 100% THROUGH MY HEADPHONES and I was very upset. I don't have a git account or any of the stuff necessary to actually submit a patch so I would appreciate if someone or pulseaudio dev would take note of this and make the necessary change