Skip to content
This repository has been archived by the owner on Mar 30, 2019. It is now read-only.

[Toolkit.Audio] Adding audio support to the toolkit. #262

Closed
wants to merge 21 commits into from
Closed

[Toolkit.Audio] Adding audio support to the toolkit. #262

wants to merge 21 commits into from

Conversation

dfkeenan
Copy link
Contributor

As requested:

Added new assembly implementing audio support for the toolkit. This is roughly based on XNA and the DirectX toolkit. It is still a work in progress. Pull request for sake of tracking progress.

Known issues:

  • Crashes when when reverb is enabled. (Problem has been identified, just needs to be corrected).
  • Missing documentation comments.
  • Missing nuspec file
  • Some code styling issues.

Tried to improve SoundEffect.Duration based on DirectXTk.

Started Code tidy-up and adding Document Comments.
Added SoundEffectInstance.Apply3D and SoundEffectInstance.Apply2D

Suped-up demo to make testing easier
…e type when possible, falls back to pooling per SoundEffect if unable to. As yet does not restrict maximum number of allocated voices.
Tried to fix audio disposal so it does not crash, seems ok now.  Because GameSystems get disposed before assets, changes were required so the AudioManager disposes of assets before it shutsdown the rest of the XAudio graph.
Added master limiter effect to AudioManager
… "ReverbConvertI3DL2ToNative".

Minor changes to Audio manager so Enabling Reverb does not crash initialisation.  Enabling Reverb still causes Apply3D to crash.
@ArtiomCiumac ArtiomCiumac mentioned this pull request Jan 29, 2014
…ys calculates reverb level of zero.

Added low pass filter, causes crash if enabled.
@dfkeenan
Copy link
Contributor Author

dfkeenan commented Feb 8, 2014

Another Update:

I have fixed the crash when reverb is enabled. However when using X3DAudio.Calcluate it always returns a reverb level of zero. I have not been able to workout why. There is reverb when I hard code a level greater than zero.

