http://bugs.winehq.org/show_bug.cgi?id=28622
--- Comment #58 from Jörg Höhle hoehle@users.sourceforge.net 2012-10-18 00:53:09 CDT --- - if(snd_pcm_state(This->pcm_handle) == SND_PCM_STATE_XRUN || - avail > This->alsa_bufsize_frames){ + if(snd_pcm_state(This->pcm_handle) == SND_PCM_STATE_XRUN){ IMHO that's a good hint that the suggested This->quirks flag is needed.
We set snd_pcm_sw_params_set_stop_threshold(This->alsa_bufsize_frames) thus we expect XRUN to be signaled should avail grow beyond that. Yet I remember observing a large avail without XRUN or sound with some devices and that's why the extra check makes sense -- as a safety. Resetting is a means of rejuvenation.
With pulse, things are different, again. IIRC I observed huge avails, yet no audible overrun had happened and it could not have played all the data that I had sent by that time. So this check is somehow not appropriate for a broken pulse ALSA plugin when one wants to keep it playing. (OTOH one may argue that such huge avail is already an indication of broken state in pulse -- too late).
I think it would be good to split the buffer sizing from the rate limiting patch.
+ snd_pcm_hw_params_set_buffer_size_min( + snd_pcm_hw_params_set_periods_min( MAX_FED=4 ) Why hard limits? I've seen all kinds of ALSA periods from 2 to 500ms.
One of my TODO notes is about making sure that our driver accommodates nearly all ALSA periods and buffer sizes, i.e. that it decouples native's 10ms shared mode periods sufficiently from ALSA's behaviour.
set_*_near helps. Anything else presumably stands in the way.
Another TODO goal is to make our driver works with solely 2 ALSA periods, much like the traditional 2 buffer ping pong technique. This ought to work with ALSA periods > mmdevdrv 10ms periods.
Yet another TODO goal is to double check the limits in our driver by playing with mmdevdrv period sizes from 10ms to the 500ms that native allows (in exclusive mode). E.g. 4 ALSA periods will likely not be granted with 500ms periods -- but in that setting we don't need 4 periods. 2 shall be enough.
All these TODO require a lot of time...
BTW, the old code had one WARN(err, snd_strerror(err)) per failed call to snd_pcm_*. So we knew how ALSA reacted to each and every call.
Both PulseAudio and Jack seem to favour large 50-150ms periods over native 10ms. It should not be really surprising if snd_pcm_avail stalls within one period. Yet I don't think we handle that well. I had a sketch of a driver that decremented padding regularly in 10ms intervals regardless of ALSA periods but IIRC that was not one that would feed ALSA in ALSA-period-sized chunks. Given the number of min(..., to_write_frames) in your patch, it doesn't look like the code still tries to do that. Well, that may have been good with hw and dmix devices, but is dubious with large "virtual" 50-150ms periods from PA or Jack: better feed the 10ms snippets that XA2 gives us each round.
About the code, I think the while loop should contain one single alsa_write_best_effort and delegate repeating to the loop. I believe I found a flaw in the fed_frames updates that can make it overflow. And I suggest you invert the meaning of fed_frames, i.e. have fed_frames>N like avail>N or held_frames>N mean that N can be written.