osu icon indicating copy to clipboard operation
osu copied to clipboard

Add a 'ducking' effect to the currently playing track when changing ruleset

Open nekodex opened this issue 1 year ago • 1 comments

A combination volume+filter effect is added to attenuate the currently playing track to allow the ruleset selection samples to be better heard.

Also adds 'sample-choking' logic to prevent overlapping sample playback when rapidly cycling through rulesets.

https://github.com/ppy/osu/assets/272140/3c505417-1263-4835-836a-65edc28ff157

The ducking is implemented via new methods added to MusicController, which are generic enough to also replace some AudioFilter usages in the codebase that provide a similar effect.

I originally wrote this as a component ala AudioFilter, but having it coupled with an ITrackStore to do audio adjustments for volume attenuation felt weird, so I instead just implemented them as methods on MusicControler (for now?). After ManagedBass is updated to support the volume effect type (i.e. via peppy/ManagedBass#1 ?), we can do attenuation via a volume effect on the mixer instead of using audio adjustments and that'd allow for a cleaner component.

I also tried adding the ducking effect to other places (f.e. ButtonSystem), but due to audio adjustments being relative/percentage-based, I couldn't get it sounding consistently nice... with music volume at 70% it would sound okay, but at 100% it sounded off, especially with louder songs. I'll look into it again later, but maybe a fixed decibel reduction could work better... but that's for a separate/future PR.

nekodex avatar Jun 21 '24 21:06 nekodex

It seems the game crashes when switching to a custom ruleset with this PR...

https://github.com/ppy/osu/assets/62504841/f9e0fbfd-b5fe-4684-b2cc-ed0614b67112

I'm not really a developer at all, just looking around, but noticed this issue with this PR and thought I should report it.

Logs: compressed-logs.zip

LittleLilyBun avatar Jun 22 '24 00:06 LittleLilyBun

I'm hoping that we can simplify the API down for now to follow KISS principles, removing the easing and unduck settings. @nekodex do you have use cases already in mind which would be overriding these (ie. different duck and unduck duration, or different easing curves)?

diff --git a/osu.Game/Collections/ManageCollectionsDialog.cs b/osu.Game/Collections/ManageCollectionsDialog.cs
index 0396fd531c..b60d31ef8d 100644
--- a/osu.Game/Collections/ManageCollectionsDialog.cs
+++ b/osu.Game/Collections/ManageCollectionsDialog.cs
@@ -125,7 +125,7 @@ protected override void Dispose(bool isDisposing)
 
         protected override void PopIn()
         {
-            audioDucker = musicController?.Duck(100, 1f, unduckDuration: 100);
+            audioDucker = musicController?.Duck(100, 1f);
 
             this.FadeIn(enter_duration, Easing.OutQuint);
             this.ScaleTo(0.9f).Then().ScaleTo(1f, enter_duration, Easing.OutQuint);
diff --git a/osu.Game/Overlays/DialogOverlay.cs b/osu.Game/Overlays/DialogOverlay.cs
index 7c52081053..04daabda6c 100644
--- a/osu.Game/Overlays/DialogOverlay.cs
+++ b/osu.Game/Overlays/DialogOverlay.cs
@@ -106,7 +106,7 @@ void dismiss()
 
         protected override void PopIn()
         {
-            audioDucker = musicController.Duck(100, 1f, unduckDuration: 100);
+            audioDucker = musicController.Duck(100, 1f);
         }
 
         protected override void PopOut()
diff --git a/osu.Game/Overlays/MusicController.cs b/osu.Game/Overlays/MusicController.cs
index 54a0436f2a..3e9f801df8 100644
--- a/osu.Game/Overlays/MusicController.cs
+++ b/osu.Game/Overlays/MusicController.cs
@@ -263,13 +263,10 @@ public void NextTrack(Action? onSuccess = null) => Schedule(() =>
         /// <summary>
         /// Attenuates the volume and/or filters the currently playing track.
         /// </summary>
-        /// <param name="duration">Duration of the ducking transition, in ms.</param>
+        /// <param name="duckDuration">Duration of the ducking transition, in ms.</param>
         /// <param name="duckVolumeTo">Level to drop volume to (1.0 = 100%).</param>
         /// <param name="duckCutoffTo">Cutoff frequency to drop `AudioFilter` to. Use `null` to skip filter effect.</param>
-        /// <param name="easing">Easing for the ducking transition.</param>
-        /// <param name="unduckDuration">Duration of the unducking transition, in ms.</param>
-        /// <param name="unduckEasing">Easing for the unducking transition.</param>
-        public IDisposable? Duck(int duration = 0, float duckVolumeTo = 0.25f, int? duckCutoffTo = 300, Easing easing = Easing.OutCubic, int unduckDuration = 500, Easing unduckEasing = Easing.InCubic)
+        public IDisposable? Duck(int duckDuration = 0, float duckVolumeTo = 0.25f, int? duckCutoffTo = 300)
         {
             if (audioDuckActive) return null;
 
@@ -278,31 +275,27 @@ public void NextTrack(Action? onSuccess = null) => Schedule(() =>
             Schedule(() =>
             {
                 if (duckCutoffTo.IsNotNull())
-                    audioDuckFilter?.CutoffTo((int)duckCutoffTo, duration, easing);
+                    audioDuckFilter?.CutoffTo((int)duckCutoffTo, duckDuration, Easing.Out);
 
-                this.TransformBindableTo(audioDuckVolume, duckVolumeTo, duration, easing);
+                this.TransformBindableTo(audioDuckVolume, duckVolumeTo, duckDuration, Easing.Out);
             });
 
-            return new InvokeOnDisposal(() => unduck(unduckDuration, unduckEasing));
+            return new InvokeOnDisposal(() => unduck(duckDuration, Easing.In));
         }
 
         /// <summary>
         /// A convenience method that ducks the currently playing track, then after a delay, unducks it.
         /// </summary>
-        /// <param name="delay">Delay after audio is ducked before unducking begins, in ms.</param>
-        /// <param name="unduckDuration">Duration of the unducking transition, in ms.</param>
-        /// <param name="unduckEasing">Easing for the unducking transition.</param>
+        /// <param name="delayBeforeUnduck">Delay after audio is ducked before unducking begins, in ms.</param>
         /// <param name="duckVolumeTo">Level to drop volume to (1.0 = 100%).</param>
         /// <param name="duckCutoffTo">Cutoff frequency to drop `AudioFilter` to. Use `null` to skip filter effect.</param>
         /// <param name="duckDuration">Duration of the ducking transition, in ms.</param>
-        /// <param name="duckEasing">Easing for the ducking transition.</param>
-        public void DuckMomentarily(int delay, int unduckDuration = 500, Easing unduckEasing = Easing.InCubic, float duckVolumeTo = 0.25f, int? duckCutoffTo = 300, int duckDuration = 0,
-                                    Easing duckEasing = Easing.OutCubic)
+        public void DuckMomentarily(int delayBeforeUnduck, float duckVolumeTo = 0.25f, int? duckCutoffTo = 300, int duckDuration = 0)
         {
             if (audioDuckActive) return;
 
-            Duck(duckDuration, duckVolumeTo, duckCutoffTo, duckEasing);
-            Scheduler.AddDelayed(() => unduck(unduckDuration, unduckEasing), delay);
+            Duck(duckDuration: duckDuration, duckVolumeTo: duckVolumeTo, duckCutoffTo: duckCutoffTo);
+            Scheduler.AddDelayed(() => unduck(duckDuration, Easing.In), delayBeforeUnduck);
         }
 
         private void unduck(int duration, Easing easing)

peppy avatar Jul 05 '24 04:07 peppy

I'm hoping that we can simplify the API down for now to follow KISS principles, removing the easing and unduck settings. @nekodex do you have use cases already in mind which would be overriding these (ie. different duck and unduck duration, or different easing curves)?

The ruleset selector has a duck duration of 0 (instant) with a different unduck duration - instant duck to make room for the initial transient of the samples, while having a more gradual unduck to bring the track back in more gently.

Easing settings I'm probably fine with removing for now, it was more forward-thinking to allow for choosing different curves in the future, but I have no specific use cases for it right now (if you're fine with changing dialogs to be non-cubic).

nekodex avatar Jul 05 '24 04:07 nekodex

The ruleset selector has a duck duration of 0 (instant) with a different unduck duration - instant duck to make room for the initial transient of the samples, while having a more gradual unduck to bring the track back in more gently.

Hmm, I'm thinking the unduck duration and easing should be locked globally (for momentary ducks), and keeping the parameter as only the duck duration.

peppy avatar Jul 05 '24 04:07 peppy

Hmm, I'm thinking the unduck duration and easing should be locked globally (for momentary ducks), and keeping the parameter as only the duck duration.

I get the desire for simplicity, but thinking ahead to future potential usages (for ButtonSystem, etc), I'd definitely prefer to have more control here. Actually, thinking about ButtonSystem makes me think I may miss the ability to have selectable easings... but oh well.

nekodex avatar Jul 05 '24 04:07 nekodex

hmm, okay then. i'll refactor the api surface with that in mind (keep full customisation for now).

peppy avatar Jul 05 '24 04:07 peppy

I've restructured the API but still have a question:

The way this currently works means that if a duck operation is in progress, all further operations will fail. I can see this going very wrong very fast, where say:

  • Two things both want to duck, both try to duck, then the first restored ducking and the game is not ducking when it should be
  • Something like the toolbar ruleset selection is ducking on quick operations, but any duck requests during the duration of the first one will be ignored.

@nekodex what are your thoughts on these? fixing requires first deciding how these cases should be handled (multiplicative ducking? always applying the most extreme ducking?)

peppy avatar Jul 05 '24 06:07 peppy

I had considered keeping a list of duck calls and unducking each one respectively on dispose, but that felt somewhat overcomplicated... but maybe that's required for correctness after all.

@nekodex what are your thoughts on these? fixing requires first deciding how these cases should be handled (multiplicative ducking? always applying the most extreme ducking?)

I'm thinking that only applying the most extreme ducking is the correct approach.

The only place I found that currently has overlapping Duck() usages is when triggering a confirmation popup from ManageCollectionsDialog. That dialog is also the only place that allows triggering DuckMomentarily() while a Duck() is in progress - and arguably the ability to do that (clicking Toolbar to change ruleset while the dialog is visible) feels out of place.

In the above scenario, I think it would be weird if the confirmation popup increased the ducking amount when shown (i.e. multiplicative). The ducking parameters of the dialogs are identical, so I'd expect the ducking amount to stay the same?

I also don't think we should make a habit of overlapping Duck() usage, I don't really foresee using it for things other than modal dialogs/overlays anyway?

nekodex avatar Jul 05 '24 07:07 nekodex

The other case which already happens with DuckMomentarily in the ruleset selector with fast selection – you can end up getting the duck to end earlier than expected after a few quick ruleset button changes using hotkeys.

peppy avatar Jul 05 '24 08:07 peppy

@nekodex i've applied changes as proposed (the "deepest" duck is applied, separately for volume and lowpass for the best effect). give it a try and see what you think.

I've also added tests to be able to test this change (dunno how you work without tests 🤷).

peppy avatar Jul 05 '24 09:07 peppy

@ppy/team-client request a second pair of eyes on this since I made considerable changes.

peppy avatar Jul 05 '24 09:07 peppy