Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update the FMOD audio system, DSP system #1032

Open
wants to merge 41 commits into
base: master
Choose a base branch
from

Conversation

lachbr
Copy link
Contributor

@lachbr lachbr commented Oct 8, 2020

This PR updates the FMOD audio implementation to use the modern FMOD Core API, makes general improvements and fixes to the FMOD implementation, and implements a new interface for creating and applying DSP filters.

More specific change list:

  • Updated to use FMOD Core API
  • CMake/makepanda look for the FMOD Core libraries and link it with libp3fmod_audio
  • set_play_rate() on a MIDI sequence changes the tempo instead of the frequency, keeping the pitch unchanged
  • Pause/resume of a looped sound correctly resumes it from where it was paused
  • Added 7.1.4 speaker configuration
  • Sample rate of software mixer can be changed via a PRC variable
  • Implemented the concurrent sound limit system that OpenAL has
  • Fixed sound memory leaks by following what OpenAL does and keep a reference to sounds that are playing on the manager, and release the reference when the sound is finished during the manager's update() method.
  • Only update the FMOD System once per frame
  • set_speaker_mix() takes a single speaker ID and mix value instead of a mix value for all speakers, so speaker mix values can be changed one at a time instead of all at once.
  • Cleaned up the code to better follow Panda's coding style and conventions
  • Replace FilterProperties with a cleaner and more flexible system, described below

Info about DSP system:

  • Each type of DSP filter is a subclass of DSP, each with specific parameters that have documentation and can be set with Python properties
  • DSP filters can be individually inserted and removed from an audio manager
  • Changing the values of a DSP filter's parameters after adding it will automatically reflect in the audio manager
  • The same DSP filter can be added to multiple audio managers, and changes to parameters will automatically reflect in all audio managers the DSP has been added to.
  • Each DSP filter has default values for all its parameters
  • The SFXReverbDSP has preset configurations for different types of environments, such as city, hallway, underwater, etc.

At the moment the DSP filters, parameters, and units come directly from FMOD since there isn't another audio implementation that supports DSP filters. There does appear to be a mapping to SoLoud for some of the filters, however.

Let me know what you think and if there's anything that should be done differently. Thanks!

Checklist

I have done my best to ensure that…

  • …I have familiarized myself with the CONTRIBUTING.md file
  • …this change follows the coding style and design patterns of the codebase
  • …I own the intellectual property rights to this code
  • …the intent of this change is clearly explained
  • …existing uses of the Panda3D API are not broken
  • …the changed code is adequately covered by the test suite, where possible.

@codecov
Copy link

codecov bot commented Oct 8, 2020

Codecov Report

Merging #1032 (a865fe8) into master (c9aedc2) will increase coverage by 0.08%.
The diff coverage is 22.01%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1032      +/-   ##
==========================================
+ Coverage   15.67%   15.76%   +0.08%     
==========================================
  Files        3715     3742      +27     
  Lines      358334   357896     -438     
==========================================
+ Hits        56177    56409     +232     
+ Misses     302157   301487     -670     
Impacted Files Coverage Δ
panda/src/audio/audioManager.I 0.00% <0.00%> (ø)
panda/src/audio/audioManager.cxx 49.19% <0.00%> (-1.64%) ⬇️
panda/src/audio/audioManager.h 50.00% <ø> (ø)
panda/src/audio/audioSound.cxx 30.95% <0.00%> (+3.29%) ⬆️
panda/src/audio/audioSound.h 75.00% <ø> (ø)
panda/src/audio/chorusDSP.I 0.00% <0.00%> (ø)
panda/src/audio/compressorDSP.I 0.00% <0.00%> (ø)
panda/src/audio/config_audio.h 75.00% <ø> (ø)
panda/src/audio/distortionDSP.I 0.00% <0.00%> (ø)
panda/src/audio/dsp.I 0.00% <0.00%> (ø)
... and 73 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c9aedc2...f0bf80e. Read the comment docs.

@lachbr lachbr marked this pull request as draft October 9, 2020 07:03
@Moguri
Copy link
Collaborator

Moguri commented Oct 9, 2020

Nice work! As a heads up there was some work on getting DSPs working with OpenAl using EFX in #667. The comments contain a lot of information on trying to map the FMOD parameters to OpenAl.

It would be really nice to have the DSPs not be FMOD centric (e.g., with their names and parameters). However, I also understand finding a common set of names and parameters that maps well to multiple/any audio backends is likely a fool's errand.

@lachbr
Copy link
Contributor Author

lachbr commented Oct 9, 2020

Thanks. Yeah, I agree that it would be better to have the filters and parameters not be tied to a specific audio library. However it's not quite clear how to do that without imposing limits on the filters that can be taken advantage of in one audio library vs another.

@rdb
Copy link
Member

rdb commented Oct 9, 2020

It'd be good to have a core set of filters that will work with every API.

