dsound: clamp input buffer size at 62.5 ms

[dechamps]

This PR works around a DirectSound limitation where input host buffer sizes smaller than 31.25 ms are basically unworkable and make PortAudio hang. The workaround is to impose a minimal buffer size of 2*31.25 ms on input-only and full-duplex streams. This is enough for the read cursor to advance twice around the buffer, basically resulting in de facto double buffering.

This change was tested with paloopback under a wide variety of half/full-duplex, framesPerBuffer, and suggested latency parameters. (Note the testing was done on top of #772 as otherwise paloopback is not usable.)

In addition, this PR includes a small refactoring of the DirectSound buffer calculation code which was giving me headaches as I was working on the fix.

Fixes #775

[philburk]

@dechamps - Thanks for doing this. Handling quirks in the underlying host APIs is an important feature for PortAudio.

[RossBencina]

This change was tested ...

@dechamps please could you update the PR description with information about which OS version(s) and audio devices were used for your testing. This gives us traceability if someone wants to compare your results later.

[RossBencina]

Based on my testing reported at #775, I can get stable bidirectional audio with a 5.3ms input buffer size no problems with standard Realtek motherboard sound. I do get stalling with low output buffer sizes, so maybe there is a bug, but I don't think we have characterized it correctly yet. In any case with the available information, I don't think it's appropriate to put a blanket clamp to 31ms. I think we should continue the root-cause analysis at #775.

If it were the case that there are no systems where lower input buffer size works, or there was a way to detect that we were on such a system where the buffer limit applied, then the clamp would make total sense.

While I'm all for quirk workarounds, my take is that there will always be "low" latency/buffer settings which will produce unreliable output on some or all systems with some or all host APIs. It's up to the end user to tweak the buffering for best performance on their system. And that includes discovering the system limits. In other words, we should allow for parameter tuning beyond functioning limits. This PR prevents that.

If your goal is to provide a "foolproof" configuration that never fails due to quirks of some devices (by clamping even if it degrades latency on some system), then that logic should be put in the application code. (i.e. the application implements the clamp). PortAudio already provides some infrastructure to support this in PaDeviceInfo:

portaudio/include/portaudio.h

Lines 510 to 515 in 68e963a

	/** Default latency values for interactive performance. */
	PaTime defaultLowInputLatency;
	PaTime defaultLowOutputLatency;
	/** Default latency values for robust non-interactive applications (eg. playing sound files). */
	PaTime defaultHighInputLatency;
	PaTime defaultHighOutputLatency;

It might make sense to update these default latency values to reflect the current findings especially for the non-interactive case.

[dechamps]

Based on my testing reported at #775, I can get stable bidirectional audio with a 5.3ms input buffer size no problems with standard Realtek motherboard sound. I do get stalling with low output buffer sizes, so maybe there is a bug, but I don't think we have characterized it correctly yet. In any case with the available information, I don't think it's appropriate to put a blanket clamp to 31ms. I think we should continue the root-cause analysis at #775.

I just commented on #775 to show you that you did precisely reproduce the problem I characterized. Given the various indications that this is a widespread problem, and no evidence that there exists systems that aren't affected, I would say the blanket clamp is appropriate.

If it were the case that there are no systems where lower input buffer size works

It certainly looks like it. And you can add your own system to that list since you did reproduce it.

It's up to the end user to tweak the buffering for best performance on their system. And that includes discovering the system limits. In other words, we should allow for parameter tuning beyond functioning limits. This PR prevents that.

If your goal is to provide a "foolproof" configuration that never fails due to quirks of some devices (by clamping even if it degrades latency on some system), then that logic should be put in the application code.

If this failure mode merely resulted in glitchy audio, then I would agree (for example this is why I closed #788 instead of trying to implement a clamp in MME). However the failure mode we're dealing with here is much worse - it doesn't just cause glitchy audio, instead it results in a stall which is impossible to recover from and can easily compromise the stability of the application (deadlock), forcing the user to kill the app and restart it. I don't think this is acceptable behaviour from any library under any circumstances, regardless of parameters.

[mirh]

Isn't there somebody you forgot to ask?
(cue XP, especially given that's presumably the major use case for dsound)

[dechamps]

Isn't there somebody you forgot to ask?
(cue XP, especially given that's presumably the major use case for dsound)

Do you have evidence that #775 does not apply to XP? If so, I would be happy to add some kind of version check for that.

Also, to be clear: even if this new code runs on systems that are not subject to #775, that still doesn't mean these systems would break. The worst that can happen is that there might be some increase to the minimum achievable DS input latency, and even that is not certain (it depends on various parameters and on how DS relates input read cursor granularity to buffer size).

On the other hand, #775 does break modern systems in a pretty bad way.

(cue XP, especially given that's presumably the major use case for dsound)

My project does not even support XP, yet it uses dsound by default. This is because, when that decision was made, dsound looked like the most "modern" PortAudio Windows Host API that seemed reliable enough (WASAPI had lots of problems back then, and I suspect it still does, but I need to look into it).

I don't think it's fair to assume that users are using DS primarily to be compatible with XP. If a user cares deeply about compatibility with old platforms, then I would expect them to use MME which is the default and is more reliable and more widely supported than DS.

[mirh]

Do you have evidence that #775 does not apply to XP?

Not really.
But it would still seem almost impossible tbf?
It's not just that some functions may have been rewritten (or abstracted to wasapi), in your usual XP machine a lot of of the processing was up to the audio driver itself.
So even going from a realtek to an nforce to a xfi could give you pretty different "low level" results.

I don't think it's fair to assume that users are using DS primarily to be compatible with XP.

My judgement was just theoretical, idk about history or implementation quirks.
Of course not all shortcomings may matter to certain use case, but it seems pretty obvious that it is a deprecated second-tier api.

[dechamps]

Do you have evidence that #775 does not apply to XP?

Not really.
But it would still seem almost impossible tbf?

Okay, fair enough. Given that #775 affecting pre-Vista Windows seems unlikely, and given that the buffer size clamp could plausibly hurt low-latency DS users who are still on XP, I have updated this PR to do a version check so that it is a no-op on pre-Vista Windows.

I think all concerns with this PR have been addressed now?

[RossBencina]

My personal status: I want to do more local testing before I comment further.

[philburk]

We are considering this PR for the next release, milestone 19.8.
We need to do a more through review and resume the conversation.

[RossBencina]

As I understand it, #772 is blocking this, and I will look into that one.

[dechamps]

As I understand it, #772 is blocking this, and I will look into that one.

No, this PR does not depend on #772. #772 is only required if you want to test this PR using paloopback. This is because paloopback shows DirectSound as glitchy - regardless of this PR - due to #770, and #772 is the fix for that.