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