Re: [pulseaudio-discuss] pactl percentage bug fix

2022-06-06 Thread Tanu Kaskinen
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

2022-06-05 Thread Sean Greenslade
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

2022-06-04 Thread Patrick May

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

2022-06-04 Thread Sean Greenslade
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

2022-06-03 Thread Patrick May

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