[PATCH] winepulse: Add pulse driver, v8

[Maarten Lankhorst]

Hey Joerg,

Op 01-03-12 14:39, Joerg-Cyril.Hoehle at t-systems.com schreef:
> Hi,
>
> Maarten Lankhorst wrote:
>>>> - 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 only really need it for capture, rendering needs it too since the tests
>> show that this is the case,
>> and seems to be against the tests too that everything is a multiple of period.
> What is the case? Where did you see that?
>
> Native mmdevapi does not align GetBufferSize on period boundaries,neither render
> nor capture. The current tests may not reveal it, because 500ms is a multiple of 10ms,
> but I've enough additional tests in my git and log data from testbot to be sure about that.
>
> I consider it unsafe to diverge from native when rendering.
Ah ok. I was under the impression it was supposed to, but I guess not then.

>> but there's nothing in the code that depends on it.
> Good.
>
>> For capture it's different, you need to keep track of packets somehow.
>> [...] Having one packet that's not a period is a pain,
> I felt the same pain with winecoreaudio. I think I'm going to agree (with myself) to disagree
> (with mmdevapi), as standards would say, and align capture GetBufferSize on period boundaries.
> This will considerably simplify the code. I've not changed winecoreaudio capture yet.
>
> So I find it ok if winepulse does the same:
> - capture buffer as multiple of periods if it simplifies packet handling,
> - render buffer exactly like native, not a multiple of period (cf. MulDiv in the tests).
>
> And I'll change the capture tests to ensure it asks for buffer sizes as multiples of period,
> so the divergence won't show up :-)
>
> BTW, please use the MulDiv computations so as to minimize differences among the 3-4 drivers.
> Or exhibit tests that prove them wrong.
It is allowed to allocate a buffer with a larger than requested time, see:
http://msdn.microsoft.com/en-us/library/windows/desktop/dd370875%28v=vs.85%29.aspx
The comment in hnsBufferDuration indicates it may allocate a larger buffer.
Also, exclusive mode rendering seems to fail the period check on my laptop.

>> I'd have to recheck on my windows pc
> I forgot: Don't be wary of returning GetMixFormat with 6 channels if PA's device is 5:1. Native
> does that too (we had a few logs on test.winehq with that result)
> (it may be too early if dsound chokes on that, but that would be a bug in dsound).
I return the result of the pulseaudio configuration, which may indeed be 5.1. :)
I should probably do another getmixformat for capturing, because while it can be 5.1
if you're using the monitor stream as source, it's not necessarily the case. For example
5.1 output to speakers with a usb headset at 32kHz rate for capturing.

>> Is it really IsFormatSupported's job to deal with a WAVEFORMATEX struct
>> with only cbSize and wFormatTag and it will get out something sane all the
>> time, no matter how stupid the input?
> I've never seen IsFormatSupported return something else than GetMixFormat and that is EXTENSIBLE.
> For months I thought it would always return S_FALSE and GetMixFormat, no matter how stupid the input, but:
> a) I've no tested this with ADPCM or other such stuff;
> b) I've not tested channel variations with >2 channels, lacking a 5:1 or such card;
> c) recent test.winehq has one Vista machine return E_UNSUPP_FORMAT
>     when the rate was != GetMixFormat. I have a patch in my queue to have the render test accept that variation.
That was my belief when testing too, unfortunately for dsound
and winmm we will have to accept rate != GetMixFormat.rate,
so that is a test I won't be able to pass with winepulse on wine.

Fortunately the windows programs will not break on this, since
they get the rate from GetMixFormat or IsFormatSupported.
> What I believe is:
> 1. During mmdevapi init, MS queries a GetMixFormat from each working device and caches that.
> 2. Later, IsFormatSupported returning GetMixFormat is a cheap operation: clone_format(cached_mix_format).
> None of our drivers work this way.  Doing it would need some thinking about dynamic reconfiguration and plug-in devices.
I'm actually doing similar things. However on moving stream
from old sink to new sink I'll keep returning the metrics of the
initial sink. Hopefully it's not that big a problem, since
pulseaudio will remember the device used for each stream.
The next time the program executes it will get the updated
GetMixFormat for the new sink.

Winepulse does similar things for mix format actually, although
maybe I should just cache the WaveFormatExtensible for
capture and render instead of deriving it at runtime.

~Maarten