mmdevapi:render test failures

[Andrew Eikum]

On Mon, Nov 09, 2020 at 02:04:18PM +0100, Francois Gouget wrote:
> 
> So mmdevapi:render keeps failing randomly due to timing issues.
> All the random failures seem to follow this model:
> 
> render.c:1091: Test failed: Position 50880 too far after playing 100ms
> 
> 1077    hr = IAudioClient_Start(ac); /* #1 */
> 1080    Sleep(100);
> 1081    slept += 100;
> 1087    hr = IAudioClock_GetPosition(acl, &pos, NULL);
> 1090    /* in rare cases is slept*1.1 not enough with dmix */
> 1091    ok(pos*1000/freq <= slept*1.4, "Position %u too far after playing %ums\n", (UINT)pos, slept);
> 
> There are a number of issues with that test:
> 
> * Sleep(100) never sleeps for 100 ms!
>   It either sleeps for 93.6, 109.2 or even 124.8 ms depending on the 
>   Sleep() history. In any case it's always a multiple of 15.6 ms. So 
>   line 1081 is wrong.
> 
> * IAudioClient_Start() takes between 1.5 and 3 ms. There is no telling 
>   when the audio actually starts during that interval. So that adds up 
>   to 3 ms to the play time.
> 
> * In turn that means the 1.4 factor on line 1091 does not provide a 
>   'safety margin' but really only compensates for the bug on line 1081.
> 
> * And in any case, given the very short duration, I feel this is going 
>   to test how the driver / hardware deals with buffering and latency 
>   more than anything else. (even if the driver claims the latency is 0)
> 
> 
> So what is the goal of that test? I mean besides "Perform renderer 
> padding & position tests".
> 

Some applications are really picky about the relationship between
GetPosition, GetCurrentPadding, and actual wall clock time. I think
those tests were added to try to show that Wine is meeting the timing
requirements that Windows meets. But clearly they're not doing a good
job.

> Is the goal to check that an 8-bit stereo stream is not being played as 
> a 16-bit stereo stream, i.e. twice as fast as it should?
> 
> Then, assuming we find a suitable duration for that test, wouldn't it 
> make more sense to set the leeway factor to something between 1.5 (an 
> even split between expected (1) and too fast (2)) and 1.8 (i.e. biased 
> to account for buffering & co).
> 
> And we should be measuring how much time elapsed between just before the 
> Start() call and right after the GetPosition() one to set our 
> expectations.
> 

I'm not strongly opposed to removing them, but how about we try to
improve how they track wall time? Something like the attached patch.

Andrew
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-mmdevapi-render.patch
Type: text/x-diff
Size: 3955 bytes
Desc: not available
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20201109/bb9ad623/attachment-0001.patch>