NetBSD Problem Report #54264
From nia@netbsd.org Sun Jun 2 13:31:36 2019
Return-Path: <nia@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-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 1983A7A1AB
for <gnats-bugs@gnats.NetBSD.org>; Sun, 2 Jun 2019 13:31:36 +0000 (UTC)
Message-Id: <20190602133135.C9ED284D72@mail.netbsd.org>
Date: Sun, 2 Jun 2019 13:31:35 +0000 (UTC)
From: nia@netbsd.org
Reply-To: nia@netbsd.org
To: gnats-bugs@NetBSD.org
Subject: audio(4) API fails to detect a capture-only device
X-Send-Pr-Version: 3.95
>Number: 54264
>Category: kern
>Synopsis: audio(4) API fails to detect a capture-only device
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: isaki
>State: closed
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Sun Jun 02 13:35:00 +0000 2019
>Closed-Date: Thu Jun 06 13:51:25 +0000 2019
>Last-Modified: Thu Jun 06 13:51:25 +0000 2019
>Originator: Nia Alarie
>Release: NetBSD 8.99.42
>Organization:
>Environment:
>Description:
I'm testing with a USB microphone, which is /dev/audio1.
First I query the device with AUDIO_GETPROPS. It claims that the
device has playback support. This is incorrect.
Then I query the device with AUDIO_GETFORMAT, which only fills the
record struct with valid data, and fills the playback struct with
garbage.
Currently, I can't figure out how to detect a USB microphone other
than by hoping that the garbage in the prinfo struct is out of range.
>How-To-Repeat:
>Fix:
>Release-Note:
>Audit-Trail:
From: Tetsuya Isaki <isaki@pastel-flower.jp>
To: gnats-bugs@netbsd.org
Cc: kern-bug-people@netbsd.org,
gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: kern/54264: audio(4) API fails to detect a capture-only device
Date: Tue, 04 Jun 2019 21:34:31 +0900
At Sun, 2 Jun 2019 13:35:00 +0000 (UTC),
nia@netbsd.org wrote:
> First I query the device with AUDIO_GETPROPS. It claims that the
> device has playback support. This is incorrect.
Please try this patch.
uaudio doesn't return correct property on unidirectional device.
current audio (isaki-audio2) requires that the hardware driver's
get_props() returns exact property.
nakayama@-san,
dev/audio/audio.c,v 1.5 and 1.7 look wrong. It assumes that the
hardware driver may return incorrect property. But this assumption
is not right (and not good). The condition
(unidirection && full duplex), or
(unidirection && the device can set play and rec independently)
are obviously strange and such hardware driver should be fixed.
Would you test this patch?
--- sys/dev/usb/uaudio.c
+++ sys/dev/usb/uaudio.c
@@ -2264,8 +2264,21 @@ uaudio_round_blocksize(void *addr, int blk,
Static int
uaudio_get_props(void *addr)
{
- return AUDIO_PROP_FULLDUPLEX | AUDIO_PROP_INDEPENDENT;
+ struct uaudio_softc *sc;
+ int props;
+
+ sc = addr;
+ props = 0;
+ if ((sc->sc_mode & AUMODE_PLAY))
+ props |= AUDIO_PROP_PLAYBACK;
+ if ((sc->sc_mode & AUMODE_RECORD))
+ props |= AUDIO_PROP_CAPTURE;
+
+ /* XXX I'm not sure all bidirectional devices support FULLDUP&INDEP */
+ if (props == (AUDIO_PROP_PLAYBACK | AUDIO_PROP_CAPTURE))
+ props |= AUDIO_PROP_FULLDUPLEX | AUDIO_PROP_INDEPENDENT;
+ return props;
}
Static void
Index: sys/dev/audio/audio.c
===================================================================
RCS file: /cvsroot/src/sys/dev/audio/audio.c,v
retrieving revision 1.5
retrieving revision 1.4
diff -u -r1.5 -r1.4
--- sys/dev/audio/audio.c 13 May 2019 04:11:04 -0000 1.5
+++ sys/dev/audio/audio.c 13 May 2019 04:09:35 -0000 1.4
@@ -6149,23 +6149,17 @@
"invalid mode = %x", mode);
if (is_indep) {
- int errorp = 0, errorr = 0;
-
/* On independent devices, probe separately. */
if ((mode & AUMODE_PLAY) != 0) {
- errorp = audio_hw_probe_fmt(sc, phwfmt, AUMODE_PLAY);
- if (errorp)
+ error = audio_hw_probe_fmt(sc, phwfmt, AUMODE_PLAY);
+ if (error)
mode &= ~AUMODE_PLAY;
}
if ((mode & AUMODE_RECORD) != 0) {
- errorr = audio_hw_probe_fmt(sc, rhwfmt, AUMODE_RECORD);
- if (errorr)
+ error = audio_hw_probe_fmt(sc, rhwfmt, AUMODE_RECORD);
+ if (error)
mode &= ~AUMODE_RECORD;
}
-
- /* Return error if both play and record probes failed. */
- if (errorp && errorr)
- error = errorp;
} else {
/* On non independent devices, probe simultaneously. */
error = audio_hw_probe_fmt(sc, &fmt, mode);
Index: sys/dev/audio/audio.c
===================================================================
RCS file: /cvsroot/src/sys/dev/audio/audio.c,v
retrieving revision 1.7
retrieving revision 1.6
diff -u -r1.7 -r1.6
--- sys/dev/audio/audio.c 13 May 2019 08:50:25 -0000 1.7
+++ sys/dev/audio/audio.c 13 May 2019 07:53:56 -0000 1.6
@@ -2046,17 +2046,12 @@
* hw_if->open() is always (FREAD | FWRITE)
* regardless of this open()'s flags.
* see also dev/isa/aria.c
- * but ckeck its playback or recording capability.
* On half duplex hardware, the flags passed to
* hw_if->open() is either FREAD or FWRITE.
* see also arch/evbarm/mini2440/audio_mini2440.c
*/
if (fullduplex) {
hwflags = FREAD | FWRITE;
- if (!audio_can_playback(sc))
- hwflags &= ~FWRITE;
- if (!audio_can_capture(sc))
- hwflags &= ~FREAD;
} else {
/* Construct hwflags from af->mode. */
hwflags = 0;
---
Tetsuya Isaki <isaki@pastel-flower.jp / isaki@NetBSD.org>
From: nia <nia@netbsd.org>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: kern/54264: audio(4) API fails to detect a capture-only device
Date: Wed, 5 Jun 2019 12:45:11 +0000
On Tue, Jun 04, 2019 at 12:40:02PM +0000, Tetsuya Isaki wrote:
> Please try this patch.
> uaudio doesn't return correct property on unidirectional device.
> current audio (isaki-audio2) requires that the hardware driver's
> get_props() returns exact property.
Thank you. This makes AUDIO_GETPROPS correctly identify my USB
microphone as capture-only.
I also tested with a record/playback uaudio device, and hdafg.
Both were also identified correctly.
From: Takeshi Nakayama <nakayama@NetBSD.org>
To: isaki@pastel-flower.jp
Cc: gnats-bugs@netbsd.org, kern-bug-people@netbsd.org, gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: kern/54264: audio(4) API fails to detect a capture-only device
Date: Thu, 06 Jun 2019 10:25:54 +0900 (JST)
>>> Tetsuya Isaki <isaki@pastel-flower.jp> wrote
> At Sun, 2 Jun 2019 13:35:00 +0000 (UTC),
> nia@netbsd.org wrote:
> > First I query the device with AUDIO_GETPROPS. It claims that the
> > device has playback support. This is incorrect.
>
> Please try this patch.
> uaudio doesn't return correct property on unidirectional device.
> current audio (isaki-audio2) requires that the hardware driver's
> get_props() returns exact property.
>
> nakayama@-san,
> dev/audio/audio.c,v 1.5 and 1.7 look wrong. It assumes that the
> hardware driver may return incorrect property. But this assumption
> is not right (and not good). The condition
> (unidirection && full duplex), or
> (unidirection && the device can set play and rec independently)
> are obviously strange and such hardware driver should be fixed.
> Would you test this patch?
The patch for uaudio.c works well on my USB speaker.
Rev.1.7 of audio.c is workaround, so I agree with reverting it.
But I don't think rev.1.5 is completely wrong. Variable error is
used for result of both audio_hw_probe_fmt(..., AUMODE_PLAY) and
audio_hw_probe_fmt(..., AUMODE_RECORD), so the result for
AUMODE_PLAY is overwritten by its for AUMODE_RECORD.
I think both errors should be checked and audio_hw_probe returns
error if both are in error as of rev.1.5, or if one or the other is
in error.
Also, if such a strange hardware driver should be fixed, I think
that you should add KASSERT or else to detect such a driver.
-- Takeshi Nakayama
Responsible-Changed-From-To: kern-bug-people->isaki
Responsible-Changed-By: isaki@NetBSD.org
Responsible-Changed-When: Thu, 06 Jun 2019 12:49:13 +0000
Responsible-Changed-Why:
I take it.
From: Tetsuya Isaki <isaki@pastel-flower.jp>
To: Takeshi Nakayama <nakayama@NetBSD.org>
Cc: gnats-bugs@netbsd.org,
kern-bug-people@netbsd.org,
gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: kern/54264: audio(4) API fails to detect a capture-only device
Date: Thu, 06 Jun 2019 21:48:04 +0900
At Thu, 06 Jun 2019 10:25:54 +0900 (JST),
Takeshi Nakayama wrote:
> The patch for uaudio.c works well on my USB speaker.
>
> Rev.1.7 of audio.c is workaround, so I agree with reverting it.
Thank you.
> But I don't think rev.1.5 is completely wrong. Variable error is
> used for result of both audio_hw_probe_fmt(..., AUMODE_PLAY) and
> audio_hw_probe_fmt(..., AUMODE_RECORD), so the result for
> AUMODE_PLAY is overwritten by its for AUMODE_RECORD.
>
> I think both errors should be checked and audio_hw_probe returns
> error if both are in error as of rev.1.5, or if one or the other is
> in error.
Oh, it's my wrong about rev1.5, sorry.
> Also, if such a strange hardware driver should be fixed, I think
> that you should add KASSERT or else to detect such a driver.
Yeah I will do so.
(I should have written about it in my previous mail..)
Thanks,
---
Tetsuya Isaki <isaki@pastel-flower.jp / isaki@NetBSD.org>
From: "Tetsuya Isaki" <isaki@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/54264 CVS commit: src/sys/dev/usb
Date: Thu, 6 Jun 2019 12:59:33 +0000
Module Name: src
Committed By: isaki
Date: Thu Jun 6 12:59:33 UTC 2019
Modified Files:
src/sys/dev/usb: uaudio.c
Log Message:
Return correct properties.
It fixes PR kern/54264.
XXX I'm not sure all bidirectional uaudio devices support FULLDUPLEX
or INDEPENDENT.
To generate a diff of this commit:
cvs rdiff -u -r1.160 -r1.161 src/sys/dev/usb/uaudio.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/54264 CVS commit: src/sys/dev/audio
Date: Thu, 6 Jun 2019 13:08:30 +0000
Module Name: src
Committed By: isaki
Date: Thu Jun 6 13:08:30 UTC 2019
Modified Files:
src/sys/dev/audio: audio.c
Log Message:
Revert rev1.7.
Don't take care of incorrect drivers. They should be fixed.
PR kern/54264. OK'ed by nakayama@-san.
To generate a diff of this commit:
cvs rdiff -u -r1.10 -r1.11 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: open->closed
State-Changed-By: isaki@NetBSD.org
State-Changed-When: Thu, 06 Jun 2019 13:51:25 +0000
State-Changed-Why:
Commited.
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.43 2018/01/16 07:36:43 maya Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2017
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.
Home