Date: Fri, 30 Jun 2006 06:47:38 GMT From: Ryan Beasley <ryanb@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 100341 for review Message-ID: <200606300647.k5U6lcKe099492@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=100341 Change 100341 by ryanb@ryanb_yuki on 2006/06/30 06:46:40 I misinterpreted 4Front's definition of "audio device" as it related to each card's channels, so this was corrected. SNDCTL_SYSINFO and SNDCTL_AUDIOINFO were updated to meet the fixed scheme. Caveats: - /dev/dsp nodes are created when a sound card registers its channels. While instances of snddev_channel reference created nodes, the link is seemingly only cosmetic. For example, when an application opens a device node, the sound subsystem doesn't work on the channel that created said device. Instead, it scans for /any/ available channel. IOW, the data returned by SYSINFO and AUDIOINFO, at the individual channel level, are inaccurate. - Right now, locking on my new code is broken or non-existent. I'll fix that in the next day or two. Affected files ... .. //depot/projects/soc2006/rbeasley_sound/sys/dev/sound/pcm/dsp.c#8 edit .. //depot/projects/soc2006/rbeasley_sound/sys/dev/sound/pcm/sound.c#6 edit Differences ... ==== //depot/projects/soc2006/rbeasley_sound/sys/dev/sound/pcm/dsp.c#8 (text+ko) ==== @@ -1306,6 +1306,15 @@ * 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, however, that the usefulness of this command is significantly + * decreased when requesting info about any device other than the one serving + * the request. While each snddev_channel refers to a specific device node, + * the converse is *not* true. Currently, when a sound device node is opened, + * the sound subsystem scans for an available audio channel (or channels, if + * opened in read+write) and then assigns them to the si_drv[12] private + * data fields. As a result, any information returned linking a channel to + * a specific character device isn't really accurate. * * @param dev device on which the ioctl was issued * @param ai ioctl request data container @@ -1314,46 +1323,55 @@ * @retval EINVAL ai->dev specifies an invalid device * * @todo Verify correctness of Doxygen tags. ;) + * @todo Ask about device and channel locking; with D_NEEDGIANT, am I safe + * for now? + * @todo Discuss rectifying the channel/cdev relationship. OSS will return + * EBUSY if applications attempt to open a DSP device more than once. */ int dsp_oss_audioinfo(struct cdev *i_dev, oss_audioinfo *ai) { + struct snddev_channel *sce; + struct pcmchan_caps *caps; + struct pcm_channel *ch; struct snddev_info *d; - struct cdev *t_cdev; /* handle 1 of target device */ - device_t t_dev; /* handle 2 */ - int t_unit = 0; /* unit # of target device */ + struct cdev *t_cdev; + int i, nchan; + + /* If probing handling device, make sure it's a DSP device. */ + if ((ai->dev == -1) && (i_dev->si_devsw != &dsp_cdevsw)) + return EINVAL; + + ch = NULL; + t_cdev = NULL; + nchan = 0; - /* - * Searching by unit number, get device_t and struct cdev handles - * on target audio device. - */ - if (ai->dev == -1) { - t_unit = PCMUNIT(i_dev); - t_cdev = i_dev; + /* Search for the requested audio device. */ + for (i = 0; i < devclass_get_maxunit(pcm_devclass); i++) { + d = devclass_get_softc(pcm_devclass, i); + if (d == NULL) + continue; - if ((i_dev->si_devsw == &dsp_cdevsw) && - (t_unit < devclass_get_maxunit(pcm_devclass))) { - t_dev = devclass_get_device(pcm_devclass, t_unit); - } else { - /* Not an audio device */ - return EINVAL; - } - } else { - t_dev = devclass_get_device(pcm_devclass, ai->dev); - if (t_dev == NULL) - return EINVAL; - t_unit = device_get_unit(t_dev); - - LIST_FOREACH(t_cdev, &dsp_cdevsw.d_devs, si_list) { - if (PCMUNIT(t_cdev) == t_unit) - break; + SLIST_FOREACH(sce, &d->channels, link) { + ch = sce->channel; + if (ai->dev == -1) { + if ((ch == i_dev->si_drv1) || + (ch == i_dev->si_drv2)) { + t_cdev = i_dev; + goto dspfound; + } + } else if (ai->dev == nchan) { + t_cdev = sce->dsp_devt; + goto dspfound; + } + ++nchan; } - - if (t_cdev == NULL) - return EINVAL; } +dspfound: + if (t_cdev == NULL) + return EINVAL; - d = dsp_get_info(t_cdev); + caps = chn_getcaps(ch); /* * With all handles collected, zero out the user's container and @@ -1361,24 +1379,29 @@ */ bzero((void *)ai, sizeof(oss_audioinfo)); - ai->dev = t_unit; - strlcpy(ai->name, device_get_desc(t_dev), sizeof(ai->name)); + strlcpy(ai->name, ch->name, sizeof(ai->name)); + + if ((ch->flags & CHN_F_BUSY) == 0) + ai->busy = 0; + else + ai->busy = (ch->direction == PCMDIR_PLAY) ? OPEN_WRITE : OPEN_READ; + /** - * @todo @c busy requires examining all channels - * * @note - * @c pid - OSSv4 docs: "Value of -1 means that this information is - * not available or the device is currently not open." Since - * multiple processes may open a device, I'm going with the - * former. - * @par - * @c cmd - Same caveat as @c pid. + * @c cmd - OSSv4 docs: "Only supported under Linux at this moment." + * Cop-out, I know, but I'll save running around in the process + * table for later. Is there a risk of leaking information? */ - ai->pid = -1; + ai->pid = ch->pid; + /** - * @todo @c caps - requires going directly to sound card driver - * @todo @c iformats - same todo as @c caps - * @todo @c oformats - same todo as @c caps + * @todo @c caps - Should take the SNDCTL_DSP_GETCAPS route. + * Question: Since no drivers actually use the caps + * field of pcmchan_caps, why not store DSP_CAP_* + * values there? + * @todo @c iformats - chn_getformats includes sw emulated formats, + * but cmd is supposed to ask hardware directly + * @todo @c oformats - same as iformats * * @note * @c magic - OSSv4 docs: "Reserved for internal use by OSS." @@ -1389,6 +1412,11 @@ * Applications should normally not use this field for any * purpose." */ + if (ch->direction == PCMDIR_PLAY) + ai->iformats = chn_getformats(ch); + else + ai->oformats = chn_getformats(ch); + ai->card_number = -1; /** * @todo @c song_name - depends first on SNDCTL_[GS]ETSONG @@ -1397,19 +1425,21 @@ */ ai->port_number = -1; ai->mixer_dev = (d->mixer_dev != NULL) ? PCMUNIT(d->mixer_dev) : -1; - ai->real_device = t_unit; + /** + * @note + * @c real_device - OSSv4 docs: "Obsolete." + */ + ai->real_device = -1; strlcpy(ai->devnode, t_cdev->si_name, sizeof(ai->devnode)); - ai->enabled = device_is_attached(t_dev) ? 1 : 0; + ai->enabled = device_is_attached(d->dev) ? 1 : 0; /** * @note * @c flags - OSSv4 docs: "Reserved for future use." * - * @todo @c min_rate - same todo as @c caps - * @todo @c max_rate - same todo as @c caps - * @todo @c nrates - same todo as @c caps - * @todo @c rates - same todo as @c caps - * @todo @c min_channels - same todo as @c caps - * @todo @c max_channels - same todo as @c caps + * @todo @c nrates - need new interface to query hw driver directly + * @todo @c rates - same todo as @c nrates + * @todo @c min_channels, @c max_channels - A little more research + * is required. * * @note * @c binding - OSSv4 docs: "Reserved for future use." @@ -1417,6 +1447,8 @@ * @todo @c handle - haven't decided how to generate this yet; bus, * vendor, device IDs? */ + ai->min_rate = caps->minspeed; + ai->max_rate = caps->maxspeed; return 0; } ==== //depot/projects/soc2006/rbeasley_sound/sys/dev/sound/pcm/sound.c#6 (text+ko) ==== @@ -1139,13 +1139,49 @@ { static char si_product[] = "FreeBSD native OSS ABI"; static char si_version[] = __XSTRING(__FreeBSD_version); + static int intnbits = sizeof(int) * 8; /* Better suited as macro? + Must pester a C guru. */ + + struct snddev_channel *sce; + struct snddev_info *d; + struct pcm_channel *c; + int i, j, ncards; + + ncards = 0; strlcpy(si->product, si_product, sizeof(si->product)); strlcpy(si->version, si_version, sizeof(si->version)); si->versionnum = SOUND_VERSION; - si->numaudios = (pcm_devclass != NULL) ? - devclass_get_count(pcm_devclass) : - 0; + + /* + * Iterate over PCM devices and their channels, gathering up data + * for the numaudios, ncards, and openedaudio fields. + */ + si->numaudios = 0; + bzero((void *)&si->openedaudio, sizeof(si->openedaudio)); + + if (pcm_devclass != NULL) { + j = 0; + + for (i = 0; i < devclass_get_maxunit(pcm_devclass); i++) { + d = devclass_get_softc(pcm_devclass, i); + if (!d) + continue; + si->numaudios += d->devcount; + ++ncards; + + SLIST_FOREACH(sce, &d->channels, link) { + c = sce->channel; + CHN_LOCK(c); + if (c->flags & CHN_F_BUSY) + si->openedaudio[j / intnbits] |= + (1 << (j % intnbits)); + CHN_UNLOCK(c); + j++; + } + } + } + si->numsynths = 0; /* OSSv4 docs: this field is obsolete */ /** * @todo Collect num{midis,timers}. @@ -1161,24 +1197,29 @@ si->nummidis = 0; si->numtimers = 0; si->nummixers = mixer_count; - si->numcards = 0; /* OSSv4 docs: Intended only for test apps; - API doesn't really have much of a concept - of cards. Shouldn't be used by - applications. */ + si->numcards = ncards; + /* OSSv4 docs: Intended only for test apps; API doesn't + really have much of a concept of cards. Shouldn't be + used by applications. */ /** * @todo Fill in "busy devices" fields. * - * si->openedaudio = bitmask of open/busy audio devices * si->openedmidi = " MIDI devices */ - bzero((void *)&si->openedaudio, sizeof(si->openedaudio)); bzero((void *)&si->openedmidi, sizeof(si->openedmidi)); - /* - * XXX Si->filler is a reserved array, but according to docs each + /** + * @todo Ask about altering oss_sysinfo definition to use a macro + * for size of the filler field. Doing so would deviate + * in appearance from 4Front's soundcard.h, and userland + * developers may think that the macro is official. + * + * Si->filler is a reserved array, but according to docs each * element should be set to -1. */ + for (i = 0; i < sizeof(si->filler)/sizeof(si->filler[0]); i++) + si->filler[i] = -1; } #endif /* !OSSV4_EXPERIMENT */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200606300647.k5U6lcKe099492>