osu-framework
osu-framework copied to clipboard
ppy
Add virtual audio mixer
Re: https://github.com/ppy/osu/pull/16771#discussion_r799795987
Context: The final goal is to expose audio mixers to mods so that they can apply audio filters in song select. After some discussion (linked above), the agreed-upon method is to include an AudioMixer parameter in IApplicableToTrack.ApplyToTrack method. Since this method is sometimes used in situations where the mixer is irreverent (e.g. in diff calc), this PR creates a dummy audio mixer that can be used in those cases.
Don't think tests are needed for this, since the only job of this dummy mixer is to carry the Effects list.
there's really not much to review here. i'm personally holding off to see if there's consensus on even adding this and the PR that depends on this in the first place.
really weird to leave a "request changes" review that doesn't request any actual changes.
also not seeing what the nested mixer thing has to do with this. if that gets merged (which is unclear if it will, since it's been kind of hanging there for a while), virtual mixers could just not support adding children, no?
also not seeing what the nested mixer thing has to do with this. if that gets merged (which is unclear if it will, since it's been kind of hanging there for a while), virtual mixers could just not support adding children, no?
Dunno, but fair point about the direction in #4915 not being set in stone yet I suppose.
I'm confused as to where this is actually used. I can't see any usage of it in any of the osu-side PRs, nor in this PR framework side.
This is related to https://github.com/ppy/osu/pull/16771. The virtual audio mixer would allow the whole IApplicableToTrackMixer interface to be removed. Instead, the audio mixer will be provided to mods via IApplicableToTrack.ApplyToTrack(Track, IAudioMixer). This is not implemented in that osu-side PR yet because I'm not sure if this virtual mixer approach is acceptable.
Do you happen to have a diff showing how you'd see that working instead? I think that's the minimum required to move forward with this (we usually don't add new classes in the framework without accompanying usage and tests, so I'd see that as being added here if this was a decided direction).
I believe it would pretty much amount to using the virtual mixer in this diffcalc code path (presuming that the mixer gets added to IApplicableToTrack as a second param).
Here's how https://github.com/ppy/osu/pull/16771 can be changed to make use of the virtual mixer: https://github.com/ppy/osu/commit/c500bccd2e9408359c2183b2fd0be2bd4b461dda
Full diff with master (along with basic usage of the provided audio mixer in muted mod): https://github.com/ppy/osu/compare/master...hlysine:use-virtual-audio-mixer
Are you sure that diff even works? The fact you are making a local mixer for the track seems to imply that all existing filters which are being applied to AudioManager.TrackMixer would no longer affect the game track (which I believe would require #4915 to resolve if that's the correct direction even).
Did not realize that global and local mixers are completely separate. I don't think it's a good idea to juggle with 2 track mixers then, at least not before #4915 is resolved. I've reverted to using the global track mixer: https://github.com/ppy/osu/compare/master...hlysine:use-virtual-audio-mixer
I've tested it this time that existing filters and mod filters are both working. Would this diff justify the existence of a virtual audio mixer then (as opposed to a separate IApplicableToTrackMixer interface)?