Monogame: Replace NVorbis with SDL Mixer

[harry-cpp]

I am gonna replace NVorbis with SDL Mixer so it's easier to port to .net core down the line. Does anyone have any objections (song content pipeline will stay the same)?

CC. @tomspilman @dellis1972 @KonajuGames @mrhelmut

[tomspilman]

Some questions first.

What platforms will this affect?

Is SDL Mixer part of our existing SDL or does it require extra native binaries?

Bigger topic... should we try to standardize on Vorbis for music on all platforms? Or is there a good benefit to continue to support platform specific formats/decoders/players for music?

[harry-cpp]

What platforms will this affect?

DesktopGL.

Is SDL Mixer part of our existing SDL or does it require extra native binaries?

Extra binary required.

[tomspilman]

DesktopGL.

I don't have a big issue with this if it is DesktopGL only for now.

The extra binary sucks... yet another 32bit/64bit mess to deal with.

@dellis1972 @KonajuGames ???

[dellis1972]

I'm good with this (as long as it works ;P )

On 9 March 2017 at 17:31, Tom Spilman [e-mail] wrote:

DesktopGL.

I don't have a big issue with this if it is DesktopGL only for now.

The extra binary sucks... yet another 32bit/64bit mess to deal with.

@dellis1972 https://github.com/dellis1972 @KonajuGames
https://github.com/KonajuGames ???

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/MonoGame/MonoGame/issues/5562#issuecomment-285421667,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAxeeatJrQhfx5T8VycXqHspTXk7Emwkks5rkDdtgaJpZM4MYVjJ
.

[dellis1972]

@cra0zy next task after this (if you are up for it) video support on
deksktopgl? ;)

On 9 March 2017 at 17:33, Dean Ellis [e-mail] wrote:

I'm good with this (as long as it works ;P )

On 9 March 2017 at 17:31, Tom Spilman [e-mail] wrote:

DesktopGL.

I don't have a big issue with this if it is DesktopGL only for now.

The extra binary sucks... yet another 32bit/64bit mess to deal with.

@dellis1972 https://github.com/dellis1972 @KonajuGames
https://github.com/KonajuGames ???

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/MonoGame/MonoGame/issues/5562#issuecomment-285421667,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAxeeatJrQhfx5T8VycXqHspTXk7Emwkks5rkDdtgaJpZM4MYVjJ
.

[Jjagg]

I saw something about @flibitijibibo getting proper video support in FNA. Maybe we can do something similar.

[harry-cpp]

I don't think I'll try implementing video support any time soon, if one of you wants to do it just add a comment to #3983 so no duplication of work occurs.

[flibitijibibo]

FWIW, FNA just uses a DynamicSoundEffectInstance with Vorbisfile#:

https://github.com/FNA-XNA/FNA/blob/master/src/Media/Xiph/Song.cs

This is getting replaced with FACT though; along with replacing the XACT runtime I'm also replacing the XNA Sound implementation and folding a Song implementation into it, using stb_vorbis. So this is really only helpful if you want to tie Song into your own DynamicSoundEffectInstance, which has a couple side effects (MasterVolume being the nastiest one).

