From d62c8e79debfaa6f1ae8998e40174bf22c88bdf9 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Mon, 25 Jul 2022 16:20:17 +0100 Subject: [PATCH] Driver: Add toggle for softclip This adds the `use_softclip` field to `Config`, which can currently provide a ~10us reduction in mixing cost from both a removed memcpy and the softclip itself. This PR was tested using cargo make ready. Closes #134. --- benches/mixing-task.rs | 30 +++++++++++++++---- src/config.rs | 23 +++++++++++++++ src/driver/tasks/mixer/mod.rs | 54 +++++++++++++++++++++-------------- 3 files changed, 80 insertions(+), 27 deletions(-) diff --git a/benches/mixing-task.rs b/benches/mixing-task.rs index 48c7a8894..036839294 100644 --- a/benches/mixing-task.rs +++ b/benches/mixing-task.rs @@ -23,6 +23,7 @@ use songbird::{ }, input::{cached::Compressed, codecs::*, Input, RawAdapter}, tracks, + Config, }; use std::io::Cursor; use tokio::runtime::{Handle, Runtime}; @@ -33,6 +34,7 @@ use xsalsa20poly1305::{aead::NewAead, XSalsa20Poly1305 as Cipher, KEY_SIZE}; fn dummied_mixer( handle: Handle, + softclip: bool, ) -> ( Mixer, ( @@ -55,7 +57,9 @@ fn dummied_mixer( mixer: mix_tx, }; - let mut out = Mixer::new(mix_rx, handle, ic, Default::default()); + let config = Config::default().use_softclip(softclip); + + let mut out = Mixer::new(mix_rx, handle, ic, config); let fake_conn = MixerConnection { cipher: Cipher::new_from_slice(&vec![0u8; KEY_SIZE]).unwrap(), @@ -74,6 +78,7 @@ fn dummied_mixer( fn mixer_float( num_tracks: usize, handle: Handle, + softclip: bool, ) -> ( Mixer, ( @@ -83,7 +88,7 @@ fn mixer_float( Receiver, ), ) { - let mut out = dummied_mixer(handle); + let mut out = dummied_mixer(handle, softclip); let floats = utils::make_sine(10 * STEREO_FRAME_SIZE, true); @@ -113,7 +118,7 @@ fn mixer_float_drop( Receiver, ), ) { - let mut out = dummied_mixer(handle); + let mut out = dummied_mixer(handle, true); for i in 0..num_tracks { let floats = utils::make_sine((i / 5) * STEREO_FRAME_SIZE, true); @@ -143,7 +148,7 @@ fn mixer_opus( ) { // should add a single opus-based track. // make this fully loaded to prevent any perf cost there. - let mut out = dummied_mixer(handle.clone()); + let mut out = dummied_mixer(handle.clone(), false); let floats = utils::make_sine(6 * STEREO_FRAME_SIZE, true); @@ -182,7 +187,20 @@ fn no_passthrough(c: &mut Criterion) { &track_count, |b, i| { b.iter_batched_ref( - || black_box(mixer_float(*i, rt.handle().clone())), + || black_box(mixer_float(*i, rt.handle().clone(), true)), + |input| { + black_box(input.0.cycle()); + }, + BatchSize::SmallInput, + ) + }, + ); + group.bench_with_input( + BenchmarkId::new("Single Packet (No Soft-Clip)", track_count), + &track_count, + |b, i| { + b.iter_batched_ref( + || black_box(mixer_float(*i, rt.handle().clone(), false)), |input| { black_box(input.0.cycle()); }, @@ -195,7 +213,7 @@ fn no_passthrough(c: &mut Criterion) { &track_count, |b, i| { b.iter_batched_ref( - || black_box(mixer_float(*i, rt.handle().clone())), + || black_box(mixer_float(*i, rt.handle().clone(), true)), |input| { for i in 0..5 { black_box(input.0.cycle()); diff --git a/src/config.rs b/src/config.rs index 4e2249fc1..1264aea7d 100644 --- a/src/config.rs +++ b/src/config.rs @@ -88,6 +88,20 @@ pub struct Config { /// [`Driver`]: crate::driver::Driver pub driver_retry: Retry, #[cfg(feature = "driver")] + /// Configures whether or not each mixed audio packet is [soft-clipped] into the + /// [-1, 1] audio range. + /// + /// Defaults to `true`, preventing clipping and dangerously loud audio from being sent. + /// + /// **This operation adds ~3% cost to a standard (non-passthrough) mix cycle.** + /// If you *know* that your bot will only play one sound at a time and that + /// your volume is between `0.0` and `1.0`, then you can disable soft-clipping + /// for a performance boost. If you are playing several sounds at once, do not + /// disable this unless you make sure to reduce the volume of each sound. + /// + /// [soft-clipped]: https://opus-codec.org/docs/opus_api-1.3.1/group__opus__decoder.html#gaff99598b352e8939dded08d96e125e0b + pub use_softclip: bool, + #[cfg(feature = "driver")] /// Configures the maximum amount of time to wait for an attempted voice /// connection to Discord. /// @@ -137,6 +151,8 @@ impl Default for Config { #[cfg(feature = "driver")] preallocated_tracks: 1, #[cfg(feature = "driver")] + use_softclip: true, + #[cfg(feature = "driver")] driver_retry: Retry::default(), #[cfg(feature = "driver")] driver_timeout: Some(Duration::from_secs(10)), @@ -184,6 +200,13 @@ impl Config { self } + /// Sets this `Config`'s number to enable/disable soft-clipping sent audio. + #[must_use] + pub fn use_softclip(mut self, use_softclip: bool) -> Self { + self.use_softclip = use_softclip; + self + } + /// Sets this `Config`'s timeout for establishing a voice connection. #[must_use] pub fn driver_timeout(mut self, driver_timeout: Option) -> Self { diff --git a/src/driver/tasks/mixer/mod.rs b/src/driver/tasks/mixer/mod.rs index 56c1c1d3c..7be73d546 100644 --- a/src/driver/tasks/mixer/mod.rs +++ b/src/driver/tasks/mixer/mod.rs @@ -569,7 +569,7 @@ impl Mixer { } pub fn cycle(&mut self) -> Result<()> { - let mut mix_buffer = [0f32; STEREO_FRAME_SIZE]; + let mut softclip_buffer = [0f32; STEREO_FRAME_SIZE]; // symph_mix is an `AudioBuffer` (planar format), we need to convert this // later into an interleaved `SampleBuffer` for libopus. @@ -635,22 +635,31 @@ impl Mixer { self.silence_frames = 5; if let MixType::MixedPcm(n) = mix_len { - // FIXME: When impling #134, prevent this copy from happening if softclip disabled. - // Offer sample_buffer.samples() to prep_and_send_packet. - - // to apply soft_clip, we need this to be in a normal f32 buffer. - // unfortunately, SampleBuffer does not expose a `.samples_mut()`. - // hence, an extra copy... - let samples_to_copy = self.config.mix_mode.channels() * n; - - (&mut mix_buffer[..samples_to_copy]) - .copy_from_slice(&self.sample_buffer.samples()[..samples_to_copy]); + #[cfg(test)] + let do_copy = true; + #[cfg(not(test))] + let do_copy = self.config.use_softclip; + + if do_copy { + // to apply soft_clip, we need this to be in a normal f32 buffer. + // unfortunately, SampleBuffer does not expose a `.samples_mut()`. + // hence, an extra copy... + // + // also just copy in for #[cfg(test)] to simplify the below logic + // for handing out audio data to the outside world. + let samples_to_copy = self.config.mix_mode.channels() * n; + + (&mut softclip_buffer[..samples_to_copy]) + .copy_from_slice(&self.sample_buffer.samples()[..samples_to_copy]); + } - self.soft_clip.apply( - (&mut mix_buffer[..]) - .try_into() - .expect("Mix buffer is known to have a valid sample count (softclip)."), - )?; + if self.config.use_softclip { + self.soft_clip.apply( + (&mut softclip_buffer[..]) + .try_into() + .expect("Mix buffer is known to have a valid sample count (softclip)."), + )?; + } } } @@ -662,6 +671,8 @@ impl Mixer { // usually a 20ms tick, in test modes this is either a finite number of runs or user input. self.march_deadline(); + let send_buffer = self.config.use_softclip.then(|| &softclip_buffer[..]); + #[cfg(test)] if let Some(OutputMode::Raw(tx)) = &self.config.override_connection { let msg = match mix_len { @@ -677,17 +688,17 @@ impl Mixer { OutputMessage::Passthrough(opus_frame) }, MixType::MixedPcm(_) => OutputMessage::Mixed( - mix_buffer[..self.config.mix_mode.sample_count_in_frame()].to_vec(), + softclip_buffer[..self.config.mix_mode.sample_count_in_frame()].to_vec(), ), }; drop(tx.send(msg.into())); } else { - self.prep_and_send_packet(&mix_buffer, mix_len)?; + self.prep_and_send_packet(send_buffer, mix_len)?; } #[cfg(not(test))] - self.prep_and_send_packet(&mix_buffer, mix_len)?; + self.prep_and_send_packet(send_buffer, mix_len)?; // Zero out all planes of the mix buffer if any audio was written. if matches!(mix_len, MixType::MixedPcm(a) if a > 0) { @@ -704,7 +715,8 @@ impl Mixer { } #[inline] - fn prep_and_send_packet(&mut self, buffer: &[f32; 1920], mix_len: MixType) -> Result<()> { + fn prep_and_send_packet(&mut self, buffer: Option<&[f32]>, mix_len: MixType) -> Result<()> { + let send_buffer = buffer.unwrap_or_else(|| self.sample_buffer.samples()); let conn = self .conn_active .as_mut() @@ -726,7 +738,7 @@ impl Mixer { MixType::MixedPcm(_samples) => { let total_payload_space = payload.len() - crypto_mode.payload_suffix_len(); self.encoder.encode_float( - &buffer[..self.config.mix_mode.sample_count_in_frame()], + &send_buffer[..self.config.mix_mode.sample_count_in_frame()], &mut payload[TAG_SIZE..total_payload_space], )? },