Refactor PortAudio backend

[sakertooth]

Applies various changes in an attempt to resolve certain issues with the PortAudio backend and improve the quality of the code.

This PR fixes DirectSound and MME when using PortAudio, but for some reason WDM-KS and WASAPI still lay down broken. This might have to be looked into later as I couldn't find a solution.

[bratpeki]

How does one test this? Should I play two demo tracks and look at the latency?

[bratpeki]

Here's my config, @sakertooth, with recent files and directory paths obscured:

<?xml version="1.0"?>
<!DOCTYPE lmms-config-file>
<lmms version="1.3.0-alpha.1.740+g303215f8b" configversion="3">
  <MidiJack device="lmms"/>
  <app loopmarkermode="dual" nanhandler="1" openlastproject="0" nommpz="0" sololegacybehavior="0" configured="1" displaydbfs="1" disablebackup="0" language="sr"/>
  <audioengine mididev="WinMM MIDI" framesperaudiobuffer="256" audiodev="SDL (Simple DirectMedia Layer)"/>
  <audiojack clientname="lmms" channels="2"/>
  <audioportaudio backend="" device=""/>
  <audiosdl inputdevice="" device=""/>
  <midi midiautoassign="none" autoquantize="0"/>
  <paths ladspadir="..." workingdir="..." stkdir="data:/stk/rawwaves/" defaultsf2="..." sf2dir="..." vstdir="..." theme="data:/themes/default/" backgroundtheme="" gigdir="..."/>
  <tooltips disabled="0"/>
  <ui saveinterval="2" enablerunningautosave="0" displaywaveform="1" vstembedmethod="none" animateafp="1" smoothscroll="0" sidebaronright="0" trackdeletionwarning="0" printnotelabels="1" mixerchanneldeletionwarning="1" compacttrackbuttons="0" disableautoquit="0" vstalwaysontop="0" oneinstrumenttrackwindow="0" enableautosave="0" letpreviewsfinish="0"/>
  <recentfiles>
  ...
  </recentfiles>
</lmms>

[bratpeki]

Saker claims this is merge-ready, as there is little to be done left.

MSYS compilation yields four backend, with the following results, after some crude testing:

1. For WDM-KS, the output is "Could not open PortAudio: Invalid device"
2. For WASAPI, the output is "Could not open PortAudio: Invalid sample rate"
3. DirectSound works
4. MME works

Additionally, with WDM-KS, with certain outputs selected, LMMS won't report the above message, but will instead hang and result in the following output:

Lv2 plugin SUMMARY: 0 of 0  loaded in 0 msecs.
(This is where LMMS hangs after being disabled. Ctrl-C shows the below text)
QObject::killTimer: Timers cannot be stopped from another thread
QObject::~QObject: Timers cannot be stopped from another thread

[bratpeki]

That should be it, really. Let me know if anything more is needed!

[bratpeki]

Checked again!

For WDM-KS, the output is "Could not open PortAudio: Invalid device"
For WASAPI, the output is "Could not open PortAudio: Invalid sample rate"
DirectSound works
MME works

Still true, all of it, including the hanging.

However, 2/4 backends now work well, which I think is great!

Please merge ASAP. 🚀

[sakertooth]

Please merge ASAP.

GitHub demands your blessing with an explicit approval before I can merge (in general though it needs one approver before merge, at least for this repository in particular currently). I mentioned one last test if you want to take that on though, which is just checking if MSVC builds work for WASAPI and WDM-KS. Here's a link to them for this PR.

[bratpeki]

Tested with MSVC. Pretty much the same as MSYS2:

• MME works.
• DirectSound works.
• WASAPI doesn't create sound.
• WDM-KS doesn't create sound. It also hangs the program, so it remains active in the Task Manager after the program is closed from the GUI.

Gonna test using MinGW again as well.

[sakertooth]

Tested with MSVC. Pretty much the same as MinGW:

• MME works.

• DirectSound works.

• WASAPI doesn't create sound.

• WDM-KS doesn't create sound. It also hangs the program, so it remains active in the Task Manager after the program is closed from the GUI.

Gonna test using MinGW again as well.

This is truly incredible

[bratpeki]

Corrected #7444 (comment).

MinGW features only two backends:

• MME works.
• WDM-KS doesn't create sound. Also, unlike MSVC, it doesn't hang the program? Odd!

[bratpeki]

After trying both, it seems neither of them generate the hanging behaviour, LOL!

In any case, I'd say that PortAudio is fixed, for the most part. It doesn't chop like it used to, and the ability to specify the channel count has been added. I would investigate this further outside of this PR and we can make future improvements to PortAudio.

[bratpeki]

