Subject: pkg/18741 : kernel or userland issue with mixers?
To: None <tech-kern@netbsd.org>
From: Quentin Garnier <netbsd@quatriemek.com>
List: tech-kern
Date: 09/21/2003 20:13:02
This is a multi-part message in MIME format.

--Multipart_Sun__21_Sep_2003_20:13:02_+0200_0a804600
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

The PR pkg/18741 describes an issue with operation on mixer settings in
mp3blaster: single-step decrement doesn't seem to do anything.

The reason for that is quite simple and not specific to mp3blaster (I had
a report for mplayer, but I haven't checked if it is indeed that same
issue).

The mixer application of mp3blaster increments or decrements by 2 percents
for a single step. This is an issue with AC-97 hardware (and maybe other,
that should be checked too), since mixer values are stored in 5 bits,
which means a single step is roughly 3 percents. Thus a single decrement
in mp3blaster doesn't reach the next value, and nothing happens.

I'm not sure if such applications should be just patched, because
finer-grained settings are usable with some other hardware, like good old
SB16.

I came up with the attached patch that does the following:

1. get current values
2. compare current and new values on 8 bits
3. if the current and new values differ, but the stripped (5 bits) values
   do not, then adjust the new values

Also, in the case 2-channel values are provided for a 1-channel control,
use 'r' value instead of 'l'. Thus l is always the 8 most significant bits
of the resulting value, and the rest of the hack does the right thing in
that case.

That fixes the mixer issue for mp3blaster, and I guess will too for other
applications with that same issue.

A check should be made in order to find whether some other audio drivers
need a similar change.

So, userland or kernel issue?

-- 
Quentin Garnier - cube@cubidou.net
"Feels like I'm fiddling while Rome is burning down.
Should I lay my fiddle down and take a rifle from the ground ?"
Leigh Nash/Sixpence None The Richer, Paralyzed, Divine Discontents, 2002.

--Multipart_Sun__21_Sep_2003_20:13:02_+0200_0a804600
Content-Type: text/plain;
 name="ac97.diff"
Content-Disposition: attachment;
 filename="ac97.diff"
Content-Transfer-Encoding: 7bit

Index: ac97.c
===================================================================
RCS file: /cvsroot/src/sys/dev/ic/ac97.c,v
retrieving revision 1.47
diff -u -r1.47 ac97.c
--- ac97.c	2003/09/11 09:21:29	1.47
+++ ac97.c	2003/09/21 18:00:59
@@ -1149,7 +1149,8 @@
 	case AUDIO_MIXER_VALUE:
 	{
 		const struct audio_mixer_value *value = si->info;
-		u_int16_t  l, r;
+		u_int16_t  l, r, ol, or;
+		int deltal, deltar;
 
 		if ((cp->un.value.num_channels <= 0) ||
 		    (cp->un.value.num_channels > value->num_channels))
@@ -1173,12 +1174,23 @@
 			r = 255 - r;
 		}
 
+		ol = (val >> (8+si->ofs)) & mask;
+		or = (val >> si->ofs) & mask;
+
+		deltal = (ol << (8 - si->bits)) - l;
+		deltar = (or << (8 - si->bits)) - r;
+
 		l = l >> (8 - si->bits);
 		r = r >> (8 - si->bits);
+
+		if (deltal && ol == l)
+			l += (deltal > 0) ? (l ? -1 : 0) : (l < mask ? 1 : 0);
+		if (deltar && or == r)
+			r += (deltar > 0) ? (r ? -1 : 0) : (r < mask ? 1 : 0);
 
-		newval = ((l & mask) << si->ofs);
+		newval = ((r & mask) << si->ofs);
 		if (value->num_channels == 2) {
-			newval = (newval << 8) | ((r & mask) << si->ofs);
+			newval = newval | ((r & mask) << (si->ofs+8));
 			mask |= (mask << 8);
 		}
 		mask = mask << si->ofs;

--Multipart_Sun__21_Sep_2003_20:13:02_+0200_0a804600--