Month 5 - Week 4 - Fixing Arch Linux X11 Bug

 Last week we touched upon a bug that was occurring with X11 for Arch Linux. The bug was specifically when creating a window for a test. The window would fail to create and now show up at all, failing the test. I was only modifying the unit tests at the time but took time this week to look more deeply into the actual code to see what could be going wrong.

One of the first things I decided to do was enable the macros inside of GWindow to see the X11 debug output. This would allow me to watch in real time what was failing, and better help towards finding the problem child in the code.

As you can see, the errors being reported are talking about a bad window. This normally means that the pixel map for the internal window is bad, or the window itself failed to create and can’t have anything done to it. I knew immediately the first place to check was the create function for GWindow.

Here we see two functions are called. The first function is just initializing some basic fields, the second function, however, is the one we’re interested in. `OpenWindow()` is the function responsible for creating the window and initializing all the information that GWindow needs to properly manage it.

Remember, we can’t debug, or the error won’t occur. So, instead, I decided to inspect some documentation and play around with the code, probing it to see which function was giving problems. Upon inspection, I noticed that anytime I messed with the `XInitThreads()` function, it had a change in the functionality itself. It seems this might be the problem child I mentioned earlier. I decided to put a `std::this_thread::sleep_for()` call right after calling for the initialization of threads. The sleep time was small, only 10ms, however, doing just this resulted in all of my tests passing consistently.

I found the bug that was occurring. So, how do I fix this then? Well, the moment I discovered this, I made a commit with the sleep call change as the fix. However, using this kind of call to fix the bug isn’t exactly good. Some issues can occur. For example:

• How would this code react on faster or slower hardware? We can’t be certain that this sleep fix will work for all systems, it might even only work on mine.
• Is this truly a threading issue, or a mutex issue? The display for X11 is multithreading capable, so if I don’t lock it, it could be other X11 functions are modifying or trying to modify the variable I’m reading immediately.
• Lastly, what if this fix is temporary like the event system bug months prior? This is a real concern; it would not be the first time I “fixed” a bug only to have it come back spontaneously because I wasn’t doing the idiomatic approach expected.

So, what I have to do is test for different solutions and see what I can find. This was, unfortunately, all I could do for now. I wanted to do more but was constrained on time with due dates and graduation fast approaching. While this is going to be my last blog post, it will not be the last of the changes I make to Gateware. I have full intention of continuing work on Gateware to not only fix this bug, but also to see what I can do to fix, implement, and update features for the project. This is in the hope that I learn more from it over time, that and I simply enjoy working at a low level. So, with that, this is my last blog post.

Month 5 - Week 3 - X11 Fullscreen Bug

 After the final touches to audio, my focus has now shifted to a new and rather interesting bug involving X11 window events and, arguably more importantly, Arch Linux. This bug has shown itself to me for a little while but was largely inconsistent in how it appeared. It would show in the unit tests ~80% of the time, and the source of it seems to be unknown currently. This bug will be my last bug to fix for the project as part of FSU, however, if I can't fix this bug by the deadline, I will be continuing work to fix it after. This is largely because this is the only bug left to fix to finally add Arch Linux as a fully supported platform. This will be brief as I didn't have a lot of time to fully inspect the bug like I wanted to, but there will be enough here to mark my current progress. So, with that, let's get started.

The first thing to note is the description of the bug.

Here are the things to note:

• The bug has a visual aspect (the window will sometimes show, sometimes stay hidden, or show completely but be transparent).
• It fails specifically when the check is run to see if the window is fullscreen.
• Previous calls in the said test don't do anything.

Here is the test.

We can see on the overview that the window is initially created with a basic position (0, 0) and dimension (800, 500). It then reconfigures itself to a different position with different dimensions. Something else you might have taken notice of is the usage of `GiveWindowTimeBeforeRequiringPass`. From what I've gathered, this is here because sometimes X11 requires a bit of time to process events before it can check for what we want. So this is here to effectively wait out the processing of events to check if a function has passed properly. From here, this is where I started testing.

First off, breakpoint debugging is useless for this bug. Because if I placed a breakpoint and walked the function line by line, nothing would go wrong. Debugging would mainly be useful here if I could catch the bug failing in action, however, I'm not able to do that here. So, this leads me to test a little more conventionally. The thing I checked for immediately was timing. It could be that the code is going too quick and preventing X11 from processing everything in time. So I added small delays to check for this, however, it did not work. I also tried using the exact delays used in other tests, but the results were the same.

Interestingly enough, if I duplicated the exact same test and ran the full unit test suite, only the first test would fail, with the duplicate passing. Odd behavior.