Approved from a tester standpoint; I haven't looked at the code.

[bratpeki]

Yup, I can't reproduce the hanging now, somehow! Must be something to do with running MinGW before/after MSVC, that's my best guess.

[tresf]

@bratpeki asked me on Discord to test macOS support.

• ✅ PASS: Sound output: Testing demos/unfa-Spoken.mmpz, built-in speakers: Playback is fine
• 🚫 FAIL: Testing with Apple Airpods, each airpod shows as a single 1-channel device (sometimes they report as 0-channel and 2-channel respectively), playback is broken, closing and reopening the software will always re-prompt for the device.
  • Master shows two devices as well, but playback is possible and it does not re-prompt for the device when closing and reopening

[sakertooth]

Pushed the last few fixes I had in mind. Will merge soon if there are no objections. Any other potential issues might have to be looked at later.

[bratpeki]

There is definitely the issue stated by @tresf.

[tresf]

Pushed the last few fixes I had in mind. Will merge soon if there are no objections. Any other potential issues might have to be looked at later.

They aren't potential issues, they're actually issues because the behavior regresses from master.

Here's what the terminal says:

Could not open PortAudio: Invalid number of channels
No audio-driver working - falling back to dummy-audio-driver
You can render your songs and listen to the output files...

I wasn't able to get this message to reproduce reliably, but it's the closest thing to a usable error I was able to obtain. No matter of configuration will allow the AirPods to work on this PR.

[sakertooth]

Per agreed discussion on Discord, the PR will be modified to select the first working sample rate. In some tester's cases this will fix many back-ends, but in the OP's as well as my own experience, this will fix WASAPI, similar to how stable-1.2 behaves.

Should the user choose the correct working sample rate instead? I don't want to make a lie out of the config. This would probably require showing a dialog box to the user though, instead of just printing to console.

[tresf]

Per agreed discussion on Discord, the PR will be modified to select the first working sample rate. In some tester's cases this will fix many back-ends, but in the OP's as well as my own experience, this will fix WASAPI, similar to how stable-1.2 behaves.

Should the user choose the correct working sample rate instead? I don't want to make a lie out of the config. This would probably require showing a dialog box to the user though, instead of just printing to console.

When you say "instead" do what condition do you describe? For example, do you mean a condition such as where the "first working" sample rate can't be determined?

[sakertooth]

When you say "instead" do what condition do you describe? For example, do you mean a condition such as where the "first working" sample rate can't be determined?

I mean on the condition when the sample rate chosen in the config doesn't work.

[tresf]

When you say "instead" do what condition do you describe? For example, do you mean a condition such as where the "first working" sample rate can't be determined?

I mean on the condition when the sample rate chosen in the config doesn't work.

Oh, good question. Messaging would certainly be nice.

[sakertooth]

Oh, good question. Messaging would certainly be nice.

Great. I'll try to implement this, but the error handling situation is a bit messy. I need to report some dialog back to the GUI when trying to initialize the playback devices, but throwing an error would skip the rest of the initialization. Might need a bit of refactoring.

[sakertooth]

Oh, good question. Messaging would certainly be nice.

Hey. Its been awhile but I came back to this PR and added a popup message when an audio device fails to be setup for playback, which also advises the user to try adjusting their settings. This is nice because the application never reaches in and changes the config under the user's nose (e.g., to change to a working sample rate), which might be unexpected/unusual.

I also made some general changes/bug-fixes. Would appreciate any Windows testing as usual.

[tresf]

Would appreciate any Windows testing as usual.

I managed to resurrect a Windows laptop since our last testing, so I'd happily retest.

[tresf]

Testing HP Spectre 360 Laptop Windows 11, integrated Realtek Audio, 48000 sample rate (default)

Summary:

The only observable regression -- consistent with previous tests -- is the need to manually change the samplerate to fix WASAPI. Since we had discussed the possibility of the back-end automatically determining this samplerate for us, I feel like there may still be a bug with the implementation. I deleted the .lmmsrc.xml between tests and all PortAudio back-ends default to 441000, rather than 48000 which is the only rate offered by this particular laptop.

Note that these tests were performed on a different physical machine than previous tests.

Backend	stable-1.2	pr7444-msvc	pr7444-mingw
MME	🚫 Audio choppy/not playable	🚫 Audio choppy/not playable	🚫 Audio choppy/not playable
Windows DirectSound	🚫 Audio is choppy and not playable	🚫 Playback will not start	(missing from mingw)
Windows WASAPI	✅ Playback is fine	⚠️ Playback works only after MANUALLY selecting 48000 sample rate	(missing from mingw)
Windows WDM-KS	🚫 Init fails, fallback to dummy audio	🚫 Init fails, fallback to dummy audio	🚫 Init fails, fallback to dummy audio

