[Bug 14717] resampled sound is horrible

[wine-bugs at winehq.org]

http://bugs.winehq.org/show_bug.cgi?id=14717

--- Comment #237 from Alexander E. Patrakov <patrakov at 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.

-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.