Date: Sat, 18 Jun 2022 14:07:41 -0700 From: Kyle Evans <kevans@freebsd.org> To: Ravi Pokala <rpokala@freebsd.org> Cc: src-committers <src-committers@freebsd.org>, "<dev-commits-src-all@freebsd.org>" <dev-commits-src-all@freebsd.org>, dev-commits-src-main@freebsd.org Subject: Re: 4014365e4219 - main - mixer: remove volume backwards compat, add % interpretation Message-ID: <CACNAnaFyAoYcQCdOmNcjmVwnF0gM-wtf0Sb3k1=9rppTy4FGJg@mail.gmail.com> In-Reply-To: <7D9CA81D-1688-4B99-97ED-EB40A60EBF60@panasas.com> References: <202206180353.25I3rkKc096344@gitrepo.freebsd.org> <7D9CA81D-1688-4B99-97ED-EB40A60EBF60@panasas.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Jun 18, 2022 at 1:26 PM Ravi Pokala <rpokala@freebsd.org> wrote:
>
> H Kyle,
>
> | +Note that relative percentages are still relative to 1.0, not to the c=
urrent
> | +value.
> | +If the volume is currently 0.40 and an adjustment of +20% is specified=
, then
> | +thet final volume will be set to 0.60.
>
> What you're describing are *absolute* percentages, not relative. Please r=
eword accordingly.
>
Recommendations welcome... it's a relative adjustment by an absolute
percentage. The first "relative percentage" occurrence is because one
is doing, e.g., +10% or -10%, so it seems more intuitive for that
instance to be qualified as 'relative' (because the end result is a
10% increase, not 10%).
> Also, "thet" =3D> ("the" || "that").
>
Whoops.
> Thanks,
>
> Ravi (rpokala@)
>
> =EF=BB=BF-----Original Message-----
> From: <owner-src-committers@freebsd.org> on behalf of Kyle Evans <kevans@=
FreeBSD.org>
> Date: 2022-06-17, Friday at 20:53
> To: <src-committers@FreeBSD.org>, <dev-commits-src-all@FreeBSD.org>, <dev=
-commits-src-main@FreeBSD.org>
> Subject: git: 4014365e4219 - main - mixer: remove volume backwards compat=
, add % interpretation
>
> The branch main has been updated by kevans:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=3D4014365e42199181470324=
9d4748d6dcac6686b6
>
> commit 4014365e421991814703249d4748d6dcac6686b6
> Author: Kyle Evans <kevans@FreeBSD.org>
> AuthorDate: 2022-04-30 03:12:56 +0000
> Commit: Kyle Evans <kevans@FreeBSD.org>
> CommitDate: 2022-06-18 03:50:58 +0000
>
> mixer: remove volume backwards compat, add % interpretation
>
> The current situation is fairly confusing, where an integer is in=
terpreted
> as a percent until you slap a decimal on it and magically it beco=
mes an
> absolute value.
>
> Let's have a flag day in 14.0 and remove this shim entirely. Set=
ting with
> percent can still be useful, so allow a trailing '%' to indicate =
as such.
> As a side effect, we tighten down the format allowed in the volum=
e a little
> bit by ensuring there's no trailing garbage after the value once =
it's
> separated into left and right components.
>
> Reviewed by: christos, hselasky, pauamma_gundo.com (manpages)
> Relnotes: yes
> Differential Revision: https://reviews.freebsd.org/D35101
> ---
> sbin/devd/apple.conf | 4 ++--
> sbin/devd/asus.conf | 8 ++++----
> share/man/man4/acpi_ibm.4 | 2 +-
> usr.bin/fortune/datfiles/freebsd-tips | 2 +-
> usr.bin/usbhidaction/usbhidaction.1 | 14 +++++++-------
> usr.sbin/mixer/mixer.8 | 19 ++++++++++++-------
> usr.sbin/mixer/mixer.c | 23 +++++++++++++++--------
> 7 files changed, 42 insertions(+), 30 deletions(-)
>
> diff --git a/sbin/devd/apple.conf b/sbin/devd/apple.conf
> index 0729a9c86e7a..0a9143f7b5a5 100644
> --- a/sbin/devd/apple.conf
> +++ b/sbin/devd/apple.conf
> @@ -52,7 +52,7 @@ notify 0 {
> match "subsystem" "keys";
> match "type" "volume";
> match "notify" "down";
> - action "mixer vol.volume=3D-10";
> + action "mixer vol.volume=3D-10%";
> };
>
> notify 0 {
> @@ -60,7 +60,7 @@ notify 0 {
> match "subsystem" "keys";
> match "type" "volume";
> match "notify" "up";
> - action "mixer vol.volume=3D+10";
> + action "mixer vol.volume=3D+10%";
> };
>
> # Eject key
> diff --git a/sbin/devd/asus.conf b/sbin/devd/asus.conf
> index eed369f6ca4d..0074e7a7c55f 100644
> --- a/sbin/devd/asus.conf
> +++ b/sbin/devd/asus.conf
> @@ -14,14 +14,14 @@ notify 0 {
> match "system" "ACPI";
> match "subsystem" "ASUS";
> match "notify" "0x31";
> - action "mixer vol.volume=3D-10";
> + action "mixer vol.volume=3D-10%";
> };
>
> notify 0 {
> match "system" "ACPI";
> match "subsystem" "ASUS";
> match "notify" "0x30";
> - action "mixer vol.volume=3D+10";
> + action "mixer vol.volume=3D+10%";
> };
>
> # The next blocks enable volume hotkeys that can be found on the Asu=
s EeePC
> @@ -36,14 +36,14 @@ notify 0 {
> match "system" "ACPI";
> match "subsystem" "ASUS-Eee";
> match "notify" "0x14";
> - action "mixer vol.volume=3D-10";
> + action "mixer vol.volume=3D-10%";
> };
>
> notify 0 {
> match "system" "ACPI";
> match "subsystem" "ASUS-Eee";
> match "notify" "0x15";
> - action "mixer vol.volume=3D+10";
> + action "mixer vol.volume=3D+10%";
> };
>
> # Enable user hotkeys that can be found on the Asus EeePC
> diff --git a/share/man/man4/acpi_ibm.4 b/share/man/man4/acpi_ibm.4
> index 290c7d3b29c1..434a1a7cba2c 100644
> --- a/share/man/man4/acpi_ibm.4
> +++ b/share/man/man4/acpi_ibm.4
> @@ -455,7 +455,7 @@ case ${NOTIFY} in
> fi
> if [ $LEVEL -eq 1 ]; then
> sysctl dev.acpi_ibm.0.mic_led=3D0
> - mixer rec.volume=3D30
> + mixer rec.volume=3D30%
> fi
> ;;
> *)
> diff --git a/usr.bin/fortune/datfiles/freebsd-tips b/usr.bin/fortune/=
datfiles/freebsd-tips
> index e8fa84e02489..930110a0c3fe 100644
> --- a/usr.bin/fortune/datfiles/freebsd-tips
> +++ b/usr.bin/fortune/datfiles/freebsd-tips
> @@ -339,7 +339,7 @@ If you have sudo(8) installed and permissions to =
use it, type
> ``<ESC>w ! sudo tee %'' to force a write.
> %
> You can adjust the volume of various parts of the sound system in yo=
ur
> -computer by typing 'mixer <type>.volume=3D<volume>'. To get a list =
of what
> +computer by typing 'mixer <type>.volume=3D<volume>%'. To get a list=
of what
> you can adjust, just type 'mixer'.
> %
> You can automatically download and install binary packages by doing
> diff --git a/usr.bin/usbhidaction/usbhidaction.1 b/usr.bin/usbhidacti=
on/usbhidaction.1
> index f50af8fff1ad..54d114925714 100644
> --- a/usr.bin/usbhidaction/usbhidaction.1
> +++ b/usr.bin/usbhidaction/usbhidaction.1
> @@ -139,11 +139,11 @@ The following configuration file can be used to=
control a pair
> of Philips USB speakers with the HID controls on the speakers.
> .Bd -literal -offset indent
> # Configuration for various Philips USB speakers
> -Consumer:Volume_Increment 1 0 mixer -f $1 vol.volume=3D+1
> -Consumer:Volume_Decrement 1 0 mixer -f $1 vol.volume=3D-1
> +Consumer:Volume_Increment 1 0 mixer -f $1 vol.volume=3D+1%
> +Consumer:Volume_Decrement 1 0 mixer -f $1 vol.volume=3D-1%
> Consumer:Mute 1 0 mixer -f $1 vol.mute=
=3D^
> -Consumer:Channel_Top.Microsoft:Base_Up 1 0 mixer -f $1 bass.vol=
ume=3D+1
> -Consumer:Channel_Top.Microsoft:Base_Down 1 0 mixer -f $1 bass.volume=
=3D-1
> +Consumer:Channel_Top.Microsoft:Base_Up 1 0 mixer -f $1 bass.vol=
ume=3D+1%
> +Consumer:Channel_Top.Microsoft:Base_Down 1 0 mixer -f $1 bass.volume=
=3D-1%
> .Ed
> .Pp
> A sample invocation using this configuration would be
> @@ -153,9 +153,9 @@ A sample invocation using this configuration woul=
d be
> The following example controls the mixer volume using a Logitech Win=
gman.
> Notice the debounce of 1 for buttons and 5 for the slider.
> .Bd -literal -offset indent
> -Button:Button_1 1 1 mixer vol.volume=3D+10
> -Button:Button_2 1 1 mixer vol.volume=3D-10
> -Generic_Desktop:Z * 5 mixer vol.volume=3D`echo $V | awk '{print=
int($$1/255*100)}'`
> +Button:Button_1 1 1 mixer vol.volume=3D+10%
> +Button:Button_2 1 1 mixer vol.volume=3D-10%
> +Generic_Desktop:Z * 5 mixer vol.volume=3D`echo $V | awk '{print=
f("%.02f", $$1/255)}'`
> .Ed
> .Sh SEE ALSO
> .Xr usbhidctl 1 ,
> diff --git a/usr.sbin/mixer/mixer.8 b/usr.sbin/mixer/mixer.8
> index 284750538f7e..9a76cbe41f65 100644
> --- a/usr.sbin/mixer/mixer.8
> +++ b/usr.sbin/mixer/mixer.8
> @@ -21,7 +21,7 @@
> .\"
> .\" $FreeBSD$
> .\"
> -.Dd March 20, 2022
> +.Dd April 29, 2022
> .Dt MIXER 8
> .Os
> .Sh NAME
> @@ -112,8 +112,8 @@ with one of the available devices):
> .It Sy Name Ta Sy Value
> .It Ar dev Cm .volume Ta Xo
> .Ar vol |
> -.Oo Cm \&+ | Cm \&- Oc Ar lvol
> -.Oo Cm \&: Oo Cm \&+ | Cm \&- Oc Ar rvol Oc
> +.Oo Cm \&+ | Cm \&- Oc Ar lvol Oo % Oc
> +.Oo Cm \&: Oo Cm \&+ | Cm \&- Oc Ar rvol Oo % Oc Oc
> .Xc
> .It Ar dev Cm .mute Ta Cm 0 | 1 | ^
> .It Ar dev Cm .recsrc Ta Cm ^ | + | - | =3D
> @@ -128,16 +128,21 @@ The optional
> and/or
> .Ar rvol
> values have to be specified.
> -The values have to be normalized 32-bit floats, from 0.0 to 1.0 incl=
usively.
> -If no
> -.Ql \&.
> -character is present, the value is treated like a percentage, for ba=
ckwards compatibility.
> +The values should typically be decimal numbers between 0 and 1 with =
at most 2
> +digits after the decimal point.
> +A trailing percent sign indicates that the value should be treated a=
s a
> +percentage of 1.0, rather than an absolute value.
> +Thus, 70% means the same as 0.7.
> If the left or right volume values are prefixed with
> .Cm +
> or
> .Cm - ,
> the value following will be used as a relative adjustment, modifying=
the
> current settings by the amount specified.
> +Note that relative percentages are still relative to 1.0, not to the=
current
> +value.
> +If the volume is currently 0.40 and an adjustment of +20% is specifi=
ed, then
> +thet final volume will be set to 0.60.
> .Pp
> Volume can also be set using the shorthand
> .Ar dev Ns Cm =3Dvalue .
> diff --git a/usr.sbin/mixer/mixer.c b/usr.sbin/mixer/mixer.c
> index c0a9ec25c6fa..e216efe3313c 100644
> --- a/usr.sbin/mixer/mixer.c
> +++ b/usr.sbin/mixer/mixer.c
> @@ -341,7 +341,7 @@ mod_volume(struct mix_dev *d, void *p)
> mix_ctl_t *cp;
> mix_volume_t v;
> const char *val;
> - char lstr[8], rstr[8];
> + char *endp, lstr[8], rstr[8];
> float lprev, rprev, lrel, rrel;
> int n;
>
> @@ -356,25 +356,32 @@ mod_volume(struct mix_dev *d, void *p)
> lrel =3D rrel =3D 0;
> if (n > 0) {
> if (*lstr =3D=3D '+' || *lstr =3D=3D '-')
> - lrel =3D rrel =3D 1;
> - v.left =3D strtof(lstr, NULL);
> + lrel =3D 1;
> + v.left =3D strtof(lstr, &endp);
> + if (*endp !=3D '\0' && (*endp !=3D '%' || *(endp + 1) !=
=3D '\0')) {
> + warnx("invalid volume value: %s", lstr);
> + return (-1);
> + }
>
> - /* be backwards compatible */
> - if (strstr(lstr, ".") =3D=3D NULL)
> + if (*endp =3D=3D '%')
> v.left /=3D 100.0f;
> }
> if (n > 1) {
> if (*rstr =3D=3D '+' || *rstr =3D=3D '-')
> rrel =3D 1;
> - v.right =3D strtof(rstr, NULL);
> + v.right =3D strtof(rstr, &endp);
> + if (*endp !=3D '\0' && (*endp !=3D '%' || *(endp + 1) !=
=3D '\0')) {
> + warnx("invalid volume value: %s", rstr);
> + return (-1);
> + }
>
> - /* be backwards compatible */
> - if (strstr(rstr, ".") =3D=3D NULL)
> + if (*endp =3D=3D '%')
> v.right /=3D 100.0f;
> }
> switch (n) {
> case 1:
> v.right =3D v.left; /* FALLTHROUGH */
> + rrel =3D lrel;
> case 2:
> if (lrel)
> v.left +=3D m->dev->vol.left;
>
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CACNAnaFyAoYcQCdOmNcjmVwnF0gM-wtf0Sb3k1=9rppTy4FGJg>