[Bug 14717] resampled sound is horrible

wine-bugs at winehq.org wine-bugs at winehq.org
Wed Jan 4 01:38:28 CST 2012


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

--- Comment #263 from Alexander E. Patrakov <patrakov at gmail.com> 2012-01-04 01:38:28 CST ---
First, thanks for the fixes. It is indeed important to keep this patchset
bisectable.

As for the patch, I think I have found a theoretical problem. Your
X_speex_resampler_get_required_input_length() function can cause the resampler
to return less than count samples because it rounds down. So, for 11025 ->
48000 Hz conversion, if count happens to be 1 for some odd reason, it can
return 0 and nothing will be converted, leading to an infinite loop. So please
make absolutely sure that speex cannot request 0 samples. And for safety and
for simplification of the accounting, I'd even make it always return count
samples by supplying enough input, even if the secondary stream ends here (i.e.
pad it with zeros in this case instead of truncating total_length).

Since Speex converts until either the input or the output buffer space runs
out, for the reason above, I think it would be better to add a ceil() to the
X_speex_resampler_get_required_input_length() function. Or rewrite it using
fixed-point math to avoid the rounding issues, as shown below (completely
untested, and the standard coffee warning applies):

spx_uint32_t X_speex_resampler_get_required_input_length(SpeexResamplerState
*st, spx_uint32_t out_len)
{
    spx_uint32_t ret = out_len * st->num_rate;
    ret = (ret + st->den_rate - 1) / st->den_rate;
    return ret + st->filt_len;
}

Note that I have changed in_rate and out_rate with num_rate and den_rate,
because speex might choose to round the resampling factor to a more convenient
fraction.

OTOH, the addition of st->filt_len looks rather bogus to me (but harmless
anyway), because under normal conditions it only should apply to the first
call, and even then (read: if at all), maybe only if you haven't called
speex_resampler_skip_zeros() (and I am not quite sure if you should or should
not call it).

As for your question regarding any thoughts on Jörg's multichannel support
question: no, I don't have any thoughts about that that I am sure of and not
sent to wine-devel. And I didn't look at the old patches.

Regarding your statement about GTA:SA and Darwinia calling _SetFrequency() with
unusual values, I have an additional question: do they do that while the stream
is playing?

Overall, the patchset indeed looks nearly finished in terms of this bug. I will
give it some testing today and report the result.

BTW, there are definitely more (out-of-scope for this bug) cleanups that can be
made in dsound - e.g. mixing can be done in floating point instead of the
current mess with the mix and norm functions for every possible bitness of the
primary buffer.

-- 
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.


More information about the wine-bugs mailing list