The VideoPlayer stuff is separate, since TheoraPlay (and soon Theorafile) pull Vorbis data separately from Vorbisfile. If you grab anything from FNA VideoPlayer, wait until I replace the current version; TheoraPlay _loves_ eating up RAM, on top of spinning a pretty expensive thread. (It's kind of a miracle that it works, but it does!)

[CartBlanche]

@all A friendly reminder to please keep in mind that implementations in MG need to be done without looking at other people's implementations, where ever possible. This will avoid any potential legal ramifications later on. We faced this very dilemma in the early days when we received several submissions from contributors who had decompiled XNA, then submitted those as PRs. MG can't afford to be put in that sort of legal situation. Please refer to our contributing guidelines (particularly the 1st 3 points) - https://github.com/MonoGame/MonoGame/blob/develop/CONTRIBUTING.md#quick-guidelines

[harry-cpp]

I found a negative side, I will need to add duration to the content pipeline file since SDL_Mixer can't give me that info.

[harry-cpp]

Nevermind, we are already tracking duration... no negative side...

[KonajuGames]

Bigger topic... should we try to standardize on Vorbis for music on all platforms? Or is there a good benefit to continue to support platform specific formats/decoders/players for music?

Mobile and console platforms don't support Vorbis playback in hardware, so we'd be foregoing the hardware streaming support for a purely software solution, i.e. higher CPU usage and consequent increased power draw.

@crazy, what are the issues with .NET Core and NVorbis? I don't know what we're trying to fix here. I agree with Tom that I'm not in favour of yet another binary dependency (because we _never_ have issues with binary dependencies, right?).

[tomspilman]

Another consideration here.

Ideally whatever streaming audio solution we have would work via DynamicSoundEffect. This could allow us to more easily support both Song as well as streaming sounds in Xact.

Would we be better off with just a fast vorbis decoding solution?

[harry-cpp]

You missed a 0 :P

Anyway long story short, it's either use SDL Mixer, remove a lot of code as a dependency and add few extra binary dependencies (not a single C# lib), or make a fork of NVorbis, fix it to work with .Net Core, fix OggStream to work with .NetCore and maintain that code... I like the first option.

[KonajuGames]

How much code change is required to get them to work with .NET Core? Remember that the extra binary dependencies is a real issue for us. We've had so many issues raised here and on the forums about SDL libraries and FreeType libraries and other binary dependencies, it wouldn't be far off the biggest set of issues we've had.

I used NVorbis with a DynamicSoundEffectInstance in MediaPlayer when porting TY4 to Steam a few years ago. It's not difficult.

[harry-cpp]

Just so you guys are aware the PR for using SDL_Mixer is already up: #5566

Remember that the extra binary dependencies is a real issue for us. We've had so many issues raised here and on the forums about SDL libraries and FreeType libraries and other binary dependencies, it wouldn't be far off the biggest set of issues we've had.

The only reason SDL binary deps are an issue is because you guys didn't mention the change anywhere, and didn't autoinclude the libs in the NuGet... They work perfectly fine when the exist in the project :/

FreeType libs are a separate issue.

[tomspilman]

@cra0zy - Let's give this a little time to discuss and work out. It seems to be getting a little heated and no one wants that.

I'm fine with another binary dependency if it solves some important things in the process.

Using native code with vorbis decoding solves some performance concerns people have brought up in the past. I still think that the existing C# based decoder could be optimized a bunch, but I don't have the time myself and I can't be sure how close to native decoding speeds we could get. SDL_Mixer would certainly decode and playback faster than our current solution.

Also knowing we have to solve streaming audio support within XAct at some point, it would be better if we had vorbis decoding which can be used with DynamicSoundEffectInstance. SDL_Mixer would not allow for that (right?) meaning we would have to solve the issue again later.

[harry-cpp]

@cra0zy - Let's give this a little time to discuss and work out. It seems to be getting a little heated and no one wants that.

I'm fine with that.

[mrhelmut]

I'm fine with this move, NVorbis and OggStream are quite big and having something smaller to maintain (and quite possibly more reliable) and based on dynamic sound instances is a win to me.

By the way, there's a bug in the current OggStream: it triggers the next song event 1 buffer too early (which is 44100 samples long, so songs end about 1 second too soon if you loop it or queue multiple songs). I never noticed it because our songs are non looping.

[KonajuGames]

I'm fairly sure that's not a bug with OggStream itself because I have used it previously in a MonoGame DirectX title with DynamicSoundInstance to provide seamless looping music.

[harry-cpp]

I am gonna close this, there are a bunch of errors that NVorbis side created but nothing I couldn't fix... anyway I'll submit an updated version of NVotbis instead of submodule.

[harry-cpp]

I stand corrected, a lot of problems do get created by replacing Thread with Task... it seemed to work properly at first but there is definitely stuttering and way too much CPU use compared to SDL Mixer.

[rds1983]

Hi everyone,
I wanted to say that today I've started to port stb_vorbis.c to C# as part of the StbSharp project. Probably it would take a week to finish it. Then probably another week to test it.
If it'll work as expected then there will be pure C# vorbis decoding solution. That'll also work under .NET Core(right now, there is already StbSharp version that is .NET Standard 1.1 library)

[harry-cpp]

We are currently using NVorbis, which is a pure C# port of vorbis... the problem is that it works horrible under .Net Core (mostly since Threads can't be used).

[mrhelmut]

Is there any progress there?

I have a simple fix for NVorbis which is pending and doesn't seem to move: https://github.com/ioctlLR/NVorbis/pull/11
I would like this fix to be merged before the switch to SDL Mixer, does anybody know who to ping to get it merged and update the submodule head?

[mrhelmut]

Does anybody have contact with NVorbis maintainers? It is starting to be problematic since simple fixes aren't merged for months.

Would forking it and include fixes be ok while replacing it is being worked on? (I'm not fan of this solution myself, but well...)

[Jjagg]

He/she hasn't been active for almost a year. IMO switching to SDLMixer or @rds1983's stb_vorbis wrapper is preferable to forking.

[mrhelmut]

What concerns me is the time frame, I don't quite like NVorbis bugs to be around for weeks/months.

[ioctlLR]

Everyone: I'm the (mildly embarrassed) maintainer of NVorbis. Real (read: paying) work has kept me a bit overwhelmed for quite some time now, so I must apologize for causing any concerns. To be honest, I'd happily let someone else take over as primary maintainer since I just don't think I'll have any time for it in the foreseeable future...

[mrhelmut]

No worries, didn't intend to add pressure, life happens to everybody. We worked around this with a fork but it's cool to know that we can point back to the upstream repository.

Since MonoGame is trying to move to a native decoder, I don't know if any of us would be good candidates to keep NVorbis moving forward.

[ioctlLR]

Understood. That being the case, can someone document the issues in .Net Core (I see threading, but don't see any detail)? At the least I can add an issue with that documentation for whenever I or someone else has time to mess with it.

I'm also interested to know what optimizations @tomspilman believes should be put in; I profiled it very heavily at one point and couldn't find any further optimizations (including removing the LINQ calls). Again, I can at least add an issue documenting the pain points and possible solutions.

[harry-cpp]

Understood. That being the case, can someone document the issues in .Net Core (I see threading, but don't see any detail)? At the least I can add an issue with that documentation for whenever I or someone else has time to mess with it.

.Net Core does not support threads so you can't really get a separate thread for playing audio.

[Jjagg]

System.Threading.Thread is not in .NET Core. See dotnet/corefx#2576.

[mellinoe]

Saying that System.Threading.Thread is not supported on .NET Core is a little misleading (although it's not surprising given the general confusion around names and terminology). .NET Core has always supported the Thread type since version 1.0, but UAP does not. The next version of UAP WILL support System.Threading.Thread. In any case, I believe MonoGame already supports UAP, so that part shouldn't be a problem anyways.

[harry-cpp]

I feel blind, need to do some retesting.

[harry-cpp]

Got NVorbis properly working on .Net Core, thanks @mellinoe for correcting me, it was quite helpful :D