search

flibitijibibo / flibitBounties

Pile of programming bounties for things flibit can't do right now
27 stars 0 forks source link

Implement XAudio2 Wine DLLs with FAudio #4

Closed flibitijibibo closed 6 years ago

flibitijibibo commented 6 years ago

Introductory Information:

XAudio2 is Microsoft's low-level audio library made primarily for game developers. XAudio2 is split up in to multiple parts, including XAudio2 itself (confusingly), X3DAudio, and XAPOFX.

FAudio is FNA's upcoming replacement for the Audio subsystem. It was originally designed as just an XACT reimplementation, but was eventually expanded to act as an DirectX Audio implementation with support for XAudio2, X3DAudio, and XACT3. The repository is currently here.

The Project:

We need to take FAudio and create a series of Wine DLLs that reimplement the XAudio2 2.7 set of libraries. This will most likely be done by taking the existing framework in the Wine source and filling them in to use FAudio, removing any of CodeWeavers' existing implementation. The resulting source will be kept in the FAudio source tree as a separate wine/ folder. Naturally this will only be used for Wine hacking, most notably because it will be released under the LGPL, unlike FAudio itself which is released under the zlib license.

For this bounty we only care about XAudio2 and X3DAudio; XACT will not be a primary focus here and it's unlikely we'll ever look at XAPOFX.

A large part of portability research is seeing how compatible our work is with existing Windows code. In addition to having a much larger pool of data to test with, it also makes pitching ports a whole lot easier; many XNA Linux/macOS ports can be directly traced back to having FNA prototypes already built by the time we contacted the developer. With FAudio now being its own separate entity, it's now possible for us to try and do the same for XAudio2, which has often been a point of contention for porting away from Windows, Xbox 360, and Xbox One. Similar projects include XnaToFna, our relinker for testing XNA games with FNA, and DXVK, a Direct3D11 reimplementation using Vulkan.

Prerequisites:

C/C++ knowledge is a definite requirement, and being familiar with COM will probably help too. 99% of the job is just wrapping this weird COM interface around FAudio, which is otherwise 1:1 mapped to the XAudio2 APIs.

Example Games:

Pretty much every Windows game from the Xbox 360 era onward that doesn't use a middleware like FMOD/Miles is using this. The catch, however, is that many of those games use xWMA or XMA, two formats FAudio does not support. You'll have to find games that only push PCM/ADPCM data. Possible candidates include Jet Set Radio, Sonic CD, Sonic Mania, Sonic Adventure 1/2, PAC-MAN Championship Edition DX+, Super Meat Boy, Braid, and Wwise games on Windows. Games known to use unsupported formats include Castle Crashers, BattleBlock Theater, and Pit People.

How Much Can flibit Help?

I can help with everything except the COM part. I know the APIs very thoroughly at this point so if there's some weird legacy version compatibility issue I can take a look at that on my end.

Budget/Timeline:

Measuring in weekends, I expect this to take about 1 weekend. Most of the time will be spent on wrapping your head around the Wine DLLs, then the rest is just plugging in the FAudio API calls which should be 1:1 with XAudio2's calls. I currently have $500 USD allocated for this project.

flibitijibibo commented 6 years ago

Quick update: I stubbed in the XAPO and XAudio2FX stuff in FAudio, so now it should be possible to fill out the XAudio2 DLL in its entirety. XAPOFX is still off the table since that's a whole other can of worms that we hopefully won't have to endure.

JohanSmet commented 6 years ago

I've started working on this issue. I have some experience with COM (in C++ and Delphi) on Windows.

Right now I've got a C++ and COM wrapper almost working on Windows, there is some corruption/distortion in the output. I started developing on Windows so I could test against the reference XAudio2 implementation without confusing problems in the wrapper (or FAudio) with Wine-related issues. I've glanced at the Wine COM-implementation and it looks similar to the Windows stuff. Porting it over should, hopefully, be straightforward.

Is it a problem that I'm doing the implementation in C++ and not in straight C? Using C++ seems more natural for a COM-interface and this way has the side effect of also creating C++ bindings. But I don't mind switching over to C if you prefer that.

I found some issues with FAudio itself while testing this, mostly trivial things. I'll create a PR for some of these fixes later today.

Current state/todo:

So far I've tested with a small demo application and three games:

flibitijibibo commented 6 years ago

Oh wow, didn’t realize stuff would be up so quickly!