I have added a low pass filter (same as in DirectxTk). Currently this crashes when enabled :(
It keeps reporting an error like "XAUDIO2: ERROR: Send # 1: flags (0x8) must be 0 or XAUDIO2_SEND_USEFILTER". Which I find confusing because I am pretty sure XAUDIO2_SEND_USEFILTER (VoiceSendFlags.UseFilter) is equal to 0x8.

Any help with these 2 issues would be a great.

In other news:

I am considering changing how these features are enabled. My original plan was to make it possible to turn them on/off at will but things get kind of messy once voices are created/playing. So I was considering changing it so all these extra features (Master Limiter, positional audio, reverb, low pass filter) could be enabled before the initialization was run but once you are up and running they stay enabled. But still be able to change parameters i.e. reverb parameters Please let me know what you think.

Once all of the above is sorted out I was going to declare it feature complete for v1 and just concentrate on cleaning up code and doing to documentation comments. @xoofx, @ArtiomCiumac Please let me know what you think.

Cheers,
dfkeenan

@ArtiomCiumac
Copy link
Contributor

@dfkeenan, I have downloaded your branch - the functionality looks good, however the code definitely needs some comments and cleanup.

Please let us know when you are done with functionality - I can do some refactoring too and would like to avoid merge conflicts, so I will wait your notification.

PS: If possible - fix the audio manager to provide better error exception if no audio devices are present in system (currently it just crashes with unknown SharpDXException).

@dfkeenan
Copy link
Contributor Author

Update:

The reason why reverb level was always 0.0f was because the distance scale was too low. I changed the demo to set it higher. Not sure what a good default would be so I left it at 1.0f.

Fixed the SetOutputVoices issue. The problem was the mapping for the VoiceSendFlags enum was wrong. "XAUDIO2_VOICE_USEFILTER" != "XAUDIO2_SEND_USEFILTER". Also the only vaild flags for creating submix voices is 0 or "XAUDIO2_VOICE_USEFILTER" so I created a new enum called SubmixVoiceFlags.

So I guess I'll start working on the clean-up and documentation now. Hopefully make a start this weekend.

@ArtiomCiumac for the exception handling were you just thinking of putting a try/catch around each of the steps in initialization (create device, create master voice, etc.) and report which stage failed (and if possible why depending on how good the exception detail is)? Is a new exception class required or just something like "InvalidOperationException" do?

cheers,
dfkeenan

@ArtiomCiumac
Copy link
Contributor

Yes, a try-catch would be fine. A custom exception is usually better (you can inherit from SharpDXException) - as it is easier to filter them with different catch blocks. Maybe there should be some possibility to disable it, for example if user wants to run the game with audio disabled. But this is not strictly necessary.

@xoofx
Copy link
Member

xoofx commented Feb 14, 2014

Fine with a single AudioException (but not specialized by type of exception for Audio).

I'm considering soon (like in the next week or two) to release a 2.6 version of SharpDX. Do you think Toolkit.Audio can make it into it or do you prefer to just deliver it later to the 2.6.1 dev package?

@dfkeenan
Copy link
Contributor Author

Hi @xoofx ,

I was going to try and do a a fair bit of code clean-up and comments this weekend. I am not sure how well tested it is though. I have only really been testing the 1 platform and I am not sure how much coverage my ad-hoc testing (via demo) has gotten. So I couldn't say for sure if it would be considered good enough in time for the proposed schedule of the 2.6 release.

I guess we can review the situation again in the next few days and see how we feel about.

Cheers,
dfkeenan

@dfkeenan
Copy link
Contributor Author

I have added an AudioException class and the AudioManager class now raises it if it fails to set things up.

@ArtiomCiumac, I was going to add a special case message for errors creating an XAudio2 device if no audio devices are present. According to documentation it returns ERROR_NOT_FOUND. But I have not been able to get this to happen. I have tried disabling all sound/audio things in Device manager and it still successfully creates one. But it then crashes when creating the mastering voice, so not exactly not sure what to do here.

Anyways, guess I will start tiding things up soon.

@ArtiomCiumac
Copy link
Contributor

it then crashes when creating the mastering voice

I encountered exactly same behavior, therefore we need either to check programmatically that an audio device is present or try to create a mastering voice and act accordingly if it throws an exception.

@dfkeenan
Copy link
Contributor Author

@ArtiomCiumac

I think I have this sorted now. For non-win8 it (i.e. Net40Debug) it checks the device count is not equal to zero. For win8 it checks the return code of the call to create mastering voice if it is ERROR_NOT_FOUND then it raises an audio exception with the message "No default audio devices detected." otherwise a generic "Error creating mastering voice." message is used.

I currently have the constant for ERROR_NOT_FOUND in the AudioManager class as I didn't think it quite fit to put it with all the general ones in the result class. Not sure where else to put it really.

I really should get to that clean-up and documenting thing.

@ArtiomCiumac
Copy link
Contributor

Looks fine for me. The API can be polished afterwards.

@dazerdude
Copy link
Contributor

Unless I've missed a change somewhere, it doesn't look like you have any integration with the Tookit's ContentManager. I've got some time I could spend on this task if no one else is planning on working on it.

@dfkeenan
Copy link
Contributor Author

@dazerdude It does have ContentManager integration. The AudioManager implements interfaces IContentReader and IContentReaderFactory. See the audio sample "Samples/Toolkit/Desktop/Audio/AudioGame.cs" for example of usage.

@dazerdude
Copy link
Contributor

@dfkeenan Oh great! I missed that somehow... O.o

@dfkeenan
Copy link
Contributor Author

I have done a quick once over of the code to try tidy it up a bit. (Wanted to try out CodeMaid.)

Probably should have grouped some things together in regions i.e. disposal related methods/properties. Not sure about the ordering of some things, maybe I shouldn't have sorted somethings alphabetically??

Still haven't done anymore document comments. My momentum/interest is starting to wane and I am feeling kind of lazy so.....not sure when I will get to this.

Tip (for others and future self): Do document comments as you go so it's not such a burden at the end.

@ArtiomCiumac
Copy link
Contributor

I have looked over the code - there is possible to do some more cleanup (and to add some more documentation). Even if it is boring - someone has to do it and I have now some free time - so I can work on that. For this I will merge this pull request in my local branch, then will merge it to master_integration branch, as it doesn't seem to do any changes that may affect other areas - this pull requests only adds new stuff (actually a lot of new stuff).

@dfkeenan, after this - you will need to merge my changes to your repo if you will want to do any additional work (to avoid conflicts).

@dfkeenan, @xoofx, what do you think about my plan?

@dfkeenan
Copy link
Contributor Author

@ArtiomCiumac , If you wanted to tackle the final cleanup and do some documentation I would be more than happy to let you. 😃 (Although I feel a bit guilty about passing the buck.)

If you think you have a good shot of getting it cleaned-up in time for the 2.6 release that @xoofx is planing then go for it. If not I would be happy to keep going, albeit slowly.

(My repo is quite out of date as is, so I was probably going to "start again" once the pull happened anyways.)

I will be happy with whatever you and @xoofx decide. I just want to help make SharpDX more awesome.

@ArtiomCiumac
Copy link
Contributor

I'm back! Sorry for disappearing for long time.
I have added XML documentation and pushed all changes to a new audio branch. If you will want to make any additional commits - first make sure your local copy is synchronized with the audio branch.
As next steps, I will test the audio functionality on as many platforms as possible and, if everything will be ok, will merge it to master_integration.

Even if the code can be improved - the functionality is really well written and it is a quite big addition to SharpDX.Toolkit.

@ArtiomCiumac
Copy link
Contributor

I have changed the Audio sample to use PointerManager instead of MouseManager to allow execution of the same code on all platforms (obviously, WP8 doesn't support mouse input). Sample projects for WinRT and WP8 have been added and I can confirm that audio works correctly on all platforms - I tested Desktop (.NET 4.0), WinRT and WP8 on the following devices: Windows 8.1 x64 for Desktop and WinRT, and Nokia Lumia 920 for WP8.

I propose the following:

  1. Merge audio branch into master_integration.
  2. Close this pull request as its commits are already in the main repository and it got a little bit out of sync.
  3. If any further discussions are needed - open a separate pull request for audio branch.

@xoofx, @dfkeenan, what do you think?

@dfkeenan
Copy link
Contributor Author

@ArtiomCiumac nice work! I was wondering where you ran away to 😃

I was considering adding another public constructor to the WaveBank class that takes a dictionary of SoundEffects. So you could create a WaveBank from loose wave files. This can be done at a later stage though.

@ArtiomCiumac, @xoofx, I think it's good-to-go. If you want to merge and close this pull request thats cool with me. Hopefully this can make v2.6 then.

@ArtiomCiumac
Copy link
Contributor

The audio branch has been merged into master_integration, now the latest dev version of SharpDX SDK should have support for Audio :)

@dfkeenan, please update your copy of repository from master_integration if you will want to make any additional contributions. You did an awesome work, thank you very much!

@dfkeenan dfkeenan deleted the Toolkit_Audio branch March 26, 2014 13:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants