Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 23 May 2024 00:58:05 GMT
From:      Christos Margiolis <christos@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: e07f9178502b - main - sound: Separate implementations for SNDCTL_AUDIOINFO[_EX] and SNDCTL_ENGINEINFO
Message-ID:  <202405230058.44N0w5qL071834@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by christos:

URL: https://cgit.FreeBSD.org/src/commit/?id=e07f9178502b7cbc0769fc10e99ad0d013f437fd

commit e07f9178502b7cbc0769fc10e99ad0d013f437fd
Author:     Christos Margiolis <christos@FreeBSD.org>
AuthorDate: 2024-05-23 00:57:04 +0000
Commit:     Christos Margiolis <christos@FreeBSD.org>
CommitDate: 2024-05-23 00:57:04 +0000

    sound: Separate implementations for SNDCTL_AUDIOINFO[_EX] and SNDCTL_ENGINEINFO
    
    FreeBSD's implementation of SNDCTL_AUDIOINFO[_EX] and SNDCTL_ENGINEINFO
    does not exactly work as intended. The problem is essentially that both
    IOCTLs return the same information, while in fact the information
    returned currently by dsp_oss_audioinfo() is what _only_
    SNDCTL_ENGINEINFO is meant to return.
    
    This behavior is also noted in the OSS manual [1] (see bold paragraph in
    "Audio engines and device files" section), but since e8c0d15a64fa
    ("sound: Get rid of snd_clone and use DEVFS_CDEVPRIV(9)") we can
    actually fix this, because we now expose only a single device for each
    soundcard, and create the engines (channels) internally.
    SNDCTL_ENGINEINFO will now report info about all channels in a given
    device, and SNDCTL_AUDIOINFO[_EX] will only report information about
    /dev/dspX.
    
    To make this work, we also have to modify the SNDCTL_SYSINFO IOCTL to
    report the number of audio devices and audio engines correctly.
    
    While here, modernize the minimum and maximum channel counting in both
    SNDCTL_AUDIOINFO[_EX] and SNDCTL_ENGINEINFO. Currently these IOCTLs will
    report only up to 2 channels, which is no longer the case.
    
    [1] http://manuals.opensound.com/developer/SNDCTL_AUDIOINFO.html
    
    PR:             246231, 252761
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 day
    Reviewed by:    dev_submerge.ch
    Differential Revision:  https://reviews.freebsd.org/D45164
---
 sys/dev/sound/pcm/dsp.c   | 179 +++++++++++++++++++++++++++++++++++++++++-----
 sys/dev/sound/pcm/dsp.h   |   3 +-
 sys/dev/sound/pcm/mixer.c |   8 ++-
 sys/dev/sound/pcm/sound.c |   8 +--
 4 files changed, 173 insertions(+), 25 deletions(-)

diff --git a/sys/dev/sound/pcm/dsp.c b/sys/dev/sound/pcm/dsp.c
index 6e5fad048d40..4ee5d7c6738c 100644
--- a/sys/dev/sound/pcm/dsp.c
+++ b/sys/dev/sound/pcm/dsp.c
@@ -778,9 +778,15 @@ dsp_ioctl(struct cdev *i_dev, u_long cmd, caddr_t arg, int mode,
 			ret = sound_oss_card_info((oss_card_info *)arg);
 			break;
 		case SNDCTL_AUDIOINFO:
+			ret = dsp_oss_audioinfo(i_dev, (oss_audioinfo *)arg,
+			    false);
+			break;
 		case SNDCTL_AUDIOINFO_EX:
+			ret = dsp_oss_audioinfo(i_dev, (oss_audioinfo *)arg,
+			    true);
+			break;
 		case SNDCTL_ENGINEINFO:
-			ret = dsp_oss_audioinfo(i_dev, (oss_audioinfo *)arg);
+			ret = dsp_oss_engineinfo(i_dev, (oss_audioinfo *)arg);
 			break;
 		case SNDCTL_MIXERINFO:
 			ret = mixer_oss_mixerinfo(i_dev, (oss_mixerinfo *)arg);
@@ -2034,8 +2040,152 @@ dsp_unit2name(char *buf, size_t len, struct pcm_channel *ch)
 	return (NULL);
 }
 
+/**
+ * @brief Handler for SNDCTL_AUDIOINFO.
+ *
+ * Gathers information about the audio device specified in ai->dev.  If
+ * ai->dev == -1, then this function gathers information about the current
+ * device.  If the call comes in on a non-audio device and ai->dev == -1,
+ * return EINVAL.
+ *
+ * This routine is supposed to go practically straight to the hardware,
+ * getting capabilities directly from the sound card driver, side-stepping
+ * the intermediate channel interface.
+ *
+ * @note
+ * Calling threads must not hold any snddev_info or pcm_channel locks.
+ *
+ * @param dev		device on which the ioctl was issued
+ * @param ai		ioctl request data container
+ * @param ex		flag to distinguish between SNDCTL_AUDIOINFO from
+ *			SNDCTL_AUDIOINFO_EX
+ *
+ * @retval 0		success
+ * @retval EINVAL	ai->dev specifies an invalid device
+ */
+int
+dsp_oss_audioinfo(struct cdev *i_dev, oss_audioinfo *ai, bool ex)
+{
+	struct pcmchan_caps *caps;
+	struct pcm_channel *ch;
+	struct snddev_info *d;
+	uint32_t fmts;
+	int i, minch, maxch, unit;
+
+	/*
+	 * If probing the device that received the ioctl, make sure it's a
+	 * DSP device.  (Users may use this ioctl with /dev/mixer and
+	 * /dev/midi.)
+	 */
+	if (ai->dev == -1 && i_dev->si_devsw != &dsp_cdevsw)
+		return (EINVAL);
+
+	for (unit = 0; pcm_devclass != NULL &&
+	    unit < devclass_get_maxunit(pcm_devclass); unit++) {
+		d = devclass_get_softc(pcm_devclass, unit);
+		if (!PCM_REGISTERED(d)) {
+			d = NULL;
+			continue;
+		}
+
+		PCM_UNLOCKASSERT(d);
+		PCM_LOCK(d);
+		if ((ai->dev == -1 && d->dsp_dev == i_dev) ||
+		    (ai->dev == unit)) {
+			PCM_UNLOCK(d);
+			break;
+		} else {
+			PCM_UNLOCK(d);
+			d = NULL;
+		}
+	}
+
+	/* Exhausted the search -- nothing is locked, so return. */
+	if (d == NULL)
+		return (EINVAL);
+
+	/* XXX Need Giant magic entry ??? */
+
+	PCM_UNLOCKASSERT(d);
+	PCM_LOCK(d);
+
+	bzero((void *)ai, sizeof(oss_audioinfo));
+	ai->dev = unit;
+	strlcpy(ai->name, device_get_desc(d->dev), sizeof(ai->name));
+	ai->pid = -1;
+	ai->card_number = -1;
+	ai->port_number = -1;
+	ai->mixer_dev = (d->mixer_dev != NULL) ? unit : -1;
+	ai->legacy_device = unit;
+	snprintf(ai->devnode, sizeof(ai->devnode), "/dev/dsp%d", unit);
+	ai->enabled = device_is_attached(d->dev) ? 1 : 0;
+	ai->next_play_engine = 0;
+	ai->next_rec_engine = 0;
+	ai->busy = 0;
+	ai->caps = PCM_CAP_REALTIME | PCM_CAP_MMAP | PCM_CAP_TRIGGER;
+	ai->iformats = 0;
+	ai->oformats = 0;
+	ai->min_rate = INT_MAX;
+	ai->max_rate = 0;
+	ai->min_channels = INT_MAX;
+	ai->max_channels = 0;
+
+	/* Gather global information about the device. */
+	CHN_FOREACH(ch, d, channels.pcm) {
+		CHN_UNLOCKASSERT(ch);
+		CHN_LOCK(ch);
+
+		/*
+		 * Skip physical channels if we are servicing SNDCTL_AUDIOINFO,
+		 * or VCHANs if we are servicing SNDCTL_AUDIOINFO_EX.
+		 */
+		if ((ex && (ch->flags & CHN_F_VIRTUAL) != 0) ||
+		    (!ex && (ch->flags & CHN_F_VIRTUAL) == 0)) {
+			CHN_UNLOCK(ch);
+			continue;
+		}
+
+		if ((ch->flags & CHN_F_BUSY) == 0) {
+			ai->busy |= (ch->direction == PCMDIR_PLAY) ?
+			    OPEN_WRITE : OPEN_READ;
+		}
+
+		ai->caps |=
+		    ((ch->flags & CHN_F_VIRTUAL) ? PCM_CAP_VIRTUAL : 0) |
+		    ((ch->direction == PCMDIR_PLAY) ? PCM_CAP_OUTPUT :
+		    PCM_CAP_INPUT);
+
+		caps = chn_getcaps(ch);
+
+		minch = INT_MAX;
+		maxch = 0;
+		fmts = 0;
+		for (i = 0; caps->fmtlist[i]; i++) {
+			fmts |= AFMT_ENCODING(caps->fmtlist[i]);
+			minch = min(AFMT_CHANNEL(caps->fmtlist[i]), minch);
+			maxch = max(AFMT_CHANNEL(caps->fmtlist[i]), maxch);
+		}
+
+		if (ch->direction == PCMDIR_PLAY)
+			ai->oformats |= fmts;
+		else
+			ai->iformats |= fmts;
+
+		ai->min_rate = min(ai->min_rate, caps->minspeed);
+		ai->max_rate = max(ai->max_rate, caps->maxspeed);
+		ai->min_channels = min(ai->min_channels, minch);
+		ai->max_channels = max(ai->max_channels, maxch);
+
+		CHN_UNLOCK(ch);
+	}
+
+	PCM_UNLOCK(d);
+
+	return (0);
+}
+
 static int
