NetBSD Problem Report #56308

From mouse@Stone.Rodents-Montreal.ORG  Tue Jul 13 19:27:01 2021
Return-Path: <mouse@Stone.Rodents-Montreal.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 9E4991A921F
	for <gnats-bugs@www.NetBSD.org>; Tue, 13 Jul 2021 19:27:01 +0000 (UTC)
Message-Id: <202107131926.PAA22212@Stone.Rodents-Montreal.ORG>
Date: Tue, 13 Jul 2021 15:26:57 -0400 (EDT)
From: Mouse <mouse@Rodents-Montreal.ORG>
Reply-To: mouse@Rodents-Montreal.ORG
To: gnats-bugs@www.NetBSD.org
Subject: AUDIO_SETINFO can't set play.gain and play.balance together
X-Send-Pr-Version: 3.95

>Number:         56308
>Category:       kern
>Synopsis:       AUDIO_SETINFO can't set play.gain and play.balance together
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    isaki
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Jul 13 19:30:00 +0000 2021
>Closed-Date:    Fri Jul 30 06:03:19 +0000 2021
>Last-Modified:  Fri Jul 30 06:03:19 +0000 2021
>Originator:     Mouse
>Release:        NetBSD 9.1
>Organization:
	Dis-
>Environment:
System: NetBSD Aaeon-9.Rodents-Montreal.ORG 9.1 NetBSD 9.1 (GEN91) #0: Fri Aug 13 14:35:31 EDT 2021  mouse@Aaeon-9.Rodents-Montreal.ORG:/home/mouse/kbuild/GEN91 amd64
Architecture: x86_64
Machine: amd64
>Description:
	When using ioctl(...,AUDIO_SETINFO,...) with both play.gain and
	play.balance specified, the former is ignored.

	This is beacuse audio_hw_setinfo fails to update pgain if
	newpi->gain is specified:

	static int
	audio_hw_setinfo(struct audio_softc *sc, const struct audio_info *newai,
		struct audio_info *oldai)
	{
		u_int pgain;
	...
		/* Backup play.{gain,balance} */
		if (SPECIFIED(newpi->gain) || SPECIFIED_CH(newpi->balance)) {
			au_get_gain(sc, &sc->sc_outports, &pgain, &pbalance);
			if (oldai) {
				oldpi->gain = pgain;
				oldpi->balance = pbalance;
			}
		}
		/* Backup record.{gain,balance} */
		if (SPECIFIED(newri->gain) || SPECIFIED_CH(newri->balance)) {
	...
		}
		if (SPECIFIED(newpi->gain)) {
			error = au_set_gain(sc, &sc->sc_outports,
			    newpi->gain, pbalance);
			if (error) {
	...
			}
		}
		if (SPECIFIED(newri->gain)) {
	...
		}
		if (SPECIFIED_CH(newpi->balance)) {
			error = au_set_gain(sc, &sc->sc_outports,
			    pgain, newpi->balance);

	I find that calling AUDIO_SETINFO twice, once with balance but
	not gain specified and once with gain but not balance
	specified, works as a workaround (which is why I set priority
	to low).

>How-To-Repeat:
	Try it.  In my case, this looked like

	 AUDIO_INITINFO(&ai);
	 ai.play.sample_rate = 8000;
	 ai.play.channels = 1;
	 ai.play.precision = 16;
	 ai.play.encoding = AUDIO_ENCODING_ULINEAR_LE;
	 ai.play.gain = 254;
	 ai.play.port = oai.play.avail_ports;
	 ai.play.pause = 0;
	 ai.play.balance = AUDIO_MID_BALANCE;
	 ai.monitor_gain = 254;
	 ai.mode = AUMODE_PLAY | AUMODE_PLAY_ALL;
	 if (ioctl(fd,AUDIO_SETINFO,&ai) < 0)
	  { fprintf(stderr,"AUDIO_SETINFO: %s\n",strerror(errno));
	    exit(1);
	  }

	with the volume formerly set to something other than 254.
	Verify, by listening to the audio or by checking with
	audioctl(1), that the volume is not actually changed.
>Fix:
	Presumably, the "if (SPECIFIED(newpi->gain))" block needs to
	set pgain to something.  I have not tried this; I have a
	workaround and I'd rather leave figuring out the correct thing
	to set pgain to to someone who knows this code.  (My guess
	would be

	pgain = newpi->gain

	but that is just a guess; I'm not as sure as I'd like that
	pgain and newpi->gain use compatible scales.)

	I suspect recording needs a similar fix, but I have not tried
	that; my use case doesn't record.

/~\ The ASCII				  Mouse
\ / Ribbon Campaign
 X  Against HTML		mouse@rodents-montreal.org
/ \ Email!	     7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B

>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: kern-bug-people->isaki
Responsible-Changed-By: isaki@NetBSD.org
Responsible-Changed-When: Wed, 14 Jul 2021 14:06:48 +0000
Responsible-Changed-Why:
My bug.


From: "Tetsuya Isaki" <isaki@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/56308 CVS commit: src/sys/dev/audio
Date: Wed, 21 Jul 2021 06:14:58 +0000

 Module Name:	src
 Committed By:	isaki
 Date:		Wed Jul 21 06:14:58 UTC 2021

 Modified Files:
 	src/sys/dev/audio: audio.c

 Log Message:
 AUDIO_SETINFO: fix a bug that the gain and the balance could not be set
 at the same time.  Fix PR kern/56308.


 To generate a diff of this commit:
 cvs rdiff -u -r1.104 -r1.105 src/sys/dev/audio/audio.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Tetsuya Isaki" <isaki@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/56308 CVS commit: src/tests/dev/audio
Date: Wed, 21 Jul 2021 06:18:33 +0000

 Module Name:	src
 Committed By:	isaki
 Date:		Wed Jul 21 06:18:32 UTC 2021

 Modified Files:
 	src/tests/dev/audio: audiotest.c

 Log Message:
 Add AUDIO_SETINFO_gain_balance test.
 The test checks whether AUDIO_SETINFO can change the gain and the balance
 at the same time (if MD driver has the capability).  See PR kern/56308.


 To generate a diff of this commit:
 cvs rdiff -u -r1.13 -r1.14 src/tests/dev/audio/audiotest.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->needs-pullups
State-Changed-By: isaki@NetBSD.org
State-Changed-When: Wed, 21 Jul 2021 06:26:28 +0000
State-Changed-Why:
I've just committed the fix.  And I'll pullup to -9 next week.
Thank you for the report and your analysis.


State-Changed-From-To: needs-pullups->pending-pullups
State-Changed-By: isaki@NetBSD.org
State-Changed-When: Tue, 27 Jul 2021 09:11:36 +0000
State-Changed-Why:
[pullup-9 #1325]


From: "Soren Jacobsen" <snj@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/56308 CVS commit: [netbsd-9] src/sys/dev/audio
Date: Wed, 28 Jul 2021 14:59:02 +0000

 Module Name:	src
 Committed By:	snj
 Date:		Wed Jul 28 14:59:02 UTC 2021

 Modified Files:
 	src/sys/dev/audio [netbsd-9]: audio.c

 Log Message:
 Pull up following revision(s) (requested by isaki in ticket #1325):
 	sys/dev/audio/audio.c: revision 1.105
 AUDIO_SETINFO: fix a bug that the gain and the balance could not be set
 at the same time.  Fix PR kern/56308.


 To generate a diff of this commit:
 cvs rdiff -u -r1.28.2.22 -r1.28.2.23 src/sys/dev/audio/audio.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: pending-pullups->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Fri, 30 Jul 2021 06:03:19 +0000
State-Changed-Why:
pulled up, thanks


>Unformatted:

Home	PR Database Search