http://bugs.winehq.org/show_bug.cgi?id=14717
--- Comment #263 from Alexander E. Patrakov patrakov@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.