NetBSD Problem Report #56552
From www@netbsd.org Wed Dec 15 21:36:24 2021
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
by mollari.NetBSD.org (Postfix) with ESMTPS id BF1AA1A9239
for <gnats-bugs@gnats.NetBSD.org>; Wed, 15 Dec 2021 21:36:23 +0000 (UTC)
Message-Id: <20211215213622.514A21A923A@mollari.NetBSD.org>
Date: Wed, 15 Dec 2021 21:36:22 +0000 (UTC)
From: rvp@SDF.ORG
Reply-To: rvp@SDF.ORG
To: gnats-bugs@NetBSD.org
Subject: mixerctl(1) does not increment volume with ++ or +=1
X-Send-Pr-Version: www-1.0
>Number: 56552
>Category: bin
>Synopsis: mixerctl(1) does not increment volume with ++ or +=1
>Confidential: no
>Severity: non-critical
>Priority: low
>Responsible: bin-bug-people
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Wed Dec 15 21:40:00 +0000 2021
>Last-Modified: Thu Dec 16 07:40:01 +0000 2021
>Originator: RVP
>Release: NetBSD/amd64 9.99.92
>Organization:
>Environment:
NetBSD x202e.localdomain 9.99.92 NetBSD 9.99.92 (MYKERNEL) #0: Mon Dec 13 03:43:43 UTC 2021 bld@x202e.localdomain:/usr/obj/usr/s
rc/sys/arch/amd64/compile/MYKERNEL amd64
>Description:
On my HW (VIA product 8446) the volume increment delta (5) seems to be
an open interval rather than a closed one, so I see this:
$ mixerctl -w outputs.master++
outputs.master: 162,162 -> 162,162
$ mixerctl -w outputs.master+=1
outputs.master: 162,162 -> 162,162
$
I have to use `+=2' to increment the volume by one "step". Decrementing
works OK however:
$ mixerctl -w outputs.master--
outputs.master: 162,162 -> 156,156
$ mixerctl -w outputs.master-=1
outputs.master: 156,156 -> 150,150
$
>How-To-Repeat:
As above.
>Fix:
Add one to ford interval:
diff -urN a/mixerctl.c b/mixerctl.c
--- a/mixerctl.c 2017-02-23 14:09:11.000000000 +0000
+++ b/mixerctl.c 2021-01-10 08:03:34.758061616 +0000
@@ -230,7 +230,7 @@
return 0;
case AUDIO_MIXER_VALUE:
if (p->infp->un.v.delta)
- inc *= p->infp->un.v.delta;
+ inc *= p->infp->un.v.delta + 1;
for (i = 0; i < m->un.value.num_channels; i++) {
v = m->un.value.level[i];
v += inc;
>Audit-Trail:
From: mlelstv@serpens.de (Michael van Elst)
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: bin/56552: mixerctl(1) does not increment volume with ++ or +=1
Date: Wed, 15 Dec 2021 23:24:22 -0000 (UTC)
rvp@SDF.ORG writes:
>On my HW (VIA product 8446) the volume increment delta (5) seems to be
>an open interval rather than a closed one, so I see this:
>
>$ mixerctl -w outputs.master++
>outputs.master: 162,162 -> 162,162
>$ mixerctl -w outputs.master+=1
>outputs.master: 162,162 -> 162,162
>$
>
>I have to use `+=2' to increment the volume by one "step". Decrementing
>works OK however:
>
>$ mixerctl -w outputs.master--
>outputs.master: 162,162 -> 156,156
>$ mixerctl -w outputs.master-=1
>outputs.master: 156,156 -> 150,150
>$
mixerctl just does what the driver tells it. If the driver says that
a 'step' is 5. Then it increments from 162 to 167 and decrements
from 162 to 157. Then it reads back the value from the driver.
HD-Audio computes the delta value as:
mx[index].mx_di.un.v.delta = 256 / (masterctl->ctl_step + 1);
where ctl_step is a value 0..127, denoting 1..128 volume settings.
So, with 43 to 51 settings (ctl_step=42..50), you get a delta value of 5.
When changing a value it computes the values like:
if (ctl->ctl_step == 0)
divisor = 128; /* ??? - just avoid div by 0 */
else
divisor = 255 / ctl->ctl_step;
hdafg_control_amp_set(ctl, HDAUDIO_AMP_MUTE_NONE,
mc->un.value.level[AUDIO_MIXER_LEVEL_LEFT] / divisor,
mc->un.value.level[AUDIO_MIXER_LEVEL_RIGHT] / divisor);
But with 43 settings (ctl_step is then 42), the divisor is 6.
Reading back the value is done like:
if (ctl->ctl_step == 0)
factor = 128; /* ??? - just avoid div by 0 */
else
factor = 255 / ctl->ctl_step;
mc->un.value.level[AUDIO_MIXER_LEVEL_LEFT] = ctl->ctl_left * factor;
mc->un.value.level[AUDIO_MIXER_LEVEL_RIGHT] = ctl->ctl_right * factor;
So, with 43 settings:
delta = 5
incrementing 162 to 167
writing a hardware value of 167/6 = 27
reading back 27 and computing 27 *6 = 162
decrementing 162 to 157
writing a hardware value of 157/6 = 26
reading back 26 and computing 26 * 6 = 156
decrementing 156 to 151
writing a hardware value of 151/6 = 25
reading back 26 and computing 25 * 6 = 150
From: RVP <rvp@SDF.ORG>
To: gnats-bugs@netbsd.org
Cc: Michael van Elst <mlelstv@serpens.de>
Subject: Re: bin/56552: mixerctl(1) does not increment volume with ++ or
+=1
Date: Thu, 16 Dec 2021 07:38:17 +0000 (UTC)
On Wed, 15 Dec 2021, Michael van Elst wrote:
> HD-Audio computes the delta value as:
>
> mx[index].mx_di.un.v.delta = 256 / (masterctl->ctl_step + 1);
>
> [...]
>
> When changing a value it computes the values like:
>
> if (ctl->ctl_step == 0)
> divisor = 128; /* ??? - just avoid div by 0 */
> else
> divisor = 255 / ctl->ctl_step;
>
> hdafg_control_amp_set(ctl, HDAUDIO_AMP_MUTE_NONE,
> mc->un.value.level[AUDIO_MIXER_LEVEL_LEFT] / divisor,
> mc->un.value.level[AUDIO_MIXER_LEVEL_RIGHT] / divisor);
>
> [...]
>
> Reading back the value is done like:
>
> if (ctl->ctl_step == 0)
> factor = 128; /* ??? - just avoid div by 0 */
> else
> factor = 255 / ctl->ctl_step;
>
> mc->un.value.level[AUDIO_MIXER_LEVEL_LEFT] = ctl->ctl_left * factor;
> mc->un.value.level[AUDIO_MIXER_LEVEL_RIGHT] = ctl->ctl_right * factor;
>
After that concise explanation it was pretty clear what had to be done:
---START---
--- sys/dev/hdaudio/hdafg.c.orig 2020-06-11 02:39:30.000000000 +0000
+++ sys/dev/hdaudio/hdafg.c 2021-12-16 07:04:54.341631494 +0000
@@ -2887,7 +2887,7 @@
mx[index].mx_di.prev = mx[index].mx_di.next = AUDIO_MIXER_LAST;
mx[index].mx_di.un.v.num_channels = 2; /* XXX */
mx[index].mx_di.mixer_class = HDAUDIO_MIXER_CLASS_OUTPUTS;
- mx[index].mx_di.un.v.delta = 256 / (masterctl->ctl_step + 1);
+ mx[index].mx_di.un.v.delta = 256 / (masterctl->ctl_step ? masterctl->ctl_step : 1);
strcpy(mx[index].mx_di.label.name, AudioNmaster);
strcpy(mx[index].mx_di.un.v.units.name, AudioNvolume);
hda_trace(sc, " adding outputs.%s\n",
@@ -2922,7 +2922,7 @@
mx[index].mx_di.type = AUDIO_MIXER_VALUE;
mx[index].mx_di.prev = mx[index].mx_di.next = AUDIO_MIXER_LAST;
mx[index].mx_di.un.v.num_channels = 2; /* XXX */
- mx[index].mx_di.un.v.delta = 256 / (ctl->ctl_step + 1);
+ mx[index].mx_di.un.v.delta = 256 / (ctl->ctl_step ? ctl->ctl_step : 1);
if (ctrlcnt[audiodev] > 0)
snprintf(mx[index].mx_di.label.name,
sizeof(mx[index].mx_di.label.name),
---END---
Now the mixerctl.c patch becomes:
---START---
diff -urN a/mixerctl.c b/mixerctl.c
--- a/mixerctl.c 2017-02-23 14:09:11.000000000 +0000
+++ b/mixerctl.c 2021-12-16 07:20:30.533031095 +0000
@@ -139,6 +139,16 @@
}
static int
+clamp(int vol)
+{
+ if (vol < AUDIO_MIN_GAIN)
+ vol = AUDIO_MIN_GAIN;
+ if (vol > AUDIO_MAX_GAIN)
+ vol = AUDIO_MAX_GAIN;
+ return vol;
+}
+
+static int
rdfield(struct field *p, char *q)
{
mixer_ctrl_t *m;
@@ -181,17 +191,17 @@
case AUDIO_MIXER_VALUE:
if (m->un.value.num_channels == 1) {
if (sscanf(q, "%d", &v) == 1) {
- m->un.value.level[0] = v;
+ m->un.value.level[0] = clamp(v);
} else {
warnx("Bad number %s", q);
return 0;
}
} else {
if (sscanf(q, "%d,%d", &v0, &v1) == 2) {
- m->un.value.level[0] = v0;
- m->un.value.level[1] = v1;
+ m->un.value.level[0] = clamp(v0);
+ m->un.value.level[1] = clamp(v1);
} else if (sscanf(q, "%d", &v) == 1) {
- m->un.value.level[0] = m->un.value.level[1] = v;
+ m->un.value.level[0] = m->un.value.level[1] = clamp(v);
} else {
warnx("Bad numbers %s", q);
return 0;
@@ -234,11 +244,7 @@
for (i = 0; i < m->un.value.num_channels; i++) {
v = m->un.value.level[i];
v += inc;
- if (v < AUDIO_MIN_GAIN)
- v = AUDIO_MIN_GAIN;
- if (v > AUDIO_MAX_GAIN)
- v = AUDIO_MAX_GAIN;
- m->un.value.level[i] = v;
+ m->un.value.level[i] = clamp(v);
}
break;
default:
---END---
I'm clamping the volume everywhere so there's no unwanted wraparound now:
$ ./mixerctl.new -w outputs.master++
outputs.master: 168,168 -> 174,174
$ ./mixerctl.new -w outputs.master+=1
outputs.master: 174,174 -> 180,180
$ ./mixerctl.new -w outputs.master=11111111
outputs.master: 180,180 -> 252,252
$ ./mixerctl.new -w outputs.master=-11111111
outputs.master: 252,252 -> 0,0
$
Thanks!
-RVP
(Contact us)
$NetBSD: query-full-pr,v 1.46 2020/01/03 16:35:01 leot Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2020
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.
Home