From the discord messages above (which are posted in the Gateware Development channel), I described my difficulties with this bug as well as what I feel the bug is.

I feel this bug is a timing bug. Basically, one thread is doing things so quickly that the other thread can't keep up in time. This would explain the odd behavior, and why the test doesn't always fail. Of course, there is more testing and inspecting of the code that will be needed, but in total, this seems to be the main issue. So now, what has to happen, is we have to specialize the probes for this bug to target the timing over the functionality. If I can pinpoint that it is timing-related, then I focus my efforts on fixing that. However, if this isn't the case, I will have to inspect further to see what is exactly happening.

Currently, this is all that I have done for this bug. Next week should hopefully go into a deeper discussion on how I found the fix for it. But for now, this is all.

Month 5 - Week 2 - Final Touches

 Last week, we discussed finishing the WAV file reader and getting the overall status for it marked as completed. There were some final changes. Moving over to GFile for UWP was a big step, but there was still some branching in the code, primarily due to Linux and Windows differences. I decided to focus my energy on getting these rounded out and fixed. This will be a light post, and step through everything fairly quickly.

The thing to really focus on and correct is how Linux and Windows structure the WAV header. Internally, Linux structures a WAV fairly simple, and this is due to it not requiring the extra data that was later attached when Microsoft extended the WAV format. Windows contains many things, however. It contains the primary information which is the same as Linux, but a union that holds additional information. This is also paired with a GUID. The purpose of the union and GUID is unknown to me, but to bridge compatibility with platforms, I had to move away from a platform-specific definition and move to an internal definition, which is exactly what I did.

If you have a keen eye with the Windows API, you will notice immediately that this new structure setup for WavReader is the exact same as Windows's definition. I did this for a simple reason: to support Windows and allow support for Linux. If I used the Windows definition, it wouldn't be able to compile for Linux due to obvious reasons (Windows API not being available for other OSes). After moving to the new structure, it was now the process of hooking everything up and removing old branching.

After making these changes, now WavReader no longer branches and works on every platform universally. Additionally, it acts as a wonderful introduction to what is possible with the library, as all of the heavy lifting for data handling is done by library code (GFile). So a lot of beginners just getting into Gateware could learn a fair bit by reading through the source code of WavReader.

This marks this merge request as complete. I marked my name in the list of contributors, and let the merge get reviewed again. It was merged. The last thing I did for audio was make two merge requests, one focusing on some simple Windows refactors, and the other focusing on UWP refactors. Effectively syncing both to the same modern standard as Linux. In total, everything is now completed.

What now? Now I find a new issue I can tackle, a bug or possible feature that will take only two weeks to implement. With my time almost finished with Gateware, I'm no longer able to tackle such large issues as I have in the past, so that will be left up to others. However, I will try to provide supporting changes here and there, and see what I can do with the time I have left for finals.

Month 5 - Week 1 - Finishing the WAV File Reader

 In the previous week, we discussed and showed the introduction to the WAV file reader I'm implementing. The goal of this reader is to allow WAV files to be universal in logic while retaining the cross-platform nature of supporting not just Linux or Windows, but UWP as well. I'm excluding Mac from this as it already has a native WAV reader provided, so we don't have to implement this, thankfully. Now, I showed the very early stages of this file reader, specifically its Linux and Windows implementation, and what was needed for that. Well, over this week, we added UWP support and a lot of work had to go into that. There were also changes made to the unit tests to support the new UWP usage, among many other things. There's a lot, but I will go over them as quick as I can while retaining the main bits of information.

Firstly, UWP's management for file reading was much different from Linux and Windows. Linux and Windows used GFile for most of the file handling and file communication, however, UWP used an internal Windows library that is within the File API. This API had many quirks that I didn't like, but at the time felt it had to be used, resulting in my reader class getting very cluttered upon initial implementation.

As you can see, there was a lot of code added, and this code specifically was just for opening the file. Some interesting things to note specifically with UWP's file handling:

1. All files are stored as a handle that we pass to functions to perform actions we want. This isn't too unknown and is regular, but is normally hidden, meaning the library is even more low-level than regular filesystem code in C++.
2. `SetFilePointer` on the surface looks to be like a form of seeking inside of a file, but is actually lower-level than that, it is a function that is used to make the seek function.
3. All file paths have to be wide char paths, if they are not, it will fail to open outright.

In total, the File API worked with is extremely low-level and lightweight, but also more bare compared to traditional options. this meant more work had to be put in just to get the state of the UWP support 1:1 with the other platforms. Extra frustrating is that UWP didn't like working with GFile, so I had to take note and continue implementing using this interface. 

