XAudio2: Add WMA support #46

JohanSmet · 2018-10-18T21:20:16Z

Third PR for issue #32. This adds xWMA support to FAudio using FFmpeg.

AFAIK there isn't a nice way to add optional features to a Visual Studio project, so the support here is a bit, uhm, DIY. The only alternative I see is to add another project and unload the unwanted FAudio project from the solution depending if you want FFmpeg support or not. But that gets unmanageable really fast. Moving to a project generator seems to get more and more interesting :-)

flibitijibibo · 2018-10-18T21:37:32Z

Project generation was on my mind at one point... I'm currently weighing options to get FAudio integrated into Wine and that would certainly be a major factor for this, so I'll get back to you on that. For now, VS isn't a big deal since this is basically for XnaToFna, Wine, and not much else.

CC @aeikum who is the original author of the ffmpeg patches, and has experience with this in Wine's XAudio2 built-in. We're both going to try and review this, since I can test the XNA stuff and he can test the Wine stuff (we definitely need to run this against the existing tests).

Also CC @0x0ade who needs this for XnaToFna. If you have time to try this out, let me know!

GloriousEggroll · 2018-10-18T22:05:35Z

I'm attempting to compile this via:
export FAUDIO_FFMPEG=1
export FAUDIO_FFMPEG_PREFIX=/usr
(ffmpeg is installed and includes are in /usr/include and libs are in /usr/lib)

source cpp/scripts/cross_compile_64
make

but I'm getting:

/usr/x86_64-w64-mingw32/include/crtdefs.h:75:44: error: conflicting types for ‘uintptr_t’
 __MINGW_EXTENSION typedef unsigned __int64 uintptr_t;
                                            ^~~~~~~~~
In file included from src/F3DAudio.h:36,
                 from src/F3DAudio.c:27:
/usr/include/stdint.h:90:27: note: previous declaration of ‘uintptr_t’ was here
 typedef unsigned long int uintptr_t;
                           ^~~~~~~~~
In file included from /usr/x86_64-w64-mingw32/include/SDL2/SDL_stdinc.h:34,
                 from src/FAudio_internal.h:77,
                 from src/F3DAudio.c:28:
/usr/include/sys/types.h:108:19: error: conflicting types for ‘ssize_t’
 typedef __ssize_t ssize_t;
                   ^~~~~~~
In file included from /usr/x86_64-w64-mingw32/include/stddef.h:7,
                 from /usr/lib/gcc/x86_64-w64-mingw32/8.2.0/include/stddef.h:1,
                 from src/FAudio.h:39,
                 from src/FAudio_internal.h:27,
                 from src/F3DAudio.c:28:
/usr/x86_64-w64-mingw32/include/crtdefs.h:45:35: note: previous declaration of ‘ssize_t’ was here
 __MINGW_EXTENSION typedef __int64 ssize_t;
                                   ^~~~~~~
In file included from /usr/include/sys/types.h:129,
                 from /usr/x86_64-w64-mingw32/include/SDL2/SDL_stdinc.h:34,
                 from src/FAudio_internal.h:77,
                 from src/F3DAudio.c:28:
/usr/include/bits/types/time_t.h:7:18: error: conflicting types for ‘time_t’
 typedef __time_t time_t;
                  ^~~~~~
In file included from /usr/x86_64-w64-mingw32/include/stddef.h:7,
                 from /usr/lib/gcc/x86_64-w64-mingw32/8.2.0/include/stddef.h:1,
                 from src/FAudio.h:39,
                 from src/FAudio_internal.h:27,
                 from src/F3DAudio.c:28:

and

/usr/x86_64-w64-mingw32/include/SDL2/SDL_stdinc.h:291:20: error: size of array ‘SDL_compile_time_assert_uint64’ is negative
        typedef int SDL_compile_time_assert_ ## name[(x) * 2 - 1]
                    ^~~~~~~~~~~~~~~~~~~~~~~~
/usr/x86_64-w64-mingw32/include/SDL2/SDL_stdinc.h:300:1: note: in expansion of macro ‘SDL_COMPILE_TIME_ASSERT’
 SDL_COMPILE_TIME_ASSERT(uint64, sizeof(Uint64) == 8);
 ^~~~~~~~~~~~~~~~~~~~~~~
