http://bugs.winehq.org/show_bug.cgi?id=14717
--- Comment #237 from Alexander E. Patrakov patrakov@gmail.com 2011-11-11 10:25:00 CST --- (In reply to comment #236)
(In reply to comment #225)
If most of the improvements actually come from moving the old bad resampler to the mixer thread, then I suggest that Wine developers merge the first patch in my series, and then we can discuss the rest of the patches separately, so that the two types of issues are not mixed together.
Well, your resampler improvements depend on moving resampling back to the mixer thread, and if that causes regressions, then your improvements are blocked. So we're trying to fix these regressions so your fixes can get in.
I understand this, and agree that if regressions are found due to the move, then it is a valid reason to block the rest of my patches. What I am trying to do is to make sure that regressions due to the move and due to other refactorings are clearly separated in user reports, e.g. by submitting the first patch separately, and maybe even targetting the rest of the patchset for a different Wine version.
And one more point: I am certain that the first patch (unlike others) already has its final form, and I see nothing to discuss about it except motivation, regressions uncovered by it and possibly dead code that I might have missed. In other words, there is no point to delay its submission (unlike the rest of the patchset, where I am not sure).
(In reply to comment #227)
P.S. Sorry for hi-jacking the bug report a bit more but I have a semi-related question for Andrew to ask: looks like that the default samplerate for dsound is set to be 44100Hz, which differs from the typical 48000Hz value that's being used by most AC97 codecs.
Maybe it would be wise to change the default to be 48KHz?
I don't have a problem with that. If you can provide some evidence that most cards use that rate (or some multiple), it will help getting through the wine-patches submission process.
As I have already said, this is irrelevant because most of the distros are defaulting to PulseAudio now, and pulseaudio runs at 44.1 kHz even if the codecs use some other frequency internally.
(In reply to comment #228)
CPU usage is quite high (over 20%) for vlc especially when user mis-configure default sample rate to 96000Hz when the sound card only support 48000Hz
when normal usage is only 1% for vlc for playing 16bit 44100Hz
I think I will add options for different resampler qualities (see comment 230) so users can tweak this down when they need to.
Thanks! And I will evaluate whether going to floating point improves anything (for speex, it reportedly does).
(In reply to comment #231)
What can be done, though, is to disable the ALSA resampler (only if called from DirectSound! that's important!), ask ALSA for approximately the default sample rate, and use the result. Something like this pseudocode (copied from MPlayer that gets the logic right, but untested in my own code):
I wonder if a better solution is to have dsound use the sample rate reported by GetMixFormat() as its default primary buffer rate. Then the drivers could be improved to report the device's preferred sample rate, if they don't already.
If the waveOut API does not call GetMixFormat(), I'd say that our solutions are equivalent: e.g., one can insert my lines into GetMixFormat() and thus get what you described. There would certainly be no problem for the case when such a preferred sample rate exists (i.e., for dmix). But in the case when the single preferred sample rate does not exist (e.g. on any sound card that can do either 44100 or 48000 Hz in hardware and is accessed via hw:X) or when this rate is not reported via ALSA layer (e.g. with pulseaudio), we need some fallback mechanism, possibly inside GetMixFormat().
The current code in both OSS and ALSA mmdevdrvs contains some variant of the heuristic that the highest supported sample rate is the preferred one. On my hardware, this would cause the ALSA mmdevdrv to choose 48 kHz because of the explicit cap at that frequency (or 192 kHz if the cap were not there). However, this is not the correct result for PulseAudio, and there is no way to get the correct-for-pulseaudio result via ALSA API.
So we do need an explicit preference in the registry - i.e. we can only move it from dsound to drivers (which is IMHO pointless), but not remove it. But if you try to implement the use of that preference via ALSA API, you will probably come to something resembling my pseudo-code.