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

Feature: ChannelRouter #656

Open
wants to merge 30 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
f497703
Adding a ChannelRouter source
iluvcapra Dec 7, 2024
cfdea36
Implementation continues
iluvcapra Dec 7, 2024
121105e
Simplified handling of frame endings
iluvcapra Dec 7, 2024
edc683b
More implementation, added mod functions
iluvcapra Dec 7, 2024
6037f67
Some documentation
iluvcapra Dec 7, 2024
b45b936
Flatted-out next a little
iluvcapra Dec 8, 2024
51b1f4b
rusfmt and typo
iluvcapra Dec 8, 2024
67b16a1
Typos and added methods, also documentation
iluvcapra Dec 8, 2024
f7d8220
clippy
iluvcapra Dec 8, 2024
1204fdf
Inline everything!
iluvcapra Dec 8, 2024
9d43421
Added extract_channels and extract_channel sources
iluvcapra Dec 8, 2024
77763ae
Gains implemented as an atomic array of f32s
iluvcapra Dec 9, 2024
3f16c25
Mutex-implemented, but need to clean this up
iluvcapra Dec 9, 2024
d39bbf2
Implemented updates with a mpsc::channel
iluvcapra Dec 10, 2024
45c4688
rustfmt
iluvcapra Dec 10, 2024
1662db2
Added more router conveniences
iluvcapra Dec 10, 2024
9b361b0
Added some comments and stubbed-out tests
iluvcapra Dec 10, 2024
5621477
Added some static tests
iluvcapra Dec 10, 2024
ca2ee9d
Added description to changelog
iluvcapra Dec 10, 2024
f521000
Test of the controller
iluvcapra Dec 10, 2024
9ae7119
rustfmt
iluvcapra Dec 10, 2024
0fe726c
Docstring for CI
iluvcapra Dec 10, 2024
08789de
For the pickier ubuntu-latest clippy
iluvcapra Dec 10, 2024
8cc8c6e
Removing the todo and addressing clippy
iluvcapra Dec 10, 2024
084fb78
Additional tests and impl for tests
iluvcapra Dec 10, 2024
c9cc895
Update channel_router.rs
iluvcapra Dec 10, 2024
a6f0873
Added a channel_routing example
iluvcapra Dec 10, 2024
cd51ea9
Merge branch 'feature-channelrouter' of https://github.com/iluvcapra/…
iluvcapra Dec 10, 2024
d0bdd45
Made channel_routing example interactive
iluvcapra Dec 10, 2024
276f23f
rustfmt
iluvcapra Dec 10, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added
- Output audio stream buffer size can now be adjusted.
- A `ChannelRouterSource` that can mix, re-order and extract channels from a
multi-channel source.
- Several `Source` trait helper functions including `extract_channel`,
`extract_channels`, `mono`, `mono_to_stereo`, and `downmix_51`.
- A `ChannelRouterController` type to modify the `ChannelRouterSource`
across thread boundaries.
- Sources for directly generating square waves, traingle waves, square waves and
sawtooths have been added.
- An interface for defining `SignalGenerator` patterns with an `fn`, see
Expand Down
3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,6 @@ required-features = ["symphonia-isomp4", "symphonia-aac"]
[[example]]
name = "noise_generator"
required-features = ["noise"]

[[example]]
name = "channel_routing"
43 changes: 43 additions & 0 deletions examples/channel_routing.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
//! Channel router example

use std::error::Error;

