From: Joerg-Cyril.Hoehle@t-systems.com Subject: winealsa: Implement IAudioClock::GetPosition() using snd_pcm_delay. Message-Id: <8E4C156DA5797D418DBFADFD8CE655A4153337F7EF@HE113481.emea1.cds.t-internal.com> Date: Fri, 9 Dec 2011 17:12:54 +0100 Hi, Here's a version of GetPosition that exhibits the following properties: - monotonically increasing; - protection against garbage delay data; - never using ALSA padding, because that can't be relied upon in PulseAudio after an underrun; - works perfectly with dmix, reasonably well with old Ubuntu Intrepid Pulseaudio -- don't dump old Linux distributions! - after IAC_Stop, freeze to sum_written - mmdevapi_padding. - Via This->last_pos, provide a means for communication between GetPosition and other parts of the code. What I mean is that future fixes to underrun handling in the event callback could bump last_pos to maximum, causing subsequent GetPosition to follow. Please refer to bug #28273. Andrew found a patch like this "passes all tests" (presumably on top of git, not yet unsubmitted padding patches). I found it to work with: - Ubuntu Intrepid plughw:0, plug:dmix, and more or less Pulse - Ubuntu Lucid plughw:0, plug:dmix, whereas Pulse completely breaks with Wine because of missing underrun handling, independently on this GetPosition patch. In bug #28273, comment #74, I mentioned bumping position to maximum when delay becomes smaller than a period. I've not added that now. This could be part of a future patch about better underrun handling. I've seen valid delays < alsa_period_size where ALSA entered XRUN state with delay < 0 as well as cases where it froze with delay > 0. Comparison with an enum value in alsa_state > SND_PCM_STATE_RUNNING looks strange, but I don't want to enumerate all XRUN, SUSPEND, DRAINING, PREPARED and other states. (PREPARED after This->started, but before the first event writes data, causing ALSA to start). A note about old (Intrepid) Pulseaudio: my enhanced render.c tests mostly work, but produce failures because PulseAudio eats data too fast into its huge 2s buffer. Old PulseAudio produces much better results with Andrew's unsubmitted "change drain behaviour" patch. Valgrind should flag an uninitialized read when tracing: delay may not be written to when snd_pcm_delay() returns < 0. That doesn't cause an uninitialized read when computing the position, but I TRACE the raw values. Regards, Jörg Höhle From 03b3c3f2ccfbdafe12d2d8a72312909c557734df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20H=C3=B6hle?= Date: Sun, 4 Dec 2011 10:20:29 +0200 Subject: winealsa: Implement IAudioClock::GetPosition() using snd_pcm_delay. --- dlls/winealsa.drv/mmdevdrv.c | 50 ++++++++++++++++++++++++++++++++--------- 1 files changed, 39 insertions(+), 11 deletions(-) diff --git a/dlls/winealsa.drv/mmdevdrv.c b/dlls/winealsa.drv/mmdevdrv.c index bfff159..359997e 100644 --- a/dlls/winealsa.drv/mmdevdrv.c +++ b/dlls/winealsa.drv/mmdevdrv.c @@ -109,7 +109,7 @@ struct ACImpl { BOOL initted, started; REFERENCE_TIME mmdev_period_rt; - UINT64 written_frames; + UINT64 written_frames, last_pos_frames; UINT32 bufsize_frames, held_frames, tmp_buffer_frames; UINT32 lcl_offs_frames; /* offs into local_buffer where valid data starts */ @@ -1790,6 +1790,7 @@ static HRESULT WINAPI AudioClient_Reset(IAudioClient *iface) if(snd_pcm_prepare(This->pcm_handle) < 0) WARN("snd_pcm_prepare failed\n"); + This->last_pos_frames = 0; This->held_frames = 0; This->written_frames = 0; This->lcl_offs_frames = 0; @@ -2291,8 +2292,12 @@ static HRESULT WINAPI AudioClock_GetPosition(IAudioClock *iface, UINT64 *pos, UINT64 *qpctime) { ACImpl *This = impl_from_IAudioClock(iface); - UINT32 pad; - HRESULT hr; + UINT64 written_frames, position; + UINT32 held_frames; + int err; + snd_pcm_state_t alsa_state; + snd_pcm_uframes_t avail_frames; + snd_pcm_sframes_t delay_frames; TRACE("(%p)->(%p, %p)\n", This, pos, qpctime); @@ -2301,19 +2306,42 @@ static HRESULT WINAPI AudioClock_GetPosition(IAudioClock *iface, UINT64 *pos, EnterCriticalSection(&This->lock); - hr = IAudioClient_GetCurrentPadding(&This->IAudioClient_iface, &pad); - if(FAILED(hr)){ - LeaveCriticalSection(&This->lock); - return hr; + /* call required to get accurate snd_pcm_state() */ + avail_frames = snd_pcm_avail_update(This->pcm_handle); + alsa_state = snd_pcm_state(This->pcm_handle); + written_frames = This->written_frames; + held_frames = This->held_frames; + + err = snd_pcm_delay(This->pcm_handle, &delay_frames); + if(err < 0){ + /* old Pulse, shortly after start */ + WARN("snd_pcm_delay failed in state %u: %d (%s)\n", alsa_state, err, snd_strerror(err)); } - if(This->dataflow == eRender) - *pos = This->written_frames - pad; - else if(This->dataflow == eCapture) - *pos = This->written_frames + pad; + if(This->dataflow == eRender){ + position = written_frames - held_frames; /* maximum */ + if(!This->started || alsa_state > SND_PCM_STATE_RUNNING) + ; /* mmdevapi stopped or ALSA underrun: pretend everything was played */ + else if(err<0 || delay_frames > position - This->last_pos_frames) + /* Pulse bug: past underrun, despite recovery, avail_frames & delay + * may be larger than alsa_bufsize_frames, as if cumulating frames. */ + /* Pulse bug: EIO(-5) shortly after starting: nothing played */ + position = This->last_pos_frames; + else if(delay_frames > 0) + position -= delay_frames; + }else + position = written_frames + held_frames; + + /* ensure monotic growth */ + This->last_pos_frames = position; LeaveCriticalSection(&This->lock); + TRACE("frames written: %u, held: %u, avail: %ld, delay: %ld state %d, pos: %u\n", + (UINT32)(written_frames%1000000000), held_frames, + avail_frames, delay_frames, alsa_state, (UINT32)(position%1000000000)); + *pos = position; + if(qpctime){ LARGE_INTEGER stamp, freq; QueryPerformanceCounter(&stamp); -- 1.7.0.4