http://bugs.winehq.org/show_bug.cgi?id=28723
--- Comment #83 from Jörg Höhle hoehle@users.sourceforge.net 2011-12-13 09:56:52 CST --- Here's my mini-review. 0003 alsa_period_us = 10000; If a constant, use DefaultPeriod. But that's wrong. It should be mmdev_period after due clamping of values. In exclusive mode, the app could ask for a period not 10ms.
0002/0004 -/+ TRACE("alsa_period_frames... The first patch should move it once and for all.
0004 Use MulDiv for both duration and period + This->mmdev_period_frames = (fmt->nSamplesPerSec * This->mmdev_period_rt) / 10000000.; + This->bufsize_frames = MulDiv(duration, rate, 10000000); Why rate here vs nSamplesPerPsec there? bufsize_frames=GetBufferSize should be computed with app-visible units, i.e. nSamplesPerSec, not ALSA's actual internal rate.
0002 In alsa_write_data I found the logic not clear enough. I suggest /* try to keep 3 ... */ max_period = 3 * max(); if(in_alsa >= max_period) return; and would get rid of the "while () write_limit+=;" loop which is bad with alsa_period=8.
0002 Continue using the same variable (who remembers to_write == written?): + if(This->held_frames && (to_write < write_limit)){ + min(This->held_frames, write_limit - written), This);
Overall, I don't like the complexity that 0002 adds to alsa_write_data. It would be preferable to use set_hw_params_buffer_size_max(3xboth_periods) because that's what the code in effect realizes! Often enough, set_period_near returns the period that ALSA chose, even before set_buffer_time|size or the final set_hw_params. (Another idea would be to iterate twice through set_hw_params. Such initialisation overhead is still preferable to overhead in the audio loop).