fn main() -> Result<(), Box<dyn Error>> {
use rodio::source::{Function, SignalGenerator, Source};
use std::thread;
use std::time::Duration;

let stream_handle = rodio::OutputStreamBuilder::open_default_stream()?;

// let test_signal_duration = Duration::from_millis(1000);
let interval_duration = Duration::from_millis(100);
let sample_rate = cpal::SampleRate(48000);

let (mut controller, router) = SignalGenerator::new(sample_rate, 1000.0, Function::Triangle)
.amplify(0.1)
.channel_router(2, vec![vec![0.0f32, 0.0f32]]);

println!("Playing 1000Hz tone");

stream_handle.mixer().add(router);

for i in 0..1000 {
thread::sleep(interval_duration);
let n = i % 20;
match n {
0 => println!("Left speaker ramp up"),
1..10 => {
_ = controller.map(0, 0, n as f32 / 10.0);
_ = controller.map(0, 1, 0f32);
Copy link
Collaborator

@dvdsk dvdsk Dec 11, 2024

Choose a reason for hiding this comment

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

this is not very readable right? I can guess its channel from, channel too, gain? But we might be able to do better. I see a number of options, there might well be more:
1️⃣ make map take a struct like: controller.map(Mapping { from: 0, to: 1, gain: 0f32 })

2️⃣ make map a builder: controller.map().from(0).to(1).with_gain(0.f32).appy(). Here you could make the gain optional and do a default of 1.0

3️⃣ rename it so the argument names are in the name: controller.map_from_to_with_gain(0, 1, 0f32).

I think 2️⃣ is my favorite, though its a little more code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like the builder too.

}
10 => println!("Right speaker ramp up"),
11..20 => {
_ = controller.map(0, 0, 0.0f32);
_ = controller.map(0, 1, (n - 10) as f32 / 10.0);
}
_ => {}
}
}

Ok(())
}
296 changes: 296 additions & 0 deletions src/source/channel_router.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,296 @@
// Channel router types and implementation.

use crate::{Sample, Source};
use std::{
cmp::min,
sync::mpsc::{channel, Receiver, Sender},
};

/// A matrix to map inputs to outputs according to a gain
///
/// A two-dimensional matrix of `f32`s:
/// - The first dimension is respective to the input channels
/// - The second is respective to the output channels
///
/// Thus, if a value at `map[1,1]` is 0.2, this signifies that the signal on
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is still confusing, I'd picked different numbers for input and output, or maybe just use names instead of constants (e.g. map[in,out])

/// channel 1 should be mixed into channel 1 with a coefficient of 0.2.
pub type ChannelMap = Vec<Vec<f32>>;
// doing this as Vec<Vec<atomic_float::AtomicF32>> would require feature=experimental, so I decided
// to just use a channel to do updates.
//
// Doing it as a HashMap<(u16,u16), f32> is an option too but there's a penalty hashing these
// values, there's ways to speed that up though. It'd be great if the object upgraded its
// implementation if it got sufficiently big.
Copy link
Collaborator

Choose a reason for hiding this comment

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

No we do not need complex containers, if a sparse matrix is preferable I have an example how that can be implemented.


// pub fn empty_channel_map(inputs: u16, outputs: u16) -> ChannelMap {
// vec![vec![0.0f32; outputs.into()]; inputs.into()]
// }

/// Internal function that builds a [`ChannelRouter<I>`] object.
pub fn channel_router<I>(
input: I,
channel_count: u16,
channel_map: ChannelMap,
) -> (ChannelRouterController, ChannelRouterSource<I>)
where
I: Source,
I::Item: Sample,
{
ChannelRouterSource::new(input, channel_count, channel_map)
}

struct ChannelRouterMessage(usize, usize, f32);

/// `ChannelRouterController::map()` returns this error if the router source has been dropped.
#[derive(Debug, Eq, PartialEq)]
pub struct ChannelRouterControllerError {}

/// A controller type that sends gain updates to a corresponding [`ChannelRouterSource`].
#[derive(Debug, Clone)]
pub struct ChannelRouterController {
sender: Sender<ChannelRouterMessage>,
}