Since this is just an experimental DLL and the dev experience isn’t excessively important, I’m fine with C++. Whatever gets us past the COM goo...

And definitely patch whatever you like in FAudio, I’m sure there are bugs like crazy outside of our little XNA world (ignoring the obvious “flibit was very bad and didn’t use atomics/mutexes anywhere” bit).

flibitijibibo commented 6 years ago

Oh yeah and this is a totally stupid thing, but does audio distortion get fixed if you set the period size to 528?

https://github.com/flibitijibibo/FACT/blob/master/src/FAudio_platform_sdl2.c#L139

Got a couple reports from the Xbox team that this was needed due to a WASAPI bug in SDL, haven’t fully investigated yet.

JohanSmet commented 6 years ago

I've created a PR for the things I can do myself. But I've found one thing that I can't fix myself (without spending quite some time looking at the decoder functions):

MSDN specifies when the PlayLength and the LoopLength fields of XAUDIO2_BUFFER are zero it means play/loop the entire buffer. But without an explicit length FAudio either crashes or doesn't play anything depending on the source format. I tried working around this by filling in the length myself. This works fine for PCM and Float formats but I'm unsure how to calculate this value for an ADPCM buffer. I looked briefly into handling this in the decoder functions but quickly decided to back out :-)

This might also be the cause of the bad audio in SuperMeatBoy because that uses ADPCM samples. JetSetRadio uses plain PCM and sounds fine (until it totally breaks). Changing the period size to 528 didn't help, unfortunately.

flibitijibibo commented 6 years ago

Oh, that’s an easier bug than it looks! You just want this guy here:

https://github.com/flibitijibibo/FACT/blob/master/src/FAudio_internal.c#L155

Strangely I have looping working fine but my test cases are FACT-based, not pure XAudio2.

flibitijibibo commented 6 years ago

Starting to see the real problem - what I need to do is set PlayLength to be a real value 100% of the time. What we can do is add a line to SubmitSourceBuffer...

https://github.com/flibitijibibo/FACT/blob/master/src/FAudio.c#L1169

... that checks PlayLength == 0 and uses the WaveFormatEx to determine the sample count using the buffer's byte length. This should fix the bug and also gives us a very good reason to validate the Play/Loop offset/length values at submit time!

flibitijibibo commented 6 years ago

(Sorry for the triple post)

Sample of what I mean regarding PlayLength calculation and validation - for ADPCM, the CreateSourceVoice format should have a value like this:

https://github.com/flibitijibibo/FACT/blob/master/src/FACT.c#L1136

So when the Play values == 0 we can use this along with the buffer size in bytes to get the correct length.

JohanSmet commented 6 years ago

Thanks for the pointers! After I fix the current PR later tonight, I'll work on a patch to add the necessary calculations to SubmitSourceBuffer.

It didn't fix the corruption with SMB but it did point me in the right direction. The buffers it submits aren't an exact multiple of the ADPCM block size. So you end up with part of an ADPCM block at the end of one buffer and the remainder at the beginning of the next. This trips up the ADPCM-decoder and it sounds horrible until a block happens to start at the beginning of a new buffer and it sounds ok for it bit ...

I did a quick test in the C++ wrapper to make sure the buffers are always multiples of the block size (leaking memory like crazy) and then it sounds perfectly fine.

I think I'm going to focus on the other interfaces first before I dig any further into this, if that's ok with you?

flibitijibibo commented 6 years ago

Definitely focus on the interfaces for now, the block size validation is an FAudio bug on my end so I can fix that up today or tomorrow. Good to know that the size can actually be wrong, so I guess we have to do a mod on it or something...

flibitijibibo commented 6 years ago

Buffer region validation is in:

https://github.com/flibitijibibo/FACT/commit/19baff46b22990b67e688c187a374d6c3eed0a7a

JohanSmet commented 6 years ago

The size isn't really wrong, I guess. It's just not a multiple of the block size but that seems to be allowed. You'll have to somehow carry over the partial block from the end of one buffer to the beginning of the next ... Or keep extra state for the decoder so it can continue decoding on the next buffer?

Edit: Oops, I didn't see your last comment before I wrote this ... I'll pull in those changes and test again :-)

flibitijibibo commented 6 years ago

Might have stepped on this ever so slightly with some changes for multichannel support:

https://github.com/flibitijibibo/FACT/compare/3e637aa...c1d1293

I don't think it'll affect the wrapper but if you have any more bugfixes this might have stomped on that. However, if I did this right it'll fix any attempts games make to use multichannel sources/submixes, which is probably more common than I'm thinking.

