- Enable 4 automatic vchan's by default.
- Add some comments which provide ides/questions for improvement.
- Prefix some temporary sysctl's with an underscore to denote that it is not
an official API but a workaround until the real solution is implemented.
--------------------
- Seal the fate of long standing memory leak (4 years, 7 months) during
pcm_unregister(). While destroying cdevs, scan / detect possible
children and free its SLIST placeholder properly.
- Optimize channel allocation / numbering even further. Do brute cyclic
checking only if the channel numbering screwed.
- Mega vchan create/destroy cleanup:
o Implement pcm_setvchans() so everybody can use it freely instead
of implementing their own, be it through sysctl or channel auto
allocation.
o Increase vchan creation/destruction resiliency:
+ it's possible to increase/decrease total vchans even during
busy playback/recording. Busy channel will be left alone, untouched.
Abusive test sample:
# play whatever...
#
while : ; do
sysctl hw.snd.pcm0.vchans=1
sysctl hw.snd.pcm0.vchans=10
sysctl hw.snd.pcm0.vchans=100
sysctl hw.snd.pcm0.vchans=200
done
# Play something else, leave above loop running frantically.
+ Seal another 4 years old bug where it is possible to destroy (virtual)
channel even when its cdevs being referenced by other process.
The "First Come First Served" nature of dsp_clone() is the main
culprit of this issue, and usually manifest itself as dangling
channel <-> process association. Ensure that all of its cdevs
are free from being referenced before destroying it (through
ORPHAN_CDEVT() macross).
All these fixes (including previous fixes) will be MFCed, later.
to avoid possible device unregister race (impossible to reproduce, yet
possible).
- Extra sanity check to ensure proper parent channel is being selected.
- Reset parent channel once all of its children gone.
- Determine open direction using 'flags', not 'mode'. This bug exist since
past 4 years.
- Don't allow opening the same device twice, be it in a same or different
direction.
- O_RDWR is allowed, provided that it is done by a single open (for example
by mixer(8)) and the underlying hardware support true full-duplex operation.
- Do various paranoid checking in case other process/thread trying to hijack
the same device twice (or more).
MFC after: 5 days
especially for vchans. It turns out that channel numbering always depend
on d->devcount counter (which keep increasing), while PCMMKMINOR() truncate
everything to 8bit length. At some point the truncation cause the newly
created character device overlapped with the existence one, causing erratic
overall system behaviour and panic. Easily reproduce with something like:
(Luckily, only root can reproduce this)
while : ; do
sysctl hw.snd.pcm0.vchans=200
sysctl hw.snd.pcm0.vchans=100
done
- Enforce channel/chardev numbering within 8bit boundary. Return E2BIG
if necessary.
- Traverse d->channels SLIST and try to reclaim "free" counter during channel
creation. Don't rely on d->devcount at all.
- Destroy vchans in reverse order.
Anyway, this is not the fault of vchans. It is just that vchans are so cute
and begging to be abused ;) . Don't blame her.
Old, hidden bugs.. sigh..
MFC after: 3 days
dereferencing) since a NULL value would be a bug here.
Note: Both affected functions look very similar. A refactoring may
be beneficial.
CID: 483, 485
Found with: Coverity Prevent(tm)
Discussed with: ariff
MFC after: 5 days
This is supposed to fix some Coverity Prevent errors (Ariff didn't
looked at the CID's (ENOTIME), I just told him that there are some problems
in function dsp_ioctl()).
CID: 215-218
Found with: Coverity Prevent(tm)
Submitted by: ariff
MFC after: 5 days
(1) Fix DMA alignment, based on bytes per sample.
feeder_rate.c:
Handle strayed bytes (mostly caused by #1) better.
This DMA alignment issues are extremely hard to reproduce unless
the user happen to have a 32bit capable soundcards (ATI IXP) and
knowledgeable enough to force it to operate under pure 32bit
operations on both record and play directions.
feeder.h:
feeder.c:
- Implement scoring mechanisme to select best format for conversion.
This is actually part of newer format chaining procedures which
will be commited someday. Confusion during chaining process solved
by this scoring since it will try to reduce list of from/to formats
to a single, best format.
Related PR: kern/91683
channel.c:
- Simplify feeder building process since we have smarter format
chaining.
feeder_fmt.c:
- Add few more sign conversion feeders for 24 and 32 bit format.
feeder_rate.c:
- Force buffer / bytes allignment. Unaligned buffer may cause
panics during recording on pure 32bit sample format if it
involves feeder_rate as part of feeders chain.
Tested on: ATI IXP, force 32bit recording.
MFC after: 5 days
This should reduce huge playback / recording latency for
applications that try to act smarter and manage their own
buffering (XMMS, Skype, etc.).
Note to Skype + via8xxx users: Remove previous hackish
"hint.pcm.<unit>.via_dxs_disabled" from kernel hint and see
whether this changes cure all those annoying sound issues.
This one simply tries to simplify the logic to select the
buffer sizes. I am not sure it is necessary but the code
seems a bit more readable to me. And at least i have tried
to document how the buffer sizes are computed.
Thanks to luigi for deciphering one of the most cryptic part of
sound driver.
Submitted by: luigi
Approved by: netchild (mentor)
In SNDCTL_DSP_SETFRAGMENT, if you specify both read and
write channels, the existing code first acts on the
read channel, but as a side effect it updates the
arguments (maxfrags, fragsz) passed by the caller according
to acceptable values for the read channel, and then uses the
modified values to act on the write channel.
The problem with this approach is that, given a
(maxfrags, fragsz) user-specified value, the actual
values computed by the read and write channels may differ:
e.g. the read channel might want to allocate more fragments
than what the user specified because it has no side-effects
on the delay and it helps in case of slow readers,
whereas the write channel needs to use as few fragments
as possible to keep the audio latency low (very important
with telephony apps).
This patch stores the values computed by the read channel
into temproary variables so the write channel will use
the actual arguments of the ioctl.
This patch is very helpful with telephony apps such as asterisk.
Submitted by: luigi
Approved by: netchild (mentor)
- Added new codec id for CX20468-21 and VIA1617A.
Submitted by: Chen Lihong <lihong.chen@gmail.com>
- Re-enable SOUND_MIXER_IGAIN, but set the default level as 0 (mute)
Suggested by: luigi
mixer.c:
- Set default value for SOUND_MIXER_IGAIN as 0 (mute) to avoid
feedback problems on some laptops (was disabled by jhb during
ac97.c revision 1.42).
Approved by: netchild (mentor)
- Return EINVAL if play_format or rec_format is set but the corresponding
sample rate is 0.
- Don't try to set the playback or recording format to 0. Previously,
issuing an AIOSFMT ioctl with an all-zeroes snd_chan_param would
trigger a KASSERT in chn_fmtchain(); I'm unsure about the effects on
a kernel without INVARIANTS. After this commit, issuing AIOSFMT with
an all-zeroes snd_chan_param is equivalent to issuing AIOGFMT.
MFC after: 2 weeks
sampling rate:
- Improve vchan chn_setspeed() strategy. Try to avoid FEEDER_RATE
on parent channel if the requested value is not supported
by the hardware.
- Fix vchan default speed calculation. In any case, vchan should
rely on parent bufsoft speed instead of bufhard since it is
possible that the entire feeder chain might involve FEEDER_RATE.
This is possible under extreme, rare condition if the above
chn_setspeed() strategy failed.
Approved by: netchild (mentor)
* General spl* cleanup. It doesn't serve any purpose anymore.
* Nuke sndstat_busy(). Addition of sndstat_acquire() /
sndstat_release() for sndstat exclusive access. [1]
sys/dev/sound/pcm/sound.c:
* Remove duplicate SLIST_INIT()
* Use sndstat_acquire() / release() to lock / release the entire
sndstat during pcm_unregister(). This should fix LOR #159 [1]
sys/dev/sound/pcm/sound.h:
* Definition of SD_F_SOFTVOL (part of feeder volume)
* Nuke sndstat_busy(). Addition of sndstat_acquire() /
sndstat_release() for exclusive sndstat access. [1]
Submitted by: Ariff Abdullah <skywizard@MyBSD.org.my>
LOR: 159 [1]
Discussed with: yongari [1]
* Added codec id for CMI9761.
* feeder_volume *whitelist* through ac97_fix_volume()
sys/dev/sound/pcm/ac97.h:
* Added AC97_F_SOFTVOL definition.
sys/dev/sound/pcm/channel.c:
* Slight changes for chn_setvolume() to conform with OSS.
* FEEDER_VOLUME is now part of feeder building process.
sys/dev/sound/pcm/mixer.c:
* General spl* cleanup. It doesn't serve any purpose anymore.
* Main hook for feeder_volume.
Submitted by: Ariff Abdullah <skywizard@MyBSD.org.my>
Tested by: multimedia@
While I'm here add KASSERT(9) to notify failure of SYSUNINIT handler.
Reported by: Ben Kaduk < minimarmot AT gmail DOT com >
Tested by: Ben Kaduk < minimarmot AT gmail DOT com >
- Remove an assertion in sound.c, it's not needed (and causes a panic now).
From the conversation via mail between glebius and Ariff:
---snip---
> Well, but which mutex protects now? Do we own anything else
> in pcm_chnalloc()? I see some queue(4) macros in pcm_chnalloc(),
> they should be protected, shouldn't they?
Queue insertion/removal occur during
1) driver loading (which is pretty much single thread /
sequential) or unloading (mutex protected, bail out if there is
any channel with refcount > 0 or busy).
2) vchan_create()/destroy(), (which is *sigh* quite complicated), but
somehow protected by 'master'/parent channel mutex. Other
thread cannot add/remove vchan (or even continue traversing
that queue) unless it can acquire parent channel mutex.
---snip---
Fix the locking in dsp.c to prevent a LOR (AFAIK not on the LOR page).
Submitted by: Ariff Abdullah <skywizard@MyBSD.org.my>
Tested with: INVARIANTS[1] and DIAGNOSTICS[2]
Tested by: netchild [1,2], David Reid <david@jetnet.co.uk> [1]
* New definition CHN_F_HAS_VCHAN.
- channel.c
* Use CHN_F_HAS_VCHAN to mark channel with vchan capability instead
of relying on SLIST_EMPTY(&channel->children) == true for better
clarification and future possible usages of children (like
'slave' channel).
* Various fixes, including blocksize / format bps allignment,
better 24bit seeking (mplayer, others).
* Improve format chain building, it's now possible to record something
to a format non-native to the soundcard through various feeder format
converters or to higher sampling rate. This also gains another feature,
like doing vchan mixing on non s16le soundcard such as sb8.
- sound.c
* Increase robustness within various function that handle vchan
creation / termination (these function need a total rewrite, but
that would cause other major rewrite within various places too!).
As far as its robustness can be guaranteed, leave it as is.
* Optimize channel ordering, prefer *real* hardware playback
channels over virtual channels. cat /dev/sndstat should look
better.
* Increase sndstat verbosity to include bufsoft/bufhard allocation.
- vchan.c
* Fix LOR 119.
- http://sources.zabbadoz.net/freebsd/lor.html#119
* Reorder / increase robustness of vchan_create() / destroy().
Enforce destroy_dev() during destroy operation, fix possible
panic / dangling character device.
- http://lists.freebsd.org/pipermail/freebsd-current/2005-May/050308.html
* Tolerate a little bit more during mixing process, this should help
non s16le soundcards.
Note: Recoring in a non-native rate/format may result in overruns. A friendly
application is wavrec from audio/wavplay. The problem is under
investigation.
Submitted by: Ariff Abdullah <skywizard@MyBSD.org.my>
event handler, dev_clone, which accepts a credential argument.
Implementors of the event can ignore it if they're not interested,
and most do. This avoids having multiple event handler types and
fall-back/precedence logic in devfs.
This changes the kernel API for /dev cloning, and may affect third
party packages containg cloning kernel modules.
Requested by: phk
MFC after: 3 days