/usr/x86_64-w64-mingw32/include/SDL2/SDL_stdinc.h:291:20: error: size of array ‘SDL_compile_time_assert_sint64’ is negative
        typedef int SDL_compile_time_assert_ ## name[(x) * 2 - 1]
                    ^~~~~~~~~~~~~~~~~~~~~~~~
/usr/x86_64-w64-mingw32/include/SDL2/SDL_stdinc.h:301:1: note: in expansion of macro ‘SDL_COMPILE_TIME_ASSERT’
 SDL_COMPILE_TIME_ASSERT(sint64, sizeof(Sint64) == 8);
 ^~~~~~~~~~~~~~~~~~~~~~~
make: *** [Makefile:91: src/F3DAudio.o] Error 1

compiling master works as intended.

flibitijibibo · 2018-10-18T22:34:02Z

The F3DAudio unit doesn’t change with this patch, so you’re looking at a different issue - this looks like stdint conflicts with crtbase, which I think isn’t us at all...?

JohanSmet · 2018-10-18T22:39:41Z

@GloriousEggroll: you're cross compiling, right? In that case, the FFmpeg binary you're using also needs to be cross compiled. I'm guessing yours is not because it would be weird for it to be installed to /usr.
(https://trac.ffmpeg.org/wiki/CompilationGuide/CrossCompilingForWindows has more information about this).

aeikum · 2018-10-19T14:10:18Z

I gave it a real brief review and the general idea looks fine to me. I remember it being important that the buffer stuff is tracked in terms of bytes, unlike with PCM data which is tracked in frames. That was what I was working on when I stopped, I don't know if that got fixed already.

flibitijibibo · 2018-10-19T19:29:52Z

utils/testxwma/testxwma.cpp

+	/* process command line arguments. This is just a test program, didn't go too nuts with validation. */
+	if (argc < 2 || (argc > 4 && argc != 7)) {
+		printf("Usage: %s filename [PlayBegin] [PlayLength] [LoopBegin LoopLength LoopCount]\n", argv[0]);
+		printf(" - filename (required): can be either a WAV or XMWA audio file.\n");


"xWMA" instead of "XMWA"

flibitijibibo · 2018-10-19T19:30:09Z

src/FAudio_internal.c

@@ -1277,3 +1277,4 @@ void FAudio_INTERNAL_DecodeStereoMSADPCM(

 	*samples = done;
 }
+


Extra newline...?

flibitijibibo · 2018-10-19T19:34:09Z

The only broad thing I see is spaces instead of tabs for some parts of the files. At the moment I'm trying to wire up FACT to test this with WaveBank data but it's giving me a hard time; here's the patch I threw together to test:

diff --git a/src/FACT.c b/src/FACT.c
index 6baa32b..cb36aa8 100644
--- a/src/FACT.c
+++ b/src/FACT.c
@@ -1374,6 +1374,7 @@ uint32_t FACTWaveBank_Prepare(
 	FACTWave **ppWave
 ) {
 	FAudioBuffer buffer;
+	FAudioBufferWMA bufferWMA;
 	FAudioVoiceSends sends;
 	FAudioSendDescriptor send;
 	FAudioADPCMWaveFormat format;
@@ -1452,8 +1453,6 @@ uint32_t FACTWaveBank_Prepare(
 		format.wfx.nBlockAlign = aWMABlockAlign[entry->Format.wBlockAlign & 0x1F];
 		format.wfx.wBitsPerSample = 16;
 		format.wfx.cbSize = 0;
-
-		FAudio_assert(0 && "Rebuild your WaveBanks with ADPCM!");
 	}
 	else /* Includes 0x1 - XMA */
 	{
@@ -1499,6 +1498,11 @@ uint32_t FACTWaveBank_Prepare(
 				format.wfx.nBlockAlign
 			);
 		}
+		else
+		{
+			/* TODO: WMA streaming support */
+			FAudio_assert(0 && "Rebuild your WaveBanks with ADPCM!");
+		}
 		(*ppWave)->streamCache = (uint8_t*) FAudio_malloc((*ppWave)->streamSize);
 		(*ppWave)->streamOffset = entry->PlayRegion.dwOffset;
 
@@ -1534,7 +1538,26 @@ uint32_t FACTWaveBank_Prepare(
 			buffer.LoopCount = nLoopCount;
 		}
 		buffer.pContext = NULL;
-		FAudioSourceVoice_SubmitSourceBuffer((*ppWave)->voice, &buffer, NULL);
+		if (format.wfx.wFormatTag == FAUDIO_FORMAT_WMAUDIO2)
+		{
+			bufferWMA.pDecodedPacketCumulativeBytes =
+				pWaveBank->seekTables[nWaveIndex].entries;
+			bufferWMA.PacketCount =
+				pWaveBank->seekTables[nWaveIndex].entryCount;
+			FAudioSourceVoice_SubmitSourceBuffer(
+				(*ppWave)->voice,
+				&buffer,
+				&bufferWMA
+			);
+		}
+		else
+		{
+			FAudioSourceVoice_SubmitSourceBuffer(
+				(*ppWave)->voice,
+				&buffer,
+				NULL
+			);
+		}
 	}
 
 	/* Add to the WaveBank Wave list */

Error message is [wmav2 @ 0xb3e2c0] next_block_len_bits 6 out of range when testing with the Murder Miners WaveBank, haven't tried any others yet.

flibitijibibo · 2018-10-19T19:57:18Z

Updated the test patch above after the shocking revelation that the Duration value in the WaveBank represents the duration of the wave:

2fdfc19

JohanSmet · 2018-10-19T20:18:46Z

@aeikum Yes, but most of those changes were merged in a previous PR (#45). The decoder stores stores the current position in the encoded stream in bytes (https://github.com/JohanSmet/FACT/blob/8864fcae768b323704e1d81439e91500437a3120/src/FAudio_ffmpeg.c#L17).

@flibitijibibo Those style errors should be fixed. I was just looking at FACT wavebanks when I saw your review remarks. I'll try out your patch shortly.

GloriousEggroll · 2018-10-20T06:41:23Z

Happy to report Voices in Skyrim SE now working as they should, and Warframe x64 bit no longer crashing in the usual mission loading locations due to wma decoding issues. Used winetricks xact (modified winetricks script to enable 64 bit xaudio dlls.), then ran the wine_setup_native afterwards (removed x3daudio section as instructed via email).

flibitijibibo · 2018-10-22T19:26:45Z

Just a quick heads up that CustomAllocatorEXT just got pushed to master:

c821784

Thankfully this doesn't appear to have introduced a conflict, so I think this can be rebased and fixed to use audio->pMalloc and friends without any trouble.

JohanSmet · 2018-10-22T19:55:39Z

@flibitijibibo Your FACT patch looks good. Tested with the Murder Miners WaveBank, everything plays fine except the entries with sample rate 32000 and (coded) block align 194.

Checked with MS XACT3 and it passes the same parameters as FACT. So that's looking good.

Unfortunately when I extract a sample from the wavebank and run it through ffmpeg (or vlc for that matter) it gives the same error messages. So it would seem there's something about these files the FFmpeg WMA decoder does not like. (To verify the extracted file I tried it with xWMAEncode from the DirectX SDK and that decodes it without issues.)

…y piece

…eam.

flibitijibibo · 2018-10-22T20:07:59Z

Oh, neat! I think I was testing the 32KHz samples, so a really dumb coincidence on my part. If you want you can throw that patch in and do a rebase, and once it's verified as working with HEAD I'll merge this in.

JohanSmet · 2018-10-22T20:23:36Z

Ok, the FACT patch and updates for CustomAllocatorEXT are in.

flibitijibibo · 2018-10-22T20:26:01Z

src/FAudio.c

+				i = FAudio_FFMPEG_init(*ppSourceVoice);	
+				if (i != 0)
+				{
+					FAudio_free((*ppSourceVoice)->src.format);


audio->pFree for these two. Other than that, new changes LGTM!

Sorry, didn't see those. Fixed!

flibitijibibo · 2018-10-22T20:45:10Z

Just tried the patch and sure enough it works. I guess we'll need to talk to the FFmpeg developers about 32KHz support?

In any case I'll merge this, thanks for digging into it so quickly! Never thought I'd see the day where FNA could work with WMA samples.

flibitijibibo reviewed Oct 19, 2018

View reviewed changes

aeikum and others added 17 commits October 22, 2018 21:56

ffmpeg

6cf9ca4

Move Andrew's work to a separate file and get it to compile.

c6d93a9

Planar float format's are also okay

cafe716

Buffer the WMA packet decoded by FFmpeg and give it to FAudio piece b…

7850df3

…y piece

SubmitSourceBuffer: require pBufferWMA for WMA formatted voices

2c620aa

WAVEFORMATEXTENSIBLE is used by games to create WMAUDIO2 voices

544dd70

Move FFmpeg variables from FAudioVoice to private struct

7017084

Don't use voice->src.curBufferOffset to store position in encoded str…

919732d

…eam.

Free FFmpeg resources when destroying a SourceVoice

d5e0b24

Save pBufferWMA, if supplied, so it can be usec by the decoder.

9e01b23

Support for PlayBegin/PlayLength

8bed355

Test utility to play xWMA files with FAudio

ae5ac60

Refactoring and cleanup ffmpeg support

2b0f8d5

Copy ffmpeg dlls when cross compiling

a3a7e41

Fix empty translation unit warning when compiling utils

365b6db

Add small README to explain FFmpeg support

16369a1

Style fixes after code review

4f0d25c

JohanSmet added 2 commits October 22, 2018 22:08

Changes for CustomAllocatorEXT

51b47e8

Forgot to free two buffers used by the FFmpeg decoder

d80b9e4

JohanSmet force-pushed the ffmpeg branch from 8864fca to d80b9e4 Compare October 22, 2018 20:09

FACT: support for WMAUDIO2 in non-streaming wavebanks

7ed02c7

flibitijibibo reviewed Oct 22, 2018

View reviewed changes

Two more FAudio_free calls needed replacing due to CustomAllocatorEXT

b8ac31f

flibitijibibo merged commit 68fa906 into FNA-XNA:master Oct 22, 2018

flibitijibibo mentioned this pull request Oct 22, 2018

Add optional support for WMA via FFmpeg #32

Closed

JohanSmet deleted the ffmpeg branch November 12, 2018 12:12

arno01 mentioned this pull request Dec 31, 2018

Assassins Creed Origins (582160) ValveSoftware/Proton#928

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

XAudio2: Add WMA support #46

XAudio2: Add WMA support #46

JohanSmet commented Oct 18, 2018

flibitijibibo commented Oct 18, 2018

GloriousEggroll commented Oct 18, 2018 •

edited

Loading

flibitijibibo commented Oct 18, 2018

JohanSmet commented Oct 18, 2018

aeikum commented Oct 19, 2018

flibitijibibo Oct 19, 2018

flibitijibibo Oct 19, 2018

flibitijibibo commented Oct 19, 2018 •

edited

Loading

flibitijibibo commented Oct 19, 2018

JohanSmet commented Oct 19, 2018

GloriousEggroll commented Oct 20, 2018

flibitijibibo commented Oct 22, 2018

JohanSmet commented Oct 22, 2018

flibitijibibo commented Oct 22, 2018

JohanSmet commented Oct 22, 2018

flibitijibibo Oct 22, 2018

JohanSmet Oct 22, 2018

flibitijibibo commented Oct 22, 2018

		@@ -1277,3 +1277,4 @@ void FAudio_INTERNAL_DecodeStereoMSADPCM(

		*samples = done;
		}

XAudio2: Add WMA support #46

XAudio2: Add WMA support #46

Conversation

JohanSmet commented Oct 18, 2018

flibitijibibo commented Oct 18, 2018

GloriousEggroll commented Oct 18, 2018 • edited Loading

flibitijibibo commented Oct 18, 2018

JohanSmet commented Oct 18, 2018

aeikum commented Oct 19, 2018

flibitijibibo Oct 19, 2018

Choose a reason for hiding this comment

flibitijibibo Oct 19, 2018

Choose a reason for hiding this comment

flibitijibibo commented Oct 19, 2018 • edited Loading

flibitijibibo commented Oct 19, 2018

JohanSmet commented Oct 19, 2018

GloriousEggroll commented Oct 20, 2018

flibitijibibo commented Oct 22, 2018

JohanSmet commented Oct 22, 2018

flibitijibibo commented Oct 22, 2018

JohanSmet commented Oct 22, 2018

flibitijibibo Oct 22, 2018

Choose a reason for hiding this comment

JohanSmet Oct 22, 2018

Choose a reason for hiding this comment

flibitijibibo commented Oct 22, 2018

GloriousEggroll commented Oct 18, 2018 •

edited

Loading

flibitijibibo commented Oct 19, 2018 •

edited

Loading