impl ChannelRouterController {
/// Set or update the gain setting for a channel mapping.
///
/// A channel from the input may be routed to any number of channels in the output, and a
/// channel in the output may be a mix of any number of channels in the input.
///
/// Successive calls to `map` with the same `from` and `to` arguments will replace the
/// previous gain value with the new one.
pub fn map(
&mut self,
from: u16,
to: u16,
gain: f32,
) -> Result<(), ChannelRouterControllerError> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

❓ If the routing has to change one will have to remember to clear all the gains that are not needed anymore.
Maybe a "reset" function that sets all the gains to zero would be helpful.

❓ I wonder if we should allow specifying matrix at the initialization time, or let users to pass a list of coefficients.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably that is why I wanted to set the whole gain set at once: users may have the matrix on their side process and inspect it a they prefer and then use its to update the router.

if self
.sender
.send(ChannelRouterMessage(from as usize, to as usize, gain))
.is_err()
{
Err(ChannelRouterControllerError {})
} else {
Ok(())
}
}
}

/// A source for extracting, reordering, mixing and duplicating audio between
/// channels.
#[derive(Debug)]
pub struct ChannelRouterSource<I>
where
I: Source,
I::Item: Sample,
{
/// Input [`Source`]
input: I,

/// Mapping of input to output channels
channel_map: ChannelMap,

/// The output channel that [`next()`] will return next.
current_channel: u16,

/// The number of output channels
channel_count: u16,

/// The current input audio frame
input_buffer: Vec<I::Item>,

/// Communication channel with the controller
receiver: Receiver<ChannelRouterMessage>,
}

impl<I> ChannelRouterSource<I>
where
I: Source,
I::Item: Sample,
{
/// Creates a new [`ChannelRouter<I>`].
///
/// The new `ChannelRouter` will read samples from `input` and will mix and map them according
/// to `channel_mappings` into its output samples.
///
/// # Panics
///
/// - if `channel_count` is not equal to `channel_map`'s second dimension
/// - if `input.channels()` is not equal to `channel_map`'s first dimension
pub fn new(
input: I,
channel_count: u16,
channel_map: ChannelMap,
) -> (ChannelRouterController, Self) {
assert!(channel_count as usize == channel_map[0].len());
assert!(input.channels() as usize == channel_map.len());

let (tx, rx) = channel();

let controller = ChannelRouterController { sender: tx };
let source = Self {
input,
channel_map,
current_channel: channel_count,
// this will cause the input buffer to fill on first call to next()
channel_count,
// channel_count is redundant, it's implicit in the channel_map dimensions
// but maybe it's saving us some time, we do check this value a lot.
input_buffer: vec![],
receiver: rx,
};

(controller, source)
}

/// Destroys this router and returns the underlying source.
#[inline]
pub fn into_inner(self) -> I {
self.input
}

/// Get mutable access to the inner source.
#[inline]
pub fn inner_mut(&mut self) -> &mut I {
&mut self.input
}
}

impl<I> Source for ChannelRouterSource<I>
where
I: Source,
I::Item: Sample,
{
#[inline]
fn current_frame_len(&self) -> Option<usize> {
self.input.current_frame_len()
}

#[inline]
fn channels(&self) -> u16 {
self.channel_count
}

#[inline]
fn sample_rate(&self) -> u32 {
self.input.sample_rate()
}

#[inline]
fn total_duration(&self) -> Option<std::time::Duration> {
self.input.total_duration()
}
}