[sakertooth]

Testing HP Spectre 360 Laptop Windows 11, integrated Realtek Audio, 48000 sample rate (default)

Summary:

The only observable regression -- consistent with previous tests -- is the need to manually change the samplerate to fix WASAPI. Since we had discussed the possibility of the back-end automatically determining this samplerate for us, I feel like there may still be a bug with the implementation. I deleted the .lmmsrc.xml between tests and all PortAudio back-ends default to 441000, rather than 48000 which is the only rate offered by this particular laptop.

Note that these tests were performed on a different physical machine than previous tests.
Backend stable-1.2 pr7444-msvc pr7444-mingw
MME 🚫 Audio choppy/not playable 🚫 Audio choppy/not playable 🚫 Audio choppy/not playable
Windows DirectSound 🚫 Audio is choppy and not playable 🚫 Playback will not start (missing from mingw)
Windows WASAPI ✅ Playback is fine ⚠️ Playback works only after MANUALLY selecting 48000 sample rate (missing from mingw)
Windows WDM-KS 🚫 Init fails, fallback to dummy audio 🚫 Init fails, fallback to dummy audio 🚫 Init fails, fallback to dummy audio

Yeah I don't think its in our control at this point.

[sakertooth]

Since we had discussed the possibility of the back-end automatically determining this samplerate for us, I feel like there may still be a bug with the implementation.

If there was a popup that said there was a failure to load the PortAudio device when you used the 44100 sample rate and they should try adjusting their settings, then I think this is fine IMO. My reasoning for this was stated in my messages a few months ago. If there was no popup, that's a problem.

Still a bummer that the other Windows APIs didn't work though.. I feel like maybe this PR isn't even useful at that point. My initial hope was that I could maybe fix those APIs not working :(

[tresf]

If there was a popup that said there was a failure to load the PortAudio device when you used the 44100 sample rate and they should try adjusting their settings, then I think this is fine IMO. My reasoning for this was stated in my messages a few months ago. If there was no popup, that's a problem.

Backends don't pop-up invalid settings until after a restart, sadly (and even then, unless you're reading the logs, it's only implied when the dailog shows). Not the fault of this PR of course, it's been this way. I see code for a new pop-up, I don't see that at all.

Furthemore -- and this is an unrelated issue -- we really need a way to allow the settings screen to change the backend without a restart. This is out of scope for this PR, but (as an example) it's very confusing to start LMMS with it working on SDL, open the settings screen to change to PortAudio, only to have to restart for it to use PortAudio, and then only know if PortAudio is broken when the Settings screen re-appears. This is a pretty big usability issue.

Still a bummer that the other Windows APIs didn't work though.. I feel like maybe this PR isn't even useful at that point. My initial hope was that I could maybe fix those APIs not working :(

Well, the code cleanup is always welcome. These backends are horribly broken on Windows anyways, I won't stop a merge because of those test results. Cleaner code is easier to troubleshoot.

Somewhat related, the project has the $$$ to send you a Windows laptop if you wanted to debug this in -- say msys2 locally. We'd be happy to send you something to work on this more, even after it's merged.

[sakertooth]

Backends don't pop-up invalid settings until after a restart, sadly (and even then, unless you're reading the logs, it's only implied when the dailog shows). Not the fault of this PR of course, it's been this way. I see code for a new pop-up, I don't see that at all.

Furthemore -- and this is an unrelated issue -- we really need a way to allow the settings screen to change the backend without a restart. This is out of scope for this PR, but (as an example) it's very confusing to start LMMS with it working on SDL, open the settings screen to change to PortAudio, only to have to restart for it to use PortAudio, and then only know if PortAudio is broken when the Settings screen re-appears. This is a pretty big usability issue.

So having the popup of invalid settings showing only after a restart is because that's the only time in the code we actually open the device and check if we can load the device from it. As you discovered, we don't currently change the device immediately after the settings are applied.

Though I do agree how this can be an issue. It is more reasonable and efficient for the application to check if the settings work immediately after applying them, and refuse to apply them on failure. This can work for backends like PortAudio since it doesn't require us to actually open the device to check if we can load from it (it has a simple function for this that doesn't require a device to be opened), but I'm not sure about the other backends.

Somewhat related, the project has the $$$ to send you a Windows laptop if you wanted to debug this in -- say msys2 locally. We'd be happy to send you something to work on this more, even after it's merged.

I like working with a single laptop since it's less technology to manage, but I appreciate the offer. I have plans to make some disk space on my current laptop and set up a lightweight Windows VM to debug this a bit more closely.