http://bugs.winehq.org/show_bug.cgi?id=28517
--- Comment #14 from Jörg Höhle hoehle@users.sourceforge.net 2011-10-05 03:04:22 CDT --- (In reply to comment #9)
I'd appreciate review on it.
Maybe it's an artefact of bugzilla's new two column diff mode, but where do you initialize mmdev_period_rt? I don't see it in http://bugs.winehq.org/attachment.cgi?id=36707&action=diff
Why do you reintroduce bufsize_frames etc. as UINT64?
I think it's good that the patch delegates write() entirely to the feeder thread. I once said that having it too inside ReleaseBuffer should be nothing more than a performance optimization. Yet this optimization may be critical to avoid underruns when event signaling is detached (not sync'ed) from ALSA's actual need, which always happens with 2 timers from independent clocks, but it was too early for me to analyse that.
I see no more sw_params_set_*. while it's ok to go with defaults as far as possible, there are a few sw_params that you want to set. For instance, ALSA must either enter XRUN mode and stop or play silence upon underrun. Not setting any sw_params does not guarantee that, in particular the circular HW buffers may loop ad infinitum.
Why do you care at all about ALSA's period? It can be as low as 1ms (e.g. with a 8000Hz sample I got 1ms period and 8.192ms buffer). I don't want to bear 1000 interrupts a second. Wine should ignore ALSA's period for as long as we don't use poll(alsa's_fd) and the buffer is large enough for our periodic feeder. See http://mailman.alsa-project.org/pipermail/alsa-devel/2011-August/042837.html
Alternatively and if it matches, Wine should equate its published period with ALSA's, e.g. try to use 10ms like mmdevapi's mixer does. The best in that case would be to sync' ALSA signaling readiness with mmdevapi's event-triggered feeder.