Sponsored by: The FreeBSD Foundation
MFC after: 2 days
Details
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped - Build Status
Buildable 58656 Build 55544: arc lint + arc unit
Event Timeline
Consider this a WIP.
- This driver can be attached only once, since it's attached on kldload. Is there a point where we'd want to lock the softc?
- Reading/writing doesn't really "work", the channel will timeout after hw.snd.timeout.
I know it's WIP, but it would be nice to have a brief comment or description somewhere, to document what is and what is not implemented / working.
sys/dev/sound/dummy.c | ||
---|---|---|
89–90 | Do we have to free() buf and caps in the error case here? | |
124 | Maybe check that speed is inside the [8000, 96000] range we advertise in the caps? |
- Add man page
- Free buf and caps in chan_init() failure
- Clamp speed between caps->minspeed and caps->maxspeed
sys/dev/sound/dummy.c | ||
---|---|---|
89–90 | In case of failure, chn_init() (the consumer of this function) calls CHANNEL_FREE() only if haven't returned NULL from here, so yes. This leak exists in other hdsp* drivers as well I think. | |
124 | I initially had this check but removed it because callers of CHANNEL_SETSPEED() either clamp speed between caps->minspeed and caps->maxspeed, or use valid speeds (i.e taken directly from the channels) before passing it to CHANNEL_SETSPEED. That being said, I don't think it'd hurt to have the check here as well for robustness. |
Implement IO using callout(9). Playback discards input, recording returns
silence (zeros).
For some reason on my machine having KASAN enabled panics when bzero() is
called from callout_init().
I do not guarantee the stability of this driver, so consider it a WIP until
more testing is done.
sys/dev/sound/dummy.c | ||
---|---|---|
289 | This should be sizeof(struct dummy_softc). |
sys/dev/sound/dummy.c | ||
---|---|---|
289 | If it attaches to the PCM driver, shouldn't the softc size be PCM_SOFTC_SIZE as well? In dummy_attach() (and all sound drivers' attach routines), we call pcm_register(), which expects the softc to be a struct snddev_info. |
sys/dev/sound/dummy.c | ||
---|---|---|
289 | Hmm, strange. You can always put a struct snddev_info at the beginning of your softc, and then pcm_register() will work. You're fetching the dummy softc as a driver ivar, but I don't see how that works - nothing is calling device_set_ivars(). Who is responsible for allocating the softc, and why is the softc pointer stored in ivars? ivars are usually used by bus drivers. |
Fix "random" panic caused by using PCM_SOFTC_SIZE instead of sizeof(struct dummy_softc): embed an snddev_info to dummy_softc and call
device_get_softc() instead of device_get_ivars() in dummy_attach().
- Improve IO.
- Implement CHANNEL_TRIGGER() and CHANNEL_GETPTR().
- Reorganize channels and caps better.
- Add locking.
share/man/man4/snd_dummy.4 | ||
---|---|---|
51 | I would omit the "physical" here, IMHO it confuses more than it helps. | |
54–57 | Stating that audio data is processed at regular intervals is fine, but the callout(9) is more of an implementation detail, and not relevant to the user / tester? | |
sys/dev/sound/dummy.c | ||
148–150 | Shouldn't this be for (i = 0; ch->caps->fmtlist[i]; i++) |