-dsp_oss_audioinfo_cb(void *data, void *arg)
+dsp_oss_engineinfo_cb(void *data, void *arg)
 {
 	struct dsp_cdevpriv *priv = data;
 	struct pcm_channel *ch = arg;
@@ -2047,10 +2197,10 @@ dsp_oss_audioinfo_cb(void *data, void *arg)
 }
 
 /**
- * @brief Handler for SNDCTL_AUDIOINFO.
+ * @brief Handler for SNDCTL_ENGINEINFO
  *
- * Gathers information about the audio device specified in ai->dev.  If
- * ai->dev == -1, then this function gathers information about the current
+ * Gathers information about the audio device's engine specified in ai->dev.
+ * If ai->dev == -1, then this function gathers information about the current
  * device.  If the call comes in on a non-audio device and ai->dev == -1,
  * return EINVAL.
  *
@@ -2066,11 +2216,9 @@ dsp_oss_audioinfo_cb(void *data, void *arg)
  *
  * @retval 0		success
  * @retval EINVAL	ai->dev specifies an invalid device
- *
- * @todo Verify correctness of Doxygen tags.  ;)
  */
 int
-dsp_oss_audioinfo(struct cdev *i_dev, oss_audioinfo *ai)
+dsp_oss_engineinfo(struct cdev *i_dev, oss_audioinfo *ai)
 {
 	struct pcmchan_caps *caps;
 	struct pcm_channel *ch;
@@ -2113,7 +2261,7 @@ dsp_oss_audioinfo(struct cdev *i_dev, oss_audioinfo *ai)
 			CHN_LOCK(ch);
 			if (ai->dev == -1) {
 				if (devfs_foreach_cdevpriv(i_dev,
-				    dsp_oss_audioinfo_cb, ch) != 0) {
+				    dsp_oss_engineinfo_cb, ch) != 0) {
 					devname = dsp_unit2name(buf,
 					    sizeof(buf), ch);
 				}
@@ -2182,18 +2330,13 @@ dsp_oss_audioinfo(struct cdev *i_dev, oss_audioinfo *ai)
 			 * if any channel is mono, minch = 1;
 			 * and if all channels are mono, maxch = 1.
 			 */
-			minch = 0;
+			minch = INT_MAX;
 			maxch = 0;
 			fmts = 0;
 			for (i = 0; caps->fmtlist[i]; i++) {
-				fmts |= caps->fmtlist[i];
-				if (AFMT_CHANNEL(caps->fmtlist[i]) > 1) {
-					minch = (minch == 0) ? 2 : minch;
-					maxch = 2;
-				} else {
-					minch = 1;
-					maxch = (maxch == 0) ? 1 : maxch;
-				}
+				fmts |= AFMT_ENCODING(caps->fmtlist[i]);
+				minch = min(AFMT_CHANNEL(caps->fmtlist[i]), minch);
+				maxch = max(AFMT_CHANNEL(caps->fmtlist[i]), maxch);
 			}
 
 			if (ch->direction == PCMDIR_PLAY)
diff --git a/sys/dev/sound/pcm/dsp.h b/sys/dev/sound/pcm/dsp.h
index b81e60dc19b5..7559e855d4e1 100644
--- a/sys/dev/sound/pcm/dsp.h
+++ b/sys/dev/sound/pcm/dsp.h
@@ -36,6 +36,7 @@ extern struct cdevsw dsp_cdevsw;
 int dsp_make_dev(device_t);
 void dsp_destroy_dev(device_t);
 char *dsp_unit2name(char *, size_t, struct pcm_channel *);
-int dsp_oss_audioinfo(struct cdev *, oss_audioinfo *);
+int dsp_oss_audioinfo(struct cdev *, oss_audioinfo *, bool);
+int dsp_oss_engineinfo(struct cdev *, oss_audioinfo *);
 
 #endif /* !_PCMDSP_H_ */
diff --git a/sys/dev/sound/pcm/mixer.c b/sys/dev/sound/pcm/mixer.c
index 9811496853c8..130333c7c442 100644
--- a/sys/dev/sound/pcm/mixer.c
+++ b/sys/dev/sound/pcm/mixer.c
@@ -1282,9 +1282,13 @@ mixer_ioctl_cmd(struct cdev *i_dev, u_long cmd, caddr_t arg, int mode,
 		case SNDCTL_CARDINFO:
 			return (sound_oss_card_info((oss_card_info *)arg));
 	    	case SNDCTL_AUDIOINFO:
+			return (dsp_oss_audioinfo(i_dev, (oss_audioinfo *)arg,
+			    false));
 	    	case SNDCTL_AUDIOINFO_EX:
-	    	case SNDCTL_ENGINEINFO:
-			return (dsp_oss_audioinfo(i_dev, (oss_audioinfo *)arg));
+			return (dsp_oss_audioinfo(i_dev, (oss_audioinfo *)arg,
+			    true));
+		case SNDCTL_ENGINEINFO:
+			return (dsp_oss_engineinfo(i_dev, (oss_audioinfo *)arg));
 		case SNDCTL_MIXERINFO:
 			return (mixer_oss_mixerinfo(i_dev, (oss_mixerinfo *)arg));
 		}
diff --git a/sys/dev/sound/pcm/sound.c b/sys/dev/sound/pcm/sound.c
index e66462af2a71..072d42c4e989 100644
--- a/sys/dev/sound/pcm/sound.c
+++ b/sys/dev/sound/pcm/sound.c
@@ -718,9 +718,9 @@ sound_oss_sysinfo(oss_sysinfo *si)
 
 	/*
 	 * Iterate over PCM devices and their channels, gathering up data
-	 * for the numaudios and openedaudio fields.
+	 * for the numaudioengines and openedaudio fields.
 	 */
-	si->numaudios = 0;
+	si->numaudioengines = 0;
 	bzero((void *)&si->openedaudio, sizeof(si->openedaudio));
 
 	j = 0;
@@ -737,7 +737,7 @@ sound_oss_sysinfo(oss_sysinfo *si)
 		PCM_UNLOCKASSERT(d);
 		PCM_LOCK(d);
 
-		si->numaudios += PCM_CHANCOUNT(d);
+		si->numaudioengines += PCM_CHANCOUNT(d);
 
 		CHN_FOREACH(c, d, channels.pcm) {
 			CHN_UNLOCKASSERT(c);
@@ -751,7 +751,6 @@ sound_oss_sysinfo(oss_sysinfo *si)
 
 		PCM_UNLOCK(d);
 	}
-	si->numaudioengines = si->numaudios;
 
 	si->numsynths = 0;	/* OSSv4 docs:  this field is obsolete */
 	/**
@@ -769,6 +768,7 @@ sound_oss_sysinfo(oss_sysinfo *si)
 	si->numtimers = 0;
 	si->nummixers = mixer_count;
 	si->numcards = devclass_get_maxunit(pcm_devclass);
+	si->numaudios = devclass_get_maxunit(pcm_devclass);
 		/* OSSv4 docs:	Intended only for test apps; API doesn't
 		   really have much of a concept of cards.  Shouldn't be
 		   used by applications. */



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202405230058.44N0w5qL071834>