impl<I> Iterator for ChannelRouterSource<I>
where
I: Source,
I::Item: Sample,
{
type Item = I::Item;

#[inline]
fn next(&mut self) -> Option<Self::Item> {
if self.current_channel >= self.channel_count {
// We've reached the end of the frame, time to grab another one from the input
let input_channels = self.input.channels() as usize;

// This might be too fussy, a source should never break a frame in the middle of an
// audio frame.
let samples_to_take = min(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, I'd prefer to require all the sources to produce only complete frames. But I think it is not actually enforced anywhere so far. This can be an assertion, though.

input_channels,
self.input.current_frame_len().unwrap_or(usize::MAX),
);

// fill the input buffer. If the input is exhausted and returning None this will make
// the input buffer zero length
self.input_buffer = self.inner_mut().take(samples_to_take).collect();

self.current_channel = 0;

for change in self.receiver.try_iter() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, it may do as first version, but polling channel is relatively expensive. In other places an additional flag is used to skip the loop altogether when there are no changes (e.g. mixer::Mixer::has_pending). See also #658.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This only checks the receiver once if there are no changes in it right? Is a single channel poll slower then checking an atomic flag?

edit: Nevermind I checked the source for mpsc and its kinda complex. Still I would like to have a benchmark for this source before making such a change. If only to enjoy the speedup, but it might also show there to be no effect. Maybe that complexity for mpsc compiles down to something real lean?

self.channel_map[change.0][change.1] = change.2;
}
}

// Find the output sample for current_channel
let retval = self
.input_buffer
.iter()
.zip(&self.channel_map)
.map(|(in_sample, input_gains)| {
// the way this works, the input_buffer need not be totally full, the router will
// work with whatever samples are available and the missing samples will be assumed
// to be equilibrium.
let gain = input_gains[self.current_channel as usize];
in_sample.amplify(gain)
})
.reduce(|a, b| a.saturating_add(b));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this is real-time I'd prefer this to be more explicit. Maybe the matrix should be transposed to streamline this. This way instead of zip it would select a gain vector and then do the dot product of coefficients by input_buffer (it can be even optimized with SIMD).

Copy link
Collaborator

Choose a reason for hiding this comment

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

I do not want to overburden @iluvcapra, this is already quite a big effort! So we can always leave optimizations for later and merge this first. Someone else can work on them too (maybe @PetrGlad). Let me know what you prefer @iluvcapra.


self.current_channel += 1;
retval
}

#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
self.input.size_hint()
}
}

#[cfg(test)]
mod tests {
use crate::buffer::SamplesBuffer;
use crate::source::channel_router::*;

#[test]
fn test_stereo_to_mono() {
let input = SamplesBuffer::new(2, 1, [0u16, 2u16, 4u16, 6u16]);
let map = vec![vec![0.5f32], vec![0.5f32]];

let (_, test_source) = ChannelRouterSource::new(input, 1, map);
let v1: Vec<u16> = test_source.take(4).collect();
assert_eq!(v1.len(), 2);
assert_eq!(v1[0], 1u16);
assert_eq!(v1[1], 5u16);
}

#[test]
fn test_upmix() {
let input = SamplesBuffer::new(1, 1, [0i16, -10, 10, 20, -20, -50, -30, 40]);
let map = vec![vec![1.0f32, 0.5f32, 2.0f32]];
let (_, test_source) = ChannelRouterSource::new(input, 3, map);
assert_eq!(test_source.channels(), 3);
let v1: Vec<i16> = test_source.take(1000).collect();
assert_eq!(v1.len(), 24);
assert_eq!(
v1,
[
0i16, 0, 0, -10, -5, -20, 10, 5, 20, 20, 10, 40, -20, -10, -40, -50, -25, -100,
-30, -15, -60, 40, 20, 80
]
);
}

#[test]
fn test_updates() {
let input = SamplesBuffer::new(2, 1, [0i16, 0i16, -1i16, -1i16, 1i16, 2i16, -4i16, -3i16]);
let initial_map = vec![vec![1.0f32], vec![1.0f32]];
let (mut controller, mut source) = ChannelRouterSource::new(input, 1, initial_map);
let v1: Vec<i16> = source.by_ref().take(2).collect();
assert_eq!(v1.len(), 2);
assert_eq!(v1[0], 0i16);
assert_eq!(v1[1], -2i16);

let r1 = controller.map(0, 0, 0.0f32);
let r2 = controller.map(1, 0, 2.0f32);
assert_eq!(r1, Ok(()));
assert_eq!(r2, Ok(()));

let v2: Vec<i16> = source.take(3).collect();
assert_eq!(v2.len(), 2);

assert_eq!(v2[0], 4i16);
assert_eq!(v2[1], -6i16);
}
}
Loading
Loading