[PATCH] winepulse: Add pulse driver, v8

[Maarten Lankhorst]

Op 29-02-12 17:29, Joerg-Cyril.Hoehle at t-systems.com schreef:
> Hi,
>
> Chris is right about the format. The shared mode mixer ought to return FLOAT(32),
> and it always appears to return the extensible format as a consequence.
> For weird reasons, wineoss may return integer formats, but that's certainly
> going to cause some unexpecting app to crash.
I think it's because wineoss may not always have float support because
mixing float in the kernel would require special attention.
>> +    assert(oldpad >= This->pad);
> Wasn't there a discussion some time ago that asserts are bad?
> Better back out, return INVALIDATED or some such, but don't crash the app
> just because there's a problem with sound. (I agree that winmm&mmdevapi
> ought to contain self/consistency tests).
This is an internal consistency problem, somehow we have more sound queued
than we know about. I think crashing is the best thing it's a genuine bug that
should never be possible, if I don't crash it will manifest itself differently,
presumably in a subtle impossible to find way..
Outside of that callback, the assertion
This->pad == This->bufsize_bytes - pa_stream_writable_size should hold.

> It would be nice if you could attach some
> WINETEST_DEBUG=2 render.log 2>&1 to bugzilla somewhere.
> There is stuff that do not cause a failure yet diverges from native.
>
> +static void pulse_wr_callback(pa_stream *s, size_t bytes, void *userdata)
> +    if (This->event)
> +        SetEvent(This->event);
> This should be the last thing in the callback.  It may well cause your thread
> to be kicked out by the scheduler.
Oops, thanks for catching, it's why the other setevents are the last indeed.
> +    if (filled_one && This->event)
> +        SetEvent(This->event);
> Don't the capture tests prove that events get in even when the buffer is full? (please cross-check)
You should get an event then, I steal the earliest full buffer. :)
I don't think the full case is completely correctly handled yet.
Unlike pulseaudio won't send multiples of fragsize, unlike for
rendering, but since pa_stream_peek is guaranteed to return
the same data until I drop it with pa_stream_drop, I suppose I
could add a workaround that will only fill a multiple of periods.
>> - Align buffer size to a multiple of period size
> How can you pass the tests with that? It's wrong with both capture (annoyingly IMHO) and playback.
>
> I don't have more time now (and know nothing about the pa_* API).
> Is pa_mainloop_run the thread that dispatches the pulse_xyz_callbacks?
>
> I wouldn't mind a winepulse driver in 1.4.0
>
I only really need it for capture, rendering needs it too since the tests
show that this is the case, but there's nothing in the code that depends
on it. For capture it's different, you need to keep track of packets somehow.
This is why I keep the meta information in little packets until the application
has come around to read them. Having one packet that's not a period
is a pain, and seems to be against the tests too that everything is a
multiple of period.

~Maarten