[PATCH] winealsa.drv: Fix AudioRenderClient write pointer calculation

Joerg-Cyril.Hoehle at t-systems.com Joerg-Cyril.Hoehle at t-systems.com
Wed Aug 10 03:53:09 CDT 2011


Hi,

The buffering code in winealsa & wineoss is still very complex and
I've had a hard time following it.  Because of the complexity, it's
potentially error-prone.  Here's an error:

Consider this sequence of calls:
           BufferSize 100
           GetBuffer(80)
           Release(80)
           pcm_write(50)
=> offs = 50, held = 30 (80-50)
           GetBuffer(30) (max 70)
=>LOCKED_WRAPPED (because pos 80 + 30 is > 100)
           Release(30)
=> wrap_buffer 20 at end & 10 at beginning
=> offs = 50, held = 60 (note offs+held > 100)
           GetBuffer(10) (max 40)
=>LOCKED_NORMAL (fits into primary)
           Release(10)
=>computes buffer = offs+held = 50+60 = 110 > 100!
=>illegal memory access

An easy fix would be to correct
         buffer = This->local_buffer +
+            (This->lcl_offs_frames + This->held_frames) * This->fmt->nBlockAlign;
into (offs+held %buffer_size)

However are we sure that's all?

I'd love to see the code as understandable as winecoreaudio (although
I know there need be more state variables to handle the two buffers).

Precisely because winecoreaudio is more readable I was able to fix the
Get/ReleaseBuffer ordering issue, while ALSA and OSS are still waiting
(waiting too for another patch to have GetCurrentPadding simply return
held_frames on top of it).

IOW, the winecoreaudio driver is the best of the three, receiving
fixes written easily, while ALSA and OSS are lagging behind because
it's tedious to patch them correctly.

Regards,
 Jörg Höhle


More information about the wine-devel mailing list