JohanSmet commented 6 years ago

No, no other bugfixes in the pipeline ATM. I've mainly focused on the other API's. I've completed the first pass on all of them. I haven't tested them yet, though. X3DAudio is just two functions so that should be doable (probably best to wait for https://github.com/flibitijibibo/flibitBounties/issues/2 to be merged first) but testing the XAPO/XAudio2FX wrappers seems a bit more challenging ...

On the plus side, working on XAPO made me realize that it's a bad idea to pass the voice effect chain objects unchanged from XAudio2 to FAudio. Another one for the todo-list 😀

Your buffer region validation fixes handle the PlayLength==0 cases perfectly in the games I've tested. I've created a small program to reproduce the problem SuperMeatBoy has with the ADPCM-decoder, code is here: https://gist.github.com/JohanSmet/706a66560133642144498bc50b4737d3. All you need is an ADPCM .wav file. The annoying thing is that XAudio2.8+ returns an error when AudioBytes isn't a multiple of the blocksize but earlier versions accept it and decode the stream without apparent issues.

I'll rebase to master and see if anything breaks :-) Next I'll move on to testing with Wine before I start adding the missing features to the XAudio2-wrapper.

flibitijibibo commented 6 years ago

Here's a possible fix, but it assumes Tommy used AudioBytes to increment his stream offset...

https://github.com/flibitijibibo/FACT/commit/1d0e4becb15b1a05b42431d30b0bd5473d7b1e02

This fixes the test program, at least! The alternative is to change the decoders slightly to request a byte size, and have the buffer list read/updated at that point rather than trying to read each buffer in separate loop iterations... which is about as fun as it sounds.

JohanSmet commented 6 years ago

Sorry, it doesn't fix SuperMeatBoy :-( But nevertheless, that was a cool solution!

Yeah, you're right, that doesn't sound fun. My first thought was trying to insert a one block temporary buffer between the two existing buffers. And somehow make the decoder ignore the first part of the second buffer. But then you'd also have issues with the callbacks etc ...

flibitijibibo commented 6 years ago

Came up with an awful idea: What happens when you take one of Meat Boy’s misaligned buffer examples and add a LoopCount to it in XAudio2? Does it get corrupted like us or does it throw an error? (Can’t check on my end as my current Windows setup doesn’t have the DXSDK)

EDIT: Also just dealt with the last of the bad asserts in SetOutputVoices, maybe PAC-MAN CEDX+ works now...?

JohanSmet commented 6 years ago

XAudio2 accepts the buffer with the LoopCount set and plays it back corrupted like FAudio does. PlayBegin/Length and LoopBegin/Length were left unchanged (0).

Nice, I'll test PAC-MAN later today.

flibitijibibo commented 6 years ago

So that pretty much confirms it then - 2.7 and lower will read buffer lists as one big raw byte stream, and then they decode what I guess is a temp buffer that is then interpreted as the input wave format. Going to take a wild guess and say 2.8+ are slightly more efficient when processing graphs! Unless we can edit the number value directly in the executable then I guess we have to put Meat Boy to rest for now ;_;

Not a big deal though, I'm sure this will only be used for Windows-only games anyhow!

JohanSmet commented 6 years ago

Quick update on PAC-MAN: it uses some features that weren't tested yet and exposed a few bugs in the wrapper. I had to fix the handling of sendlist in the wrapper and have temporary nulled out the effectchain parameters. With your SetOutputVoices work the game runs with decent sound until the first pellet is eaten. Then it tries to use an output filter and it's game over :-(

flibitijibibo commented 6 years ago

Well, shoot... guess we'll have to fill that in some day. But I guess we can just ignore that assert for now. The audio graph for CEDX+ must be quite strange!

flibitijibibo commented 6 years ago

Forgot to do weekly updates! Was working on Switch stuff this past week so I lost track, but I can happily say FAudio works on Switch now, including your recent filter work!

Anything new to report?

JohanSmet commented 6 years ago

That's awesome to hear :-)

Here's what happened in the last few days:

Todo:

I also have a question about Wine support. Is cross-compiling Windows DLLs and using them as "native" DLLS with Wine ok? You avoid a build dependency on Wine (but need mingw-w64 or msvc) and it seems easier to deploy on a per wineprefix basis (e.g. you can have a prefix with native xaudio2 and another with faudio to compare 'on-the-fly'). If not, I'll start looking into building native linux dynamic libraries to replace the Wine ones. FWIW, DXVK also uses the cross compilation method.

flibitijibibo commented 6 years ago

I ran into the infinite queue loop myself while working on this Switch stuff, and the only thread we have is the device thread! So I guess I really do have to implement thread safety stuff... I'll try to do this either today or tomorrow specifically for the buffer list.

Cross-compiling is fine with me, having the fake DLLs isn't too important since it would just skip Wine's WASAPI/DirectSound layers, which probably isn't introducing that much pain.

flibitijibibo commented 6 years ago

Still hopelessly stuck on Switch stuff so I just added some locks to bufferList modifications for now:

https://github.com/flibitijibibo/FACT/commit/ccf00e6621205003db08fbcec5e1712c0dc8ffb2

So now there shouldn't be conflicts between the application and audio threads... hoping this will fix JSR.

JohanSmet commented 6 years ago

I tested with JSR and it still behaves the same. Somehow SubmitSourceBuffers ends up with a buffer which has its next pointer pointing at itself. I don't understand, all calls from JSR seem to come from the same thread. I'll continue looking into this later.

I was planning on doing an update post today but I've spend all my available time this evening on playing (ahm, testing!) Capsized, and trying to understand JSR. I'll provide a status update tomorrow.

JohanSmet commented 6 years ago

Ok, I finally figured out what was going on with JSR. JSR plays a sound when you change the selected menu item. Reproducing the problem was as easy as changing the current menu item again while the previous sound was still playing. JSR then stops the voice, flushes the buffers, starts the voice again, and submits the new buffer.

FAudioSourceVoice_FlushSourceBuffers removes all buffers when the voice isn't active but does not reset voice->src.bufferList to NULL so it's still pointing to a buffer that was just freed. And apparently the malloc in FAudioSourceVoice_SubmitSourceBuffer reuses recently freed memory and returns a pointer to the same memory location resulting in a cycle in src.bufferList.

JSR now runs and sounds fine :-) I'll create a PR for this fix.

JohanSmet commented 6 years ago

Here's what happened since the last update:

I feel like this is getting close to being finished. Still to do:

flibitijibibo commented 6 years ago

Looks like that WaveFormatEx assumption is biting me harder than I thought it would... we need to do this for ADPCM as well, technically (though XAudio2 ignores most of the ADPCM struct data, including the coefficient tables). What we can do to pretty this up is check for 0xFFFE and then try to compare the KSDATAFORMAT GUIDs and set the format from that:

int realFormat;
if (pSourceFormat->wFormatTag >= 1 && pSourceFormat->wFormatTag <= 3)
{
    /* Plain ol' WaveFormatEx */
    realFormat = pSourceFormat->wFormatTag;
}
else if (pSourceFormat->wFormatTag == 0xFFFE)
{
    /* WaveFormatExtensible, match GUID */
    wfx = (FAudioWaveFormatExtensible*) pSourceFormat;
    #define COMPARE_GUID(guid, realFmt) \
        if (FAudio_memcmp(wfx->SubFormat, KSDATAFORMAT_SUBTYPE_##guid) == 0) \
        { \
            realFormat = realFmt; \
        }
    COMPARE_GUID(PCM, 1)
    else COMPARE_GUID(ADPCM, 2)
    else COMPARE_GUID(IEEE_FLOAT, 3)
    else
    {
        FAudio_assert(0 && "Unsupported WAVEFORMATEXTENSIBLE subtype!");
    }
    #undef COMPARE_GUID
}

/* Then do the same check we're doing now but with realFormat */
JohanSmet commented 6 years ago

I cleaned up the WaveFormatExtensible-checks and just submitted a PR for that.

The crashing in GTAV turned to be unrelated to FAudio, it was also crashing without FAudio. Reinstalled GTA and it's working fine now.

I've changed to code to more closely follow your style and extended the documentation with building and usage information.

I think we've come to the point that it's ready to evaluated to be merged into master. How do you want to handle this? ATM I've got about 40 commits in my branch. Do I create a PR with these squashed into one commit?

flibitijibibo commented 6 years ago

For now we can just use your current branch as the pull request and when I merge it I can automatically squash it down to one if needed. Since this is a whole new system I'll probably preserve the log in case we need it.

JohanSmet commented 6 years ago

Okay, I've created the PR.

flibitijibibo commented 6 years ago

This has been completed by https://github.com/flibitijibibo/FACT/pull/11 !