The sad state of audio GetPosition
Hi, Maarten Lankhorst wrote: >But it's not the same, wine doesn't officially handle exclusive mode. Eh? This is new to me! Back in June Andrew Eikum wrote: >These limitations might be true for exclusive mode, >which isn't really implemented in Wine. ... which is not the same in my ears. Andrew back in June: >I think it would be worth having a loud FIXME >when exclusive mode is requested, though. Maarten today: >Instead it might be better to just report failure and only allow shared mode. I prefer a FIXME in exclusive non-event mode and the error in exclusive event mode because that particular GetBuffer protocol is not supported in the current code. FIXME because exclusive non-event mode basically works except that we don't know yet when exactly native delivers events. As nobody else wrote that patch so far, I'll do. >On windows you're not required to support exclusive mode, >and you can disable exclusive mode in the audio control panel. I've read that games offer it as an option but believe that all apps will use the shared mode by default for the reason you say. Mixing has become a commodity. > http://msdn.microsoft.com/en-us/library/dd370844%28v=vs.85%29.aspx >I meant an entire period then, this describes how windows handles it What part of that document do you have in mind? Their example uses exclusive+EVENTCALLBACK, which my test doesn't. >my guess is that if you don't fill a period, >you get an underrun, no matter how you handle it. Sure. Yet I want to know how it affects GetPosition, e.g. bug #28039 IMHO I'm free to use GetBuffer+ReleaseBuffer twice or 3x within whatever MSDN calls a "buffer-processing period" if it pleases me. What matters is how much has accumulated once the period tick occurs. If period sizes would really matter, then the API would provide means to obtain the period size in frames. It does not. Initialize takes a period in milliseconds and MSDN does not guarantee what you actually get. So the average app simply does not know the period size in frames. (My tests do try to second-guess mmdevapi's exact behaviour). Actually, mmdevapi tries to abstract away from these parameters. BTW, a bug in MSDN: render.c:799: Test failed: BufferSize 21846 too small for duration at rate 44100 mmdevapi appears to use not ceiling(), but floor(duration, period_frames). The actual duration is *not* at least as large as asked for. (For the period, it uses ceiling). Regards, Jörg Höhle
Re: The sad state of audio GetPosition
On 08/22/2011 05:31 PM, joerg-cyril.hoe...@t-systems.com wrote: > Hi, > > Maarten Lankhorst wrote: >> exclusive mode wants you to fill the entire buffer at the same time. > You're confusing this with exclusive + EVENTCALLBACK mode, which I'm not > using. > There are 4 combinations of shared/exclusive and EVENTCALLBACK or not. > Only one of the 4 needs no GetCurrentPadding and uses a full buffer each time. > > So far I've only been testing without EVENTCALLBACK > (though test_event contains some non-rendering code using it). http://msdn.microsoft.com/en-us/library/dd370844%28v=vs.85%29.aspx I meant an entire period then, this describes how windows handles it, my guess is that if you don't fill a period, you get an underrun, no matter how you handle it. >> it wouldn't surprise me if that's why it drops things.. > I'd expect GetBuffer to return AUDCLNT_E_BUFFER_SIZE_ERROR as documented for > that case. Ah true. Event driven is slightly different. :) > > Last week I wrote: >> Somewhat I found the old behavior more consistent. > Upon reflection, the new behavior is better. It makes more sense to drop > old frames than to keep them around thus play ghost sounds from the past. > > However, I'd even more prefer shared and exclusive mode to behave the same > *and* not drop frames. > I believe shared mode does not drop partial fills (based solely on what > GetPosition returns) and would have expected exclusive mode to do that > too. I don't know why MS decided to do that but I now see > why they recommend that programs add silence at the end of their > sound: it works around that odd behaviour. But it's not the same, wine doesn't officially handle exclusive mode. Instead it might be better to just report failure and only allow shared mode. On windows you're not required to support exclusive mode, and you can disable exclusive mode in the audio control panel. > > Maarten also wrote: >> I prefer 0 tracking, and just check the return value of snd_pcm_writei. > I can understand that but then we need to throw rate-limiting into the > discussion. We are talking about GetPosition here; computing > GetPosition solely on the base of snd_pcm_writei (let's throw in > avail_update without delay) gives us already known bugs, > e.g. PulseAudio backend audio 2s off video sync. Ah true, I do think you only want to observe underruns, and not act on them in GetPosition and GetClock, just report that all data is played. If that still breaks pulseaudio, I think it's time to get a real pulseaudio driver.. > And without snd_pcm_delay, the sole culprit would be: Wine. > If, OTOH, we use it, we can hope other projects will fix their bugs. > snd_pcm_delay would map to clock, while snd_pcm_avail maps to position, they're not the same, so it would make sense to handle them separately. ~Maarten
The sad state of audio GetPosition
Hi, Maarten Lankhorst wrote: > exclusive mode wants you to fill the entire buffer at the same time. You're confusing this with exclusive + EVENTCALLBACK mode, which I'm not using. There are 4 combinations of shared/exclusive and EVENTCALLBACK or not. Only one of the 4 needs no GetCurrentPadding and uses a full buffer each time. So far I've only been testing without EVENTCALLBACK (though test_event contains some non-rendering code using it). >it wouldn't surprise me if that's why it drops things.. I'd expect GetBuffer to return AUDCLNT_E_BUFFER_SIZE_ERROR as documented for that case. Last week I wrote: >Somewhat I found the old behavior more consistent. Upon reflection, the new behavior is better. It makes more sense to drop old frames than to keep them around thus play ghost sounds from the past. However, I'd even more prefer shared and exclusive mode to behave the same *and* not drop frames. I believe shared mode does not drop partial fills (based solely on what GetPosition returns) and would have expected exclusive mode to do that too. I don't know why MS decided to do that but I now see why they recommend that programs add silence at the end of their sound: it works around that odd behaviour. Maarten also wrote: >I prefer 0 tracking, and just check the return value of snd_pcm_writei. I can understand that but then we need to throw rate-limiting into the discussion. We are talking about GetPosition here; computing GetPosition solely on the base of snd_pcm_writei (let's throw in avail_update without delay) gives us already known bugs, e.g. PulseAudio backend audio 2s off video sync. And without snd_pcm_delay, the sole culprit would be: Wine. If, OTOH, we use it, we can hope other projects will fix their bugs. Regards, Jörg Höhle
Re: The sad state of audio GetPosition
On 08/22/2011 02:13 PM, joerg-cyril.hoe...@t-systems.com wrote:
> Hi,
>
> These exclusive mode test failures look like nothing:
> render.c:1086: Test failed: GetCurrentPadding returned 576, should be 0
> render.c:1092: Test failed: Position 90720 at end vs. 91296 played frames
> but they made me completely rework my model of how mmdevapi works.
>
> 91296 = 90720 + 576 remainder
> 90720 = 720 * 126
> 720 = 15ms period at 48000 frames/s
>
> It looks like only a multiple of period_size frames are rendered.
> There are also test runs which end with GCP > period_size despite underrun
>
> Only Vista and w2k8 present these 2 errors. w2k8R2 and w7 only
> present the second one. In shared mode, GetCurrentPadding (GCP)
> returns 0 and GetPosition returns all submitted ("played") frames.
> What happened?
>
> Possible explanations:
> Vista + w2k8 exclusive mode:
> a) Bug in native that forgets to account for sub-period_size data. Or
> b) Play only multiples of period size. Wait until period_size data
>accumulates before sending the next packet.
Have to go with this one, exclusive mode wants you to fill the entire buffer at
the same time..
> c) other...
>
> w2k8R2 + w7 exclusive mode:
> a) Bug in native that sets GCP to 0 (like in shared mode). Or
> b) At each period tick, only play data if a full period is available.
>Drop partially filled buffers (explains padding = 0) instead of
>playing them much later when the buffer fills up (no ghost sounds).
> c) Bug in native that forgets to increment Position yet plays data.
> d) other...
If I look at msdn, exclusive mode wants you to fill the entire buffer in 1 go,
doing partial data doesn't make sense, and it wouldn't surprise me if that's
why it drops things..
> shared mode:
> a) At each 10ms period tick, mix a period_size of all streams that
>provided a full period of data. Ignore other streams yet always
>decrement GetCurrentPadding and sum played/ignored frames.
> b) At each 10ms period tick, mix all streams. Handle streams with
>padding < period_size as follows:
>- if previous frame was full, mix at beginning of buffer (trailer)
>- otherwise mix at end (expect beginning of next sound)
> c) At each 10ms period tick, mix all streams, padding streams with
>less than period_size frames with silence (i.e. mix at beginning).
> d) other...
> If I were MS, I'd implement heuristic b).
>
> This is all unlike what Wine does...
I'm inclined to believe it's C
> I'll probably write audible audio tests in the next few weeks to be
> able to test what native does (e.g. emit a short beep < period size
> every few ms and listen to what happens).
>
> Regards,
> Jörg Höhle
>
>
The sad state of audio GetPosition
Hi,
These exclusive mode test failures look like nothing:
render.c:1086: Test failed: GetCurrentPadding returned 576, should be 0
render.c:1092: Test failed: Position 90720 at end vs. 91296 played frames
but they made me completely rework my model of how mmdevapi works.
91296 = 90720 + 576 remainder
90720 = 720 * 126
720 = 15ms period at 48000 frames/s
It looks like only a multiple of period_size frames are rendered.
There are also test runs which end with GCP > period_size despite underrun
Only Vista and w2k8 present these 2 errors. w2k8R2 and w7 only
present the second one. In shared mode, GetCurrentPadding (GCP)
returns 0 and GetPosition returns all submitted ("played") frames.
What happened?
Possible explanations:
Vista + w2k8 exclusive mode:
a) Bug in native that forgets to account for sub-period_size data. Or
b) Play only multiples of period size. Wait until period_size data
accumulates before sending the next packet.
c) other...
w2k8R2 + w7 exclusive mode:
a) Bug in native that sets GCP to 0 (like in shared mode). Or
b) At each period tick, only play data if a full period is available.
Drop partially filled buffers (explains padding = 0) instead of
playing them much later when the buffer fills up (no ghost sounds).
c) Bug in native that forgets to increment Position yet plays data.
d) other...
shared mode:
a) At each 10ms period tick, mix a period_size of all streams that
provided a full period of data. Ignore other streams yet always
decrement GetCurrentPadding and sum played/ignored frames.
b) At each 10ms period tick, mix all streams. Handle streams with
padding < period_size as follows:
- if previous frame was full, mix at beginning of buffer (trailer)
- otherwise mix at end (expect beginning of next sound)
c) At each 10ms period tick, mix all streams, padding streams with
less than period_size frames with silence (i.e. mix at beginning).
d) other...
If I were MS, I'd implement heuristic b).
This is all unlike what Wine does...
I'll probably write audible audio tests in the next few weeks to be
able to test what native does (e.g. emit a short beep < period size
every few ms and listen to what happens).
Regards,
Jörg Höhle
Re: The sad state of audio GetPosition
On 08/19/2011 05:10 PM, joerg-cyril.hoe...@t-systems.com wrote:
> Maarten Lankhorst wrote:
> [nice to hear from you]
>
>>> IMHO AudioClient_Stop must not map to
>>> snd_pcm_drop. It is more like snd_pcm_pause. Or perhaps simply lead
>>> ALSA into an underrun.
>> afaict pause, with reset mapped to drop,
> Indeed. But I believe I need a fallback because ALSA says that
> pause "works only on the hardware which supports" it.
>
>
>> I can't remember why pause didn't work, but if it works go for it.
> I was solely thinking aloud that pause is TRT, not tried out yet.
>
> However, I received test results from a "Windows 7 Ultimate" machine.
> It exhibits a similar bug -- in exclusive mode only:
> render.c:948: Test failed: Position 18191 too far after 100ms
> Shared mode works as my tests expect it (<= 48000/10 frames).
>
>
>>> +snd_pcm_status_alloca(&status);
>> HeapAlloc(GetProcessHeap(), HEAP_ZERO_FLAG, snd_pcm_status_sizeof())
>> or something like that if available please..
> Really? I don't want to go through the overhead of memory allocation
> when all I need is a stupid small amount of stack allocated memory.
>
>
>>> +if(!This->initted){
>>> +return AUDCLNT_E_NOT_INITIALIZED;
>> Unneeded part.
> Can't you obtain a handle to that COM object prior to
> calling Initialize which sets This->fmt?
No.
>> Follow that flow..
> I beg your pardon?
IAudioClock is not available until Initialize has succeeded, so the check above
is pointless.
>
>>> +if(0){
>>> +avail_frames = snd_pcm_status_get_avail(status);
>>> +delay_frames = snd_pcm_status_get_delay(status);
>> if 0 is bad...
> I tried out pcm_status because somebody in alsa-devel mentioned that
> it allows to grab avail + delay in one (sync'ed?).
> However, I found delay to be always 0 inside status!?!
>
> Also, I found out that I need to call avail_update and delay
> in a particular order, otherwise I get stale values from an old call
> prior to the last sleep...
>
>
>>> +if(avail_frames <= This->bufsize_alsa + MAX_LATE_SECONDS *
>>> This->fmt->nSamplesPerSec
>>> + && delay_frames > 0)
>> Isn't delay_frames < 0 the definition of underrun?
> Indeed.
> There are potentially N distinct underruns:
> - the front end -- what snd_pcm_avail_update knows about;
> - intermittent buffers (USB);
> - the speaker -- what snd_pcm_delay knows about.
> There could be a short front-end buffer underrun that
> goes unnoticed by the speaker if the TCP or USB in
> between buffers enough data *and* is able to speed up.
>
>> no point in adding MAX_LATE_SECONDS
> That is some form of guard against broken values. E.g. people reporting
> in alsa-devel that PA sometimes complains about avail ~ MAXINT and such weird
> values.
>
>> Getting an avail update again? Why?
> The theory is:
> position = written_frames(into ALSA) - delay
> and translates to:
> This->written_frames - This->held_frames - delay
>
> However sometimes I can't trust delay.
> I still need to figure out when.
> - IIRC after an underrun, snd_pcm_delay yields error X.
> - or was it before starting?
> - ...
>
> The upper bound on position is always:
> This->written_frames(ReleaseBuffer) - This->held_frames - ALSA_padding
> (what ALSA's front end has not yet processed, in absence of underrun).
>
> Perhaps that would be robust:
> 1. Compute upper bound
> 2. position = clamp(0, delay > 0 ? written-delay : written, upper_bound);
> 2b. except when not yet started ...
> 2c. except while stopped ...
>
> I was even considering:
> 3. if position < This->previous_position stick to previous...
>
> Yet perhaps it's better to allow intermittent garbage values
> than to stick to garbage!
>
> OTOH, the delay values I see in the logs are subject to such variation (with
> PA)
> that I'm considering going with a clock instead, or perhaps:
> - query delay once per tick (e.g. 10ms) => last_pos
> - when asked, compute position from last_pos + time since tick * rate
>
> The last_pos slot may be needed anyway once stopped in pause mode.
>
Don't worry about underruns and checking the way you do is fragile, I prefer 0
tracking, and just check the return value of snd_pcm_writei. Experience taught
me this is the most stable way of doing underrun handling in alsa. Remove all
the checks you're doing please, they will just break things more.
The sad state of audio GetPosition
Hi, here's just for fun an interpretation of some test data: A native machine passes all my tests in shared mode. In exclusive mode, that machine behaves like PulseAudio in Wine now :-) render.c:797: BufferSize 21846 frames render.c:799: Test failed: BufferSize 21846 too small for duration at rate 44100 render.c:807: Clock Frequency 44100 render.c:872: Test failed: Position 21829 too far after 100ms render.c:882: padding 0 past sleep #2 It ate 495ms worth of samples (the completely prefilled buffer - mysterious 17) within 100ms after Start then returned a local underrun (padding 0)! Clearly a bug in MS-Windows or some driver. If you start with an underrun like my loop does, then that machine returns data that appears valid: render.c:1033: padding 1324 position 19648 slept 470ms iteration 0 Testbot shows how MS "fixed" a bug between Vista and w2k8R2/w7 in exclusive mode: render.c:1086: Test failed: GetCurrentPadding returned 576, should be 0 render.c:1092: Test failed: Position 90720 at end vs. 91296 played frames Somewhat I found the old behavior more consistent. After all, 90720 + 576 = 91296 What's noteworthy is that only a multiple of period frames appears to get written. Unlike shared mode, the remainder will be left in the buffer, even if you wait for an underrun to occur (like my tests do). IOW, native players using exclusive mode are well-advised to add a few silence frames at the end of the samples, or they won't be heard. At least, that's what I infer from render.c:1092: Test failed: Position 90720 at end vs. 91296 played frames since I've never put my hands on a native system new enough to have mmdevapi. Testbot confirms again that you must not trust timers in vmware, which I already experienced with the MCI tests. render.c:1040: hpctime 828 pcpos 843 render.c:1033: padding 16560 position 50832 slept 870ms iteration 4 render.c:1034: Test failed: Position 50832 too far after 870ms render.c:1035: Test failed: Position delta 7392 not regular render.c:1040: hpctime 937 pcpos 952 Within 109ms its speaker position advanced by 7392 frames, whereas 5232 would be normal at 48000 in that time frame. Other than that, data looks good. It will be no fun to correct the tests to accept testbot's daily results on test.winehq. Currently I lean towards if (winetest_debug > 0) position-tests such that testbot's patch watcher skips over position-based tests which we would know pass on native. Regards, Jörg Höhle
The sad state of audio GetPosition
Maarten Lankhorst wrote:
[nice to hear from you]
>> IMHO AudioClient_Stop must not map to
>> snd_pcm_drop. It is more like snd_pcm_pause. Or perhaps simply lead
>> ALSA into an underrun.
>afaict pause, with reset mapped to drop,
Indeed. But I believe I need a fallback because ALSA says that
pause "works only on the hardware which supports" it.
>I can't remember why pause didn't work, but if it works go for it.
I was solely thinking aloud that pause is TRT, not tried out yet.
However, I received test results from a "Windows 7 Ultimate" machine.
It exhibits a similar bug -- in exclusive mode only:
render.c:948: Test failed: Position 18191 too far after 100ms
Shared mode works as my tests expect it (<= 48000/10 frames).
>> +snd_pcm_status_alloca(&status);
>HeapAlloc(GetProcessHeap(), HEAP_ZERO_FLAG, snd_pcm_status_sizeof())
> or something like that if available please..
Really? I don't want to go through the overhead of memory allocation
when all I need is a stupid small amount of stack allocated memory.
>> +if(!This->initted){
>> +return AUDCLNT_E_NOT_INITIALIZED;
>Unneeded part.
Can't you obtain a handle to that COM object prior to
calling Initialize which sets This->fmt?
>Follow that flow..
I beg your pardon?
>> +if(0){
>> +avail_frames = snd_pcm_status_get_avail(status);
>> +delay_frames = snd_pcm_status_get_delay(status);
>if 0 is bad...
I tried out pcm_status because somebody in alsa-devel mentioned that
it allows to grab avail + delay in one (sync'ed?).
However, I found delay to be always 0 inside status!?!
Also, I found out that I need to call avail_update and delay
in a particular order, otherwise I get stale values from an old call
prior to the last sleep...
>> +if(avail_frames <= This->bufsize_alsa + MAX_LATE_SECONDS *
>> This->fmt->nSamplesPerSec
>> + && delay_frames > 0)
>Isn't delay_frames < 0 the definition of underrun?
Indeed.
There are potentially N distinct underruns:
- the front end -- what snd_pcm_avail_update knows about;
- intermittent buffers (USB);
- the speaker -- what snd_pcm_delay knows about.
There could be a short front-end buffer underrun that
goes unnoticed by the speaker if the TCP or USB in
between buffers enough data *and* is able to speed up.
>no point in adding MAX_LATE_SECONDS
That is some form of guard against broken values. E.g. people reporting
in alsa-devel that PA sometimes complains about avail ~ MAXINT and such weird
values.
>Getting an avail update again? Why?
The theory is:
position = written_frames(into ALSA) - delay
and translates to:
This->written_frames - This->held_frames - delay
However sometimes I can't trust delay.
I still need to figure out when.
- IIRC after an underrun, snd_pcm_delay yields error X.
- or was it before starting?
- ...
The upper bound on position is always:
This->written_frames(ReleaseBuffer) - This->held_frames - ALSA_padding
(what ALSA's front end has not yet processed, in absence of underrun).
Perhaps that would be robust:
1. Compute upper bound
2. position = clamp(0, delay > 0 ? written-delay : written, upper_bound);
2b. except when not yet started ...
2c. except while stopped ...
I was even considering:
3. if position < This->previous_position stick to previous...
Yet perhaps it's better to allow intermittent garbage values
than to stick to garbage!
OTOH, the delay values I see in the logs are subject to such variation (with PA)
that I'm considering going with a clock instead, or perhaps:
- query delay once per tick (e.g. 10ms) => last_pos
- when asked, compute position from last_pos + time since tick * rate
The last_pos slot may be needed anyway once stopped in pause mode.
Regards,
Jörg Höhle
Re: The sad state of audio GetPosition
On 08/19/2011 01:30 PM, joerg-cyril.hoe...@t-systems.com wrote:
> Reece,
>
> I wrote the text in a hurry and forgot that Wine results
> are most interesting to me with my patch applied.
>
> Please use my patch with something like:
> WINETEST_DEBUG=3 WINEDEBUG=warn+alsa wine mmdevapi_test.exe render
>
>> render.c:897: Test failed: Position 24000 too far after 200ms
> That's not PA's fault. IMHO AudioClient_Stop must not map
> to snd_pcm_drop. It is more like snd_pcm_pause. Or perhaps
> simply lead ALSA into an underrun. I've not made up my mind
> yet as the models (mmdevapi vs. ALSA) are different w.r.t. buffering.
afaict pause, with reset mapped to drop, but for historic reasons
that didn't work (surprise! love from dmix). Even worse,
snd_pcm_drain would deadlock if called twice.
I can't remember why pause didn't work, but if it works go for it.
>
> As you can see, the patch is nowhere final.
>
> #From 60689763bd21513bd9b8dbd2df3abc5f2586f1f2 Mon Sep 17 00:00:00 2001
> #From: =?UTF-8?q?J=C3=B6rg=20H=C3=B6hle?=
> Date: Wed, 17 Aug 2011 21:04:34 +0200
> Subject: winealsa: Play with GetPosition.
>
> ---
> dlls/winealsa.drv/mmdevdrv.c | 52 ++---
> 1 files changed, 43 insertions(+), 9 deletions(-)
>
> diff --git a/dlls/winealsa.drv/mmdevdrv.c b/dlls/winealsa.drv/mmdevdrv.c
> index 3e3edc3..51e9b81 100644
> --- a/dlls/winealsa.drv/mmdevdrv.c
> +++ b/dlls/winealsa.drv/mmdevdrv.c
> @@ -2289,26 +2289,60 @@ static HRESULT WINAPI
> AudioClock_GetPosition(IAudioClock *iface, UINT64 *pos,
> UINT64 *qpctime)
> {
> ACImpl *This = impl_from_IAudioClock(iface);
> -UINT32 pad;
> -HRESULT hr;
> +int err;
> +snd_pcm_uframes_t avail_frames;
> +snd_pcm_sframes_t delay_frames, pad_frames;
> +snd_pcm_status_t *status;
>
> TRACE("(%p)->(%p, %p)\n", This, pos, qpctime);
>
> if(!pos)
> return E_POINTER;
> +snd_pcm_status_alloca(&status);
HeapAlloc(GetProcessHeap(), HEAP_ZERO_FLAG, snd_pcm_status_sizeof()) or
something like that if available please..
> EnterCriticalSection(&This->lock);
>
> -hr = IAudioClient_GetCurrentPadding(&This->IAudioClient_iface, &pad);
> -if(FAILED(hr)){
> +if(!This->initted){
> LeaveCriticalSection(&This->lock);
> -return hr;
> +return AUDCLNT_E_NOT_INITIALIZED;
> }
Unneeded part. Follow that flow..
>
> -if(This->dataflow == eRender)
> -*pos = This->written_frames - pad;
> -else if(This->dataflow == eCapture)
> -*pos = This->written_frames + pad;
> +if((err = snd_pcm_status(This->pcm_handle, status)) < 0){
> +LeaveCriticalSection(&This->lock);
> +ERR("ALSA status error: %d (%s)\n",
> +err, snd_strerror(err));
> +return E_FAIL;
> +}
> +if(0){
> +avail_frames = snd_pcm_status_get_avail(status);
> +delay_frames = snd_pcm_status_get_delay(status);
> +}else{
> +avail_frames = snd_pcm_avail_update(This->pcm_handle);
> +err = snd_pcm_delay(This->pcm_handle, &delay_frames);
> +if(err < 0){ /* e.g. in STATE_PREPARED */
> +ERR("ALSA delay error: %d (%s)\n",
> +err, snd_strerror(err));
> +delay_frames = 0;
> +}
> +}
if 0 is bad...
> +pad_frames = This->bufsize_alsa - avail_frames;
> +#define MAX_LATE_SECONDS 5 /* huge USB or network latency */
> +if(avail_frames <= This->bufsize_alsa + MAX_LATE_SECONDS *
> This->fmt->nSamplesPerSec
> + && delay_frames > 0)
> +*pos = This->written_frames - This->held_frames - delay_frames;
Isn't delay_frames < 0 the definition of underrun? no point in adding
MAX_LATE_SECONDS
> +else if(pad_frames > 0)
> +/* delay may be slightly < 0 past reset */
> +*pos = This->written_frames - This->held_frames - pad_frames;
> +else
> +*pos = This->written_frames - This->held_frames;
> +/* FIXME: if(This->dataflow == eCapture) */
> +ERR("avail %lu, delay %ld, sum %ld, alsa %lu, written %lu, held %u:
> %lu\n",
> +avail_frames, delay_frames, avail_frames+delay_frames,
> This->bufsize_alsa, (ulong)This->written_frames, This->held_frames,
> (ulong)*pos);
> +avail_frames = snd_pcm_avail_update(This->pcm_handle);
> +err = snd_pcm_delay(This->pcm_handle, &delay_frames);
> +ERR("avail %lu, delay %ld, sum %ld, alsa %lu, written %lu, held %u:
> %lu\n",
> +avail_frames, delay_frames, avail_frames+delay_frames,
> This->bufsize_alsa, (ulong)This->written_frames, This->held_frames,
> (ulong)*pos);
>
> LeaveCriticalSection(&This->lock);
>
Getting an avail update again? Why?
The sad state of audio GetPosition
Reece,
I wrote the text in a hurry and forgot that Wine results
are most interesting to me with my patch applied.
Please use my patch with something like:
WINETEST_DEBUG=3 WINEDEBUG=warn+alsa wine mmdevapi_test.exe render
>render.c:897: Test failed: Position 24000 too far after 200ms
That's not PA's fault. IMHO AudioClient_Stop must not map
to snd_pcm_drop. It is more like snd_pcm_pause. Or perhaps
simply lead ALSA into an underrun. I've not made up my mind
yet as the models (mmdevapi vs. ALSA) are different w.r.t. buffering.
As you can see, the patch is nowhere final.
#From 60689763bd21513bd9b8dbd2df3abc5f2586f1f2 Mon Sep 17 00:00:00 2001
#From: =?UTF-8?q?J=C3=B6rg=20H=C3=B6hle?=
Date: Wed, 17 Aug 2011 21:04:34 +0200
Subject: winealsa: Play with GetPosition.
---
dlls/winealsa.drv/mmdevdrv.c | 52 ++---
1 files changed, 43 insertions(+), 9 deletions(-)
diff --git a/dlls/winealsa.drv/mmdevdrv.c b/dlls/winealsa.drv/mmdevdrv.c
index 3e3edc3..51e9b81 100644
--- a/dlls/winealsa.drv/mmdevdrv.c
+++ b/dlls/winealsa.drv/mmdevdrv.c
@@ -2289,26 +2289,60 @@ static HRESULT WINAPI
AudioClock_GetPosition(IAudioClock *iface, UINT64 *pos,
UINT64 *qpctime)
{
ACImpl *This = impl_from_IAudioClock(iface);
-UINT32 pad;
-HRESULT hr;
+int err;
+snd_pcm_uframes_t avail_frames;
+snd_pcm_sframes_t delay_frames, pad_frames;
+snd_pcm_status_t *status;
TRACE("(%p)->(%p, %p)\n", This, pos, qpctime);
if(!pos)
return E_POINTER;
+snd_pcm_status_alloca(&status);
EnterCriticalSection(&This->lock);
-hr = IAudioClient_GetCurrentPadding(&This->IAudioClient_iface, &pad);
-if(FAILED(hr)){
+if(!This->initted){
LeaveCriticalSection(&This->lock);
-return hr;
+return AUDCLNT_E_NOT_INITIALIZED;
}
-if(This->dataflow == eRender)
-*pos = This->written_frames - pad;
-else if(This->dataflow == eCapture)
-*pos = This->written_frames + pad;
+if((err = snd_pcm_status(This->pcm_handle, status)) < 0){
+LeaveCriticalSection(&This->lock);
+ERR("ALSA status error: %d (%s)\n",
+err, snd_strerror(err));
+return E_FAIL;
+}
+if(0){
+avail_frames = snd_pcm_status_get_avail(status);
+delay_frames = snd_pcm_status_get_delay(status);
+}else{
+avail_frames = snd_pcm_avail_update(This->pcm_handle);
+err = snd_pcm_delay(This->pcm_handle, &delay_frames);
+if(err < 0){ /* e.g. in STATE_PREPARED */
+ERR("ALSA delay error: %d (%s)\n",
+err, snd_strerror(err));
+delay_frames = 0;
+}
+}
+pad_frames = This->bufsize_alsa - avail_frames;
+#define MAX_LATE_SECONDS 5 /* huge USB or network latency */
+if(avail_frames <= This->bufsize_alsa + MAX_LATE_SECONDS *
This->fmt->nSamplesPerSec
+ && delay_frames > 0)
+*pos = This->written_frames - This->held_frames - delay_frames;
+else if(pad_frames > 0)
+/* delay may be slightly < 0 past reset */
+*pos = This->written_frames - This->held_frames - pad_frames;
+else
+*pos = This->written_frames - This->held_frames;
+/* FIXME: if(This->dataflow == eCapture) */
+
+ERR("avail %lu, delay %ld, sum %ld, alsa %lu, written %lu, held %u: %lu\n",
+avail_frames, delay_frames, avail_frames+delay_frames,
This->bufsize_alsa, (ulong)This->written_frames, This->held_frames,
(ulong)*pos);
+avail_frames = snd_pcm_avail_update(This->pcm_handle);
+err = snd_pcm_delay(This->pcm_handle, &delay_frames);
+ERR("avail %lu, delay %ld, sum %ld, alsa %lu, written %lu, held %u: %lu\n",
+avail_frames, delay_frames, avail_frames+delay_frames,
This->bufsize_alsa, (ulong)This->written_frames, This->held_frames,
(ulong)*pos);
LeaveCriticalSection(&This->lock);
--
1.7.0.4
Re: The sad state of audio GetPosition
On 18 August 2011 13:04, wrote: > I'm interested in results from: > > - Linux, esp. with something newer than Ubuntu Lucid, or different sound > cards. NOTE: This is with the alsa and oss drivers selected in the audio tab of winecfg. $ lsb_release -a No LSB modules are available. Distributor ID: Ubuntu Description:Ubuntu oneiric (development branch) Release:11.10 Codename: oneiric $ pulseaudio --version pulseaudio 0.9.23 $ cat /proc/asound/version Advanced Linux Sound Architecture Driver Version 1.0.24. $ uname -r 3.0.0-6-generic $ ./wine --version wine-1.3.26 $ ./wine /home/reece/Downloads/mmdevapi_test.exe render 2>&1 | tee render.log fixme:alsa:AudioClient_GetMixFormat Don't know what to do with 32 channels, pretending there's only 2 channels fixme:alsa:AudioClient_GetMixFormat Don't know what to do with 8 channels, pretending there's only 2 channels fixme:alsa:AudioClient_GetMixFormat Don't know what to do with 8 channels, pretending there's only 2 channels fixme:alsa:AudioClient_GetMixFormat Don't know what to do with 8 channels, pretending there's only 2 channels fixme:alsa:AudioClient_GetMixFormat Don't know what to do with 8 channels, pretending there's only 2 channels fixme:alsa:AudioClient_GetMixFormat Don't know what to do with 32 channels, pretending there's only 2 channels render.c:182: Returned periods: 20. ms 10. ms fixme:alsa:AudioClient_GetMixFormat Don't know what to do with 32 channels, pretending there's only 2 channels render.c:194: pwfx: 00130E20 render.c:195: Tag: fffe render.c:196: bits: 32 render.c:197: chan: 2 render.c:198: rate: 48000 render.c:199: align: 8 render.c:200: extra: 22 render.c:205: Res: 32 render.c:206: Mask: 3 render.c:207: Alg: FLOAT render.c:276: Returned latency: 50. ms render.c:350: Test failed: IsFormatSupported(8000x 8x1) call returns render.c:351: IsFormatSupported(8000x 8x1) returns render.c:350: Test failed: IsFormatSupported(8000x 8x2) call returns render.c:351: IsFormatSupported(8000x 8x2) returns render.c:350: Test failed: IsFormatSupported(8000x16x1) call returns render.c:351: IsFormatSupported(8000x16x1) returns render.c:350: Test failed: IsFormatSupported(8000x16x2) call returns render.c:351: IsFormatSupported(8000x16x2) returns render.c:350: Test failed: IsFormatSupported(11025x 8x1) call returns render.c:351: IsFormatSupported(11025x 8x1) returns render.c:350: Test failed: IsFormatSupported(11025x 8x2) call returns render.c:351: IsFormatSupported(11025x 8x2) returns render.c:350: Test failed: IsFormatSupported(11025x16x1) call returns render.c:351: IsFormatSupported(11025x16x1) returns render.c:350: Test failed: IsFormatSupported(11025x16x2) call returns render.c:351: IsFormatSupported(11025x16x2) returns render.c:350: Test failed: IsFormatSupported(12000x 8x1) call returns render.c:351: IsFormatSupported(12000x 8x1) returns render.c:350: Test failed: IsFormatSupported(12000x 8x2) call returns render.c:351: IsFormatSupported(12000x 8x2) returns render.c:350: Test failed: IsFormatSupported(12000x16x1) call returns render.c:351: IsFormatSupported(12000x16x1) returns render.c:350: Test failed: IsFormatSupported(12000x16x2) call returns render.c:351: IsFormatSupported(12000x16x2) returns render.c:350: Test failed: IsFormatSupported(16000x 8x1) call returns render.c:351: IsFormatSupported(16000x 8x1) returns render.c:350: Test failed: IsFormatSupported(16000x 8x2) call returns render.c:351: IsFormatSupported(16000x 8x2) returns render.c:350: Test failed: IsFormatSupported(16000x16x1) call returns render.c:351: IsFormatSupported(16000x16x1) returns render.c:350: Test failed: IsFormatSupported(16000x16x2) call returns render.c:351: IsFormatSupported(16000x16x2) returns render.c:350: Test failed: IsFormatSupported(22050x 8x1) call returns render.c:351: IsFormatSupported(22050x 8x1) returns render.c:350: Test failed: IsFormatSupported(22050x 8x2) call returns render.c:351: IsFormatSupported(22050x 8x2) returns render.c:350: Test failed: IsFormatSupported(22050x16x1) call returns render.c:351: IsFormatSupported(22050x16x1) returns render.c:350: Test failed: IsFormatSupported(22050x16x2) call returns render.c:351: IsFormatSupported(22050x16x2) returns render.c:350: Test failed: IsFormatSupported(44100x 8x1) call returns render.c:351: IsFormatSupported(44100x 8x1) returns render.c:350: Test failed: IsFormatSupported(44100x 8x2) call returns render.c:351: IsFormatSupported(44100x 8x2) returns render.c:350: Test failed: IsFormatSupported(44100x16x1) call returns render.c
The sad state of audio GetPosition
Hi, the good news first: I've some code which seems to return the "position of the sample that is currently playing through the speakers" that MSDN requires GetPosition to return. It passes my mmdevapi rendering tests when used with ALSA's dmix and hw:0 devices. The new WinMM over mmdevapi depends on GetPosition for its buffer management. Hence a correct GetPosition seems essential for Wine. The bad news: I've logs which unambiguously prove that at least in Ubuntu Lucid (2010), PulseAudio yields values of snd_pcm_avail_update and snd_pcm_delay that are completely inadequate to compute what GetPosition should return. I'll write more about that in the ALSA or PA mailing list. Before doing that, I'd like to sample the behaviour of my rendering tests on a few machines. You'll find the executable at http://testbot.winehq.org/JobDetails.pl?Key=13489 I'm interested in results from: - Linux, esp. with something newer than Ubuntu Lucid, or different sound cards. - Native, because testbot shows some failures and I'd like to see results from real machines, not vmware. Beware: - Ignore the test failures about IsFormatSupported/Initialize. I'm misusing ok() for reporting. - Please send the complete log, not the short form with failures only, because the test now loops twice: once for SHARED mode, once for EXCLUSIVE. Without the complete log, it's not possible to tell from the line number only which mode is in use. - The test may crash in Wine, after my loop, in test_session, because sometimes Initialize fails and the test doesn't catch that. Testbot shows failures such as: render.c:1034: Test failed: Position 56880 too far after 1070ms render.c:1035: Test failed: Position delta 5712 not regular I've yet to analyse testbot's logs in detail. - "Position delta" means that after 100ms, one expects the speaker position to advance by ~4800 (or 48000) positions. How come it can be 5712? - "Position too far" means that after 1000ms of playing, the speaker position cannot be larger than FramesPerSecond (modulo Frequency). How can it be 56880 when I expect 48000? (modulo some period increments) Regards, Jörg Höhle