By the end of the implementation, it bumped the full source file from around 500 lines of code to over 700 lines. Most of the additions were all focused on preprocessor branching, platform checking, and platform-specific code. Not good since GFile is meant to prevent a lot of the IO branching I had to do. During the meeting, the code was shown in a working state on all platforms and was shown to do the job intended, but I brought this issue up, and we discussed fixes. By the end of the meeting, I was assigned to figure out why GFile was failing and fix it. That's exactly what I did.

I spent a long time simply debugging and understanding the issue. I removed all traces of the File API used and used GFile instead. Here are something interesting I found.

So, come to find out, this was a pathing issue. Something that was happening because GFile wants relative pathing to work with, not absolute pathing. What this means is the unit tests have to change since they give absolute paths to the audio files. We don't want that, we want it to work with relative paths. So, we have to update the unit tests.

So after making all the edits, updating some of the code to reduce branching, and removing all the File API code, the tests now work.

There's still some cleaning I have to do to the code itself before I commit, but once that is done, I can rest easy knowing that my file reader is now using GFile completely, and executing its functionality 1:1 with every platform. The last change needed is replacing the Windows structures used currently for the WAV header info with actual full and proper internal structures. This will remove more branching, and separate any dependencies for this WAV reading functionality. Luckily, this shouldn't take me long and should be a relatively smooth process in total. It will just take some time to fully implement. Overall, this is everything I worked on this week.

Month 4 - Week 4 - WAV File Reader

Last week I went over all the changes with the audio that were done. There was a lot that happened, and a lot of different alterations were made, albeit small just to add finalizing touches. With quite some relief, the changes were merged and are now a part of the official release. However, some more modifications can be done, one, in particular, is the implementation of the WAV file reader. In its current state, there are over six individual implementations. Two for each platform (Windows, UWP, and Linux). I want to implement a universal form of this WAV reader and get it synced, so there is only one implementation that handles everything needed. That way, if we support more WAV features, it will sync across all platforms without us having to fight over all 8 implementations just to get it working as intended. We want easy usability for audio, the less complex, the better. Especially since we want to focus on the APIs we use to make the audio happen rather than the small stuff like WAV file reading implementation.

Linux was rather easy since I had just been working on it for the last month and a half, leading on to two months now. There was a lot that I focused on since Linux was probably the most low-level implementation of this WAV file reader. It had to introduce many particular structures to work the same way as Windows. The total implementation once Linux was finished was ~600 lines of code. This was the main chunk of code needed to get the base implementation down.

This is a very small glimpse into the WavReader class, but it shows perfectly what this class aims to do. You can how a lot of the code is condensed and built around making the WAV file reading as simple as possible. The code itself is fast and straightforward, and while I would enjoy simply using `constexpr` for a lot of what this class does, I sadly don't have access to that feature quite yet (however, when we update to C++17, I would like to make this class more optimized). So far, everything is looking good and the implementation across GSound and GMusic both show massive improvements in file size.

Now, we move to Windows. Windows has a lot of architecture built into it by default, so there are small improvements we can make if we are compiling for it. In particular, we can replace some of the data types we have to manually make in Linux.

As you can see, most of the code changes to WavReader from here on will simply modify basic data representation, with most of the functionality staying exactly the same. This allows us to focus more on getting WavReader properly hooked into all GSound and GMusic implementations with little confusion or frustration on how to get it done.

Same as before as well, we see a massive deduction in code once WavReader is implemented for both GSound and GMusic on Windows.

Lastly, UWP. I have just started trying to get its implementation working correctly. I had hoped that GFile and the previous data representation would work for it, however, GFile fails to work correctly for this type of use case. What has to be done now is for me to implement the low-level file handling that WinRT provides so I can read the WAV file data correctly. The reason for these issues, I suspect, is due to UWP using UTF-8/UTF-16 to read file data, making the generic GFile fail when attempting to read the files we need. I'm not sure if this is intended behavior for GFile, but I will be discussing it during the next meeting on Wednesday.

Overall, the UWP changes will be more extensive than the others but should be able to pass once I'm finished updating WavReader to support it. After that, the rest should be pretty straightforward. And with hope, this should be merged by Wednesday. For now, this is all that has been done.

