Skip to content

Commit

Permalink
Driver: Add toggle for softclip (#137)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
FelixMcFelix committed Nov 19, 2023
1 parent 0beb0f0 commit 13946b4
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 27 deletions.
30 changes: 24 additions & 6 deletions benches/mixing-task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use songbird::{
},
input::{cached::Compressed, codecs::*, Input, RawAdapter},
tracks,
Config,
};
use std::io::Cursor;
use tokio::runtime::{Handle, Runtime};
Expand All @@ -33,6 +34,7 @@ use xsalsa20poly1305::{aead::NewAead, XSalsa20Poly1305 as Cipher, KEY_SIZE};

fn dummied_mixer(
handle: Handle,
softclip: bool,
) -> (
Mixer,
(
Expand All @@ -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(),
Expand All @@ -74,6 +78,7 @@ fn dummied_mixer(
fn mixer_float(
num_tracks: usize,
handle: Handle,
softclip: bool,
) -> (
Mixer,
(
Expand All @@ -83,7 +88,7 @@ fn mixer_float(
Receiver<UdpTxMessage>,
),
) {
let mut out = dummied_mixer(handle);
let mut out = dummied_mixer(handle, softclip);

let floats = utils::make_sine(10 * STEREO_FRAME_SIZE, true);

Expand Down Expand Up @@ -113,7 +118,7 @@ fn mixer_float_drop(
Receiver<UdpTxMessage>,
),
) {
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);
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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());
},
Expand All @@ -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());
Expand Down
23 changes: 23 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -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)),
Expand Down Expand Up @@ -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<Duration>) -> Self {
Expand Down
54 changes: 33 additions & 21 deletions src/driver/tasks/mixer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)."),
)?;
}
}
}

Expand All @@ -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 {
Expand All @@ -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) {
Expand All @@ -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()
Expand All @@ -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],
)?
},
Expand Down

0 comments on commit 13946b4

Please sign in to comment.