Last week we touched upon the audio system. There was a lot that needed to be focused on, and a lot that happened after the unique pointer change. One of the main adjustments was further refactoring. I wanted to set a key point in trying to modernize the code somewhat, as a lot of it was written around the time the library used C++98. For this blog post, however, I'm not going to be focusing on all of the changes I did. I'm instead going to be a little bit more targeted and focus specifically on certain issues that I was working on. Some of these involved library code changes, other bits involved fixing issues that were found present in the code, and a lot of it involved research into understanding what some of these functions were doing in more detail. So let's start with the easiest.
Fixing Erroneous Function Calls with PulseAudio
One of the first things I really started focusing on was researching in more detail what these function calls were actually doing. I had an idea of what the underlying PulseAudio system was like, that the library itself communicated with the PulseAudio server that ran on the Linux operating system. This is rather easy to understand about Linux if you've worked in the space long enough. It's not as simple as how Windows works, where you're more or less interfacing with a driver, this is a little bit more direct in that it doesn't communicate with the drivers but more or less handles how the audio will be outputted to the drivers. With this distinction in mind, we have to imagine that we are a client that is communicating with the server whenever we are handling audio. Thus all of these function calls have to send a packet to the server, and then receive that packet with additional information on what's happening. I wanted to understand how some functions worked, particularly, a function that I had a curiosity about. This function was `pa_stream_cancel_write`.
Whenever you call to write data to the stream on the server, you have a lot of control over how this data can be written, transferred, and even used. Now what this function is supposed to do is that it is supposed to cancel the write operation. However, from reading the documentation, this function has to be called before the write is fully called. This function is only used with a particular function, `pa_stream_begin_write`. There is a problem with this, we never call that function. So what does the cancel operation do? Well, it does nothing. So when we call to stop the audio, we actually don't stop it at all, we just call to a function that doesn't do anything.
I looked deeper into this, and then started sourcing alternatives, eventually landing on the function `pa_stream_flush` which will stop the playback and empty the buffer fully. This might be what stop is expected to do, however, if not, we can use the cork function to instead pause the playback. The change for this was rather important and definitely made calls to stop much clearer.
Fixing Looping Failure Inside GMusic
This issue was a little bit more involved. There are normally two models you can roll with when doing write operations to PulseAudio. The first is asynchronous, and the second is polling. There was a problem with GMusic, it didn't use either of these models. So what would happen to cause the looping failure was that GMusic was constantly writing to the PulseAudio stream buffer, on the server side. Eventually, due to the sample song being large enough, it crossed the maximum allocation limit allowed for the stream, and the server would assert a failure that the client would receive. This was the failure that was being reported out of `pulsecore` which I mentioned last week. The fix is pretty clear, we just need to adapt one of these models for GMusic. The easiest, since we are already threading, would be polling. All we need is to know the size that's available to write to in the PulseAudio server. We can then wait until the server has a size available, and then write data to it. It doesn't have to be exact, it just needs enough room that it can write the data effectively.
This fix was surprisingly very easy.
Just with these small additions, now GMusic can play audio without overloading the buffer. This also had another benefit. Due to this change, PipeWire can now play the audio perfectly fine. There is still static and popping, but that is a result of another issue that has to be worked on. What this means, however, is that PipeWire support may be entirely possible here soon. However, I'm going to need more time to implement the changes needed for this.
Furthermore, due to both of these fixes, I've also included in the merge request that both of the new issues made around the audio will be fixed upon merge. This means that this merge request will now close a total of 4 issues in one merge, which is something I'm rather looking forward to.
Beyond these two major fixes, there was a plethora of refactoring changes that were done. Most of these work to make the code more readable and understandable. An example of this is the picture below.
Changes like these will not only make the code easier to get into but also make reading it more pleasant. This change was larger than my previous refactors, but it sought to fix the amount of nesting and confusing branch control calls (`break`, `continue`, etc.) that were being done.
Most of this week was devoted entirely to researching and understanding the PulseAudio library. There was a key focus on understanding this before I made any major functional changes. I wanted to ensure that my fixes wouldn't only stand the test of time afterwards, but also be reliable overall for users that are working on the Linux side of code.
With that, this is what I've done this week. It wasn't as intense as the previous weeks, but I found the progress to be more than fruitful for the overall stability of the audio system on Linux.
No comments:
Post a Comment