@Moguri
Copy link
Collaborator

Moguri commented Oct 9, 2020

I am not sure if I like this or not, but we could do something like generic DSP classes that specialized, backend-specific DSP classes could inherit from to add additional features/parameters/etc. So, something like ChorusDSP and ChorusFmodDSP.

@@ -99,6 +114,11 @@ ConfigVariableEnum<FmodSpeakerMode> fmod_speaker_mode
PRC_DESC("Sets the speaker configuration that the FMOD sound system will use. "
"Options: raw, mono, stereo, quad, surround, 5.1 and 7.1. "));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this text need "7.1.4" to be added in the list?

@kamgha
Copy link
Contributor

kamgha commented Oct 9, 2020

The filters could be handled similar to how the different physics libraries are organized: No generic classes, and each filter can expose the parameters that are available for their implementation. #667 mentions shortcomings of a unification, there is a tradeoff for configurability (e. g. parameters missing for the same effect) and the output may not be the same on a given input for each backend (which may not be the case depending on the used audio backend). Also, from a technical view, the question would arise why one backend would be chosen over the other when the offered features would be the same.

A generalization could be done at a higher level, e. g. in direct or something alike and it'd be up to the user whether they need the increased/higher grained flexbility that each implementation offers.

@lachbr
Copy link
Contributor Author

lachbr commented Oct 10, 2020

I think you make a good point.

The design of Panda's audio system is that the implementations for different back-ends are plug-and-play. You can just choose one and not really have to think about it, the API is the same. This works fine for basic things such as loading sounds, playing them, changing the volume, etc, but as you get into features that are specific to a particular back-end, how to expose these features under a unified API becomes a problem. For instance, FMOD has a wide variety of DSP filters and a feature-rich sound spatialization system (including geometry occlusion, reverb zones, etc). SoLoud and OpenAL have a smaller number of DSP filters with different parameters, and more primitive spatialization systems. If a Panda user wants to use FMOD for their project, they should be aware of that and should be able to take advantage of FMOD specific features. Same goes for OpenAL and theoretically SoLoud. If they can't, then there's no reason to have different options, aside from licensing.

A universal API for basic sound loading and playback is fine, but I think there should be back-end specific APIs for back-end specific features.

@Moguri
Copy link
Collaborator

Moguri commented Oct 21, 2020

I like the idea of a high-level, common, shared API for the basic stuff with more direct access for more complicated/advanced things.

…-core

# Conflicts:
#	makepanda/makepanda.py
Fixes dangling pointer segfault when calling `set_active()` or `stop_all_sounds()`
@paustian
Copy link
Contributor

Is there a downside of having a base audio class that implements the common API and then specific subclasses for each audio library? That seems like a pretty straightforward solution.

@lachbr lachbr marked this pull request as ready for review December 20, 2020 20:01
@kamgha
Copy link
Contributor

kamgha commented Dec 20, 2020

Do we have two terms, DSP and Filter, meaning the same thing now? Looks like FMOD calls them "DSP", while OpenAL and SoLoud call the effects "Filter", like we had before. Would it be sensible, to rename the occurences of DSP (back to) Filter? I see in the code of this PR some variables still being named filter, whereas the type is DSP, which could confirm this idea.

{
NormalizeDSP *norm_conf = DCAST(NormalizeDSP, dsp_conf);
dsp->setParameterFloat(FMOD_DSP_NORMALIZE_FADETIME, norm_conf->get_fade_time());
dsp->setParameterFloat(FMOD_DSP_NORMALIZE_THRESHHOLD, norm_conf->get_threshold());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to error out

In file included from panda/src/audiotraits/fmod_audio_composite1.cxx:3:
panda/src/audiotraits/fmodAudioManager.cxx:989:30: error: use of undeclared
      identifier 'FMOD_DSP_NORMALIZE_THRESHHOLD'; did you mean
      'FMOD_DSP_NORMALIZE_THRESHOLD'?
      dsp->setParameterFloat(FMOD_DSP_NORMALIZE_THRESHHOLD, norm_conf->g...
                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                             FMOD_DSP_NORMALIZE_THRESHOLD

Should be revised to FMOD_DSP_NORMALIZE_THRESHOLD?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this on the latest version of the FMOD Engine? If so, it must've been renamed to that, because the version of FMOD I have has FMOD_DSP_NORMALIZE_THRESHHOLD.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah must be a latest version thing

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you let me know which version of API you used so I can implement this in my custom panda3d version?

@rdb
Copy link
Member

rdb commented Feb 7, 2023

What's the status on this PR?

It would probably be really useful to have the FMOD update split out from the DSP system changes

@rdb
Copy link
Member

rdb commented Aug 5, 2023

Still interested in splitting this, or should someone else take over the work? Would be nice to have the FMOD update in 1.11.

@Moguri Moguri added this to the 1.11.0 milestone Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants