[PATCH] wineoss.drv: Fix IAudioRenderClient::{Get,Release}Buffer protocol

Andrew Eikum wrote:
> I think the GetPosition and GetCurrentPadding failures are caused by
> using GETOSPACE instead of GETODELAY in GetPosition().

Please write that patch too. It needs good testing to get the start/stop/underrun corner cases right.

My hope is that Wine-1.4.0 will have 3 drivers of comparable if not equal quality.

What I plan is to submit my render patch with almost all GetPosition tests lifted behind if(winetest_debug>1):
render.c:1062: Tests skipped: Rerun with WINETEST_DEBUG=2 for GetPosition tests.

This one perhaps should move as well, although it indicates a problem.
render.c:1166: Test failed: Position should have been further along...

render.c:1227: Test failed: GetCurrentPadding returned 512, should be 0
No idea, please check.

render.c:1228: Test failed: Position 142756 at end vs. 143268 submitted frames
In shared mode, the end position must be the sum of released frames.
In exclusive mode, I've seen some cards return a little less.

Your RenderClient patch looks good.

Regards,
 Jörg Höhle

[PATCH] wineoss.drv: Fix IAudioRenderClient::{Get,Release}Buffer protocol

[Andrew Eikum]

On Tue, Jan 10, 2012 at 04:57:58PM +0100, [e-mail] wrote:
> Andrew Eikum wrote:
> > I think the GetPosition and GetCurrentPadding failures are caused by
> > using GETOSPACE instead of GETODELAY in GetPosition().
>
> Please write that patch too. It needs good testing to get the start/stop/underrun corner cases right.

I've attached it here, but see caveats below...
> render.c:1227: Test failed: GetCurrentPadding returned 512, should be 0
> No idea, please check.

This is because of the workaround for an OSS bug. If there's less than
a full fragment of data in the OSS buffer, it will report some
unreliable value between 0 and fragment_size (inclusive), even in
underrun condition. Consider the conflicting scenarios:

Get/ReleaseBuffer(`size' less than fragment_size frames)
GetCurrentPadding() must return exactly `size'

vs

Get/ReleaseBuffer(`size' greater than fragment_size frames)
Start()
...Underrun
GetCurrentPadding() must return 0

In both cases, GETOSPACE returns something between 0 and fragment_size
in the GCP() call. It's hard to work around this bug... I'm trying to
come up with something now, possibly detecting and special-casing
underrun.
> render.c:1228: Test failed: Position 142756 at end vs. 143268 submitted frames
> In shared mode, the end position must be the sum of released frames.
> In exclusive mode, I've seen some cards return a little less.

And here we have a somewhat similar problem:

Get/ReleaseBuffer(`size' = a lot of frames)
Start()
GetPosition() must return `size' - delay

vs

Get/ReleaseBuffer(`size' = a lot of frames)
Start()
...Underrun
GetPosition() must return `size'

Again, we have to detect "almost empty" vs. "empty" with broken OSS.

Anyways, I'm working on it.

Andrew