Month 4 - Week 3 - Finalization of the Audio

 Last week we had discussed closely about the audio and the eventual finalization process that would take place to merge the large amount of changes that were done. This week, these changes have been made. The theory I mentioned last week was tested and sadly bared no fruit towards fixing the popping issue. So, instead of focusing more on attempting to fix the problem, I chose to move forward with finalizing the changes and merging those as it still provides fixes to over three issues on the board, each of which was much anticipated. We will quickly go over all three issues that were fixed.

Newer PulseAudio versions results in GMusic3D crashing (#255)

This issue was a tricky one. On the surface it looked to be a change to the library that altered the original functionality, however, this issue was much easier to fix than expected. Instead of internal changes, I inspected the error message that had been printing.

This assertion was hard to break down initially, but it made much more sense over time. The assertion was telling the developer that the thread's pool stat exceeded the allotted size of the stream's buffer. To make it even more simple to state, the stream's buffer reached its maximum allowed limit, and we were attempting to write more than what it could handle on the server side. Because of this deduction of the assertion, I inspected the code deeper and found we weren't polling at all for the server on how much data we were allowed to write. Because of this, I added the functionality, which was much easier than expected.

This little change fixed the issue entirely, removing any need to worry about GMusic failing. This also fixed PipeWire's distortion of music as well. This is primarily because PipeWire, instead of asserting for this situation, it will just overwrite data to fit the new data, which results in the audio sounding distorted or broken to the end user. If we poll like this, the audio sounds completely normal as we expect it to.

Pulseaudio holds onto memory after cleanup (Linux) (#17)

This is an old issue but related heavily to memory leaks that were present in the audio before I started my fixes. This issue was spotted by using Valgrind (a popular memory leak detection tool). It detected several memory leaks in the audio, which resulted in me stepping in with a keen interest in what could be causing it.

I fancy myself as a memory developer. I enjoy these kinds of issues because memory leaks are fun to tackle and understand. They are tricky, but that's why I like them in the end. Due to how sneaky they can be sometimes, I enjoy breaking them into sections and really approach how to fix them. However, this change was a little intense to fix the problem. A lot of the code internally was C++98, because of this, there was a lot of code that was old and not set to modern C++ standards. I decided the fix for this would be replacing the manual `new` and `delete` allocations with the more modern `unique_ptr` usage. This will handle the actual allocation while also managing the deallocation without us worrying about anything slipping through the cracks. The implementation was rather extensive but improved the overall code massively.

There are of course many more changes to the code, but this is just a brief glimpse into the changes made. Overall, I was happy with the results, mainly because of this; if the memory leak is still present, then it is internal to PulseAudio and not with the implementation code.

Pipewire support for PulseAudio is skipping, distorted, and sometimes fails working (#256)

Lastly, we have this behemoth.

There is not one change that fixed this issue, instead, it was a collection of different changes made to PulseAudio that improved the usage which eventually fixed this problem. The implementation in its current state is a 1:1 ratio, meaning that PulseAudio and PipeWire are acting the same. This is good because that means any outside issues are now fixed across both subsystems. Besides the popping on both subsystems, everything is working as expected now.

Final Notes

With everything done, there were over 1,600 lines of code changed. About 780 lines were added with 820 removed, that's a lot of changes. The review took a minute, there were over 103 commits to comb through and lots of changes to go through, but in the end, it was merged successfully. So, what now?

Well, there is one last change I'm doing for both Windows and Linux, a universal WAV file reader. This will lessen the amount of implementations that exist for reading WAV files. I'm mostly done with it now, just have to get the GMusic implementation to work correctly, and then we'll be golden to replace full implementations. I would provide screenshots for this, but I forgot to actually push these changes to GitLab, so they're still on my local Linux sadly, and I'm writing this blog post on my Windows. I will be providing the solution I have written up next week however once everything is fully completed, which should be by tomorrow (I would've had this done sooner, but personal life took away my ability to write it up quick enough).

Beyond that, the next issue will be the ghost window issue for Mac which will more than likely take all my focus for the rest of this month and next month. I'm hoping learning Objective-C won't be too painful, but I know XCode will have its quirks that I'll have to get used to.

Overall, this was everything done over the last month and a half, and I'm looking forward to tackling some new issues that are unrelated to audio, as, if I'm being honest, audio has been tiring. Doing a different issue would be like tasting a different ice cream flavor, and I'm looking forward to that, but I have a job to do, and I plan to at least do it right. With that, this is all I have to say for now. Till next week.

Month 4 - Week 2 - Health Issues and Finalizing the Audio Fix

 This merge request has been a beast, and sadly, the ending is not the one I would've preferred. However, a lot of time was spent on this, and a lot of effort was poured into it, so even though I couldn't get everything I wanted, the outcome is still substantially better than before.

Let's provide a little preface before we continue. My blog post this week will be very brief compared to my traditional blog posts. I've been dealing with some pesky health issues this week that have affected a lot of things for me, from food consumption to digestion issues to general body weakness, it's been a lot. To add onto this, it has also spurred a rather severe form of insomnia so my ability to work effectively has taken a massive toll as compared to normal. You can see this by the timestamps on my commits.

Safe to say, I'm in a bit of a rut right now, but with hope, I'll be back to normal soon and will be able to pour my best effort into continuing to fix bugs and get some of the issues present on the board taken down.

Now, onto the changes with this merge request, as the outcome of it has changed. Sadly, two issues will still be present after this issue closes. Over the last couple of weeks, I've been pouring in constant effort to attempt to fix this popping audio bug, however, any fix I provide doesn't get it. I've done many different solutions, from new drain flags to partitioning how much data is sent to the server, all the way to even corking the stream before flushing the buffer in an attempt to rid it of popping when the audio stops. Even with all of these changes, nothing seems to work, and the issue still persists. Even more frustrating is that the PipeWire support is affected by some of these edits, particularly the drain flag and corking. Both "fixes" result in the audio cutting off early or sounding more distorted for PipeWire, which leads me to believe that those solutions aren't correct and won't help the overall PipeWire/PulseAudio compatibility I'm seeking for the library.

So, with that, those changes have been reverted.

The last attempt I could throw is writing over what the polling write size gives for the server side of PulseAudio. This could fix it because this is exactly what GMusic does. Since GMusic relies on the streaming buffer size, PulseAudio has to support this buffer size on the server side. Something else to note is when I was making PulseAudio print out the write size it allowed initially, it supported only up to the sample rate. If you need to get more familiar with audio, the sample rate is how many samples make up a single second in audio. So PulseAudio was only allowing 1 second of audio to be written. This is a little problematic as writing just a second could cause skipping or more issues since the code to write to the stream itself could take longer than a second. GMusic doesn't suffer from this, however, since it writes the streaming buffer size, which is around ~65,000 bytes. This gives GMusic more than enough time to process and calculate what it needs to before it needs to write to the stream again. Pretty useful.

Possibly, as a theory, this is what GSound is suffering from. However, I won't be sure until I test it.

That will probably be my last attempt to fix this issue. By Monday, I will be finalizing the refactoring I've done, and the fixes I have made, before merging it completely. Even though I could fix the issue initially targeted, I was able to greatly modernize the code and bring it up to speed with C++11. I also improved much of the memory safety with the usages of `unique_ptr`, though this safety to performance won't be felt until C++17 for library developers, since `constexpr` is not introduced for `unique_ptr`s until C++14/17. Thankfully, those using the library can enjoy high performance and memory safety since this is a single header library, thus it will only compile when the user builds their application (which tends to default to C++14/17 now).

In total, there were over 93 commits, and almost 1,000 lines of code changed.

This took a lot of effort and focus to get all the changes implemented, and overall, I'm happy with the results. Though, as I mentioned, I would've liked to have seen this popping issue fixed. I have that last theory I can test, but if it doesn't fix the issue, it will have to sit unsolved for a little while longer.

While this blog post doesn't go over any problems I faced and acts more as an overview of the last month and a half of work I've done on this single interface, I feel it is worth writing all of this. Not just for me, but for others who may have to approach my code in the future. This way they know what I've tried and tested. It will help check off a lot of things from the list, and help them with diagnosing and coming up with solutions. I'm fine with that.

So what am I doing after this merge request? Well, I'm going to be focusing on implementing a universal WAV file reader, that incorporates a more C++-compliant structure to everything, as well as providing proper safety checks for everything. There were several safety checks that the old system didn't have.

There are also a lot of odd choices in terms of how the file is read for each implementation, particularly with GSound and GMusic. One will read all the data, and the other will read the file twice (if I'm not mistaken). Once for the header information, and another time to find the stream data. I find this odd and feel there could be a class for this, something that you can control to simply load all the data for you, or get a nice index/pointer to the data.

Besides that, the next issue to focus on after the quick implementation of the WAV file reader will be the long-awaited ghost windows that are plaguing Mac. I'm not sure how well I'll do tackling it, as I have no experience in Objective-C, but I should get the hang of everything relatively quick. At the end of the day, any C language compiles to the same assembly, so I should be okay.

With that, this is all I have to give for the blog post this week. I wish I could've done more, but my health has been in constant waves this entire week, so I'm having to balance everything lightly. I hope this provides some good insight into everything that has been done.