Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 13 additions & 2 deletions crates/media-info/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
}

impl AudioInfo {
pub const MAX_AUDIO_CHANNELS: u16 = 2;
pub const MAX_AUDIO_CHANNELS: u16 = 8;

pub const fn new(
sample_format: Sample,
Expand Down Expand Up @@ -62,7 +62,12 @@
SupportedBufferSize::Unknown => 1024,
};

let channels = config.channels().clamp(1, Self::MAX_AUDIO_CHANNELS);
let raw_channels = config.channels();
let channels = if Self::channel_layout_raw(raw_channels).is_some() {
raw_channels
} else {
raw_channels.min(Self::MAX_AUDIO_CHANNELS).max(1)

Check warning on line 69 in crates/media-info/src/lib.rs

View workflow job for this annotation

GitHub Actions / Clippy

clamp-like pattern without using clamp function

warning: clamp-like pattern without using clamp function --> crates/media-info/src/lib.rs:69:13 | 69 | raw_channels.min(Self::MAX_AUDIO_CHANNELS).max(1) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with clamp: `raw_channels.clamp(1, Self::MAX_AUDIO_CHANNELS)` | = note: clamp will panic if max < min = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_clamp = note: `#[warn(clippy::manual_clamp)]` on by default
};
Comment on lines +65 to +70
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Verify clamping behavior is logged for devices with >8 channels.

The clamping logic prevents panics but will silently misinterpret audio data from devices reporting more than 8 channels. For example, a 12-channel device clamped to 8 will have incorrect channel boundaries in the interleaved data, causing corruption.

Ensure this clamping scenario is logged (e.g., in microphone.rs device selection) so users understand why unsupported devices produce incorrect audio.

Run this script to check if clamping is logged:


🏁 Script executed:

#!/bin/bash
# Check if channel count clamping/validation is logged in microphone feed
rg -n -C5 'channel|clamp|MAX_AUDIO_CHANNELS' crates/recording/src/feeds/microphone.rs

Length of output: 2301


Add warning when clamping channel count
Raw channel counts outside [1, MAX_AUDIO_CHANNELS] are clamped in crates/media-info/src/lib.rs (lines 65–70) but no log records the original value. Insert a warning log there to report the original raw_channels and the clamped result.

🤖 Prompt for AI Agents
In crates/media-info/src/lib.rs around lines 65–70, when raw_channels is outside
[1, MAX_AUDIO_CHANNELS] it gets clamped but the original value isn't logged; add
a warning log that reports both the original raw_channels and the clamped
channels value before returning/using the clamped result. Use the crate’s
logging facility (e.g., log::warn! or tracing::warn!) and ensure the appropriate
macro is imported so the warning reads something like "clamping audio channel
count: raw=X -> clamped=Y" with raw and clamped values interpolated.


Self {
sample_format,
Expand Down Expand Up @@ -92,6 +97,12 @@
Some(match channels {
1 => ChannelLayout::MONO,
2 => ChannelLayout::STEREO,
3 => ChannelLayout::SURROUND,
4 => ChannelLayout::QUAD,
5 => ChannelLayout::_5POINT0,
6 => ChannelLayout::_5POINT1,
7 => ChannelLayout::_6POINT1,
8 => ChannelLayout::_7POINT1,
_ => return None,
})
}
Expand Down
58 changes: 47 additions & 11 deletions crates/recording/src/feeds/microphone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,35 +126,38 @@ impl MicrophoneFeed {
}

fn get_usable_device(device: Device) -> Option<(String, Device, SupportedStreamConfig)> {
device
let device_name_for_logging = device.name().ok();

let result = device
.supported_input_configs()
.map_err(|error| {
error!(
"Error getting supported input configs for device: {}",
error
"Error getting supported input configs for device {:?}: {}",
device_name_for_logging, error
);
error
})
.ok()
.and_then(|configs| {
let mut configs = configs.collect::<Vec<_>>();

configs.sort_by(|a, b| {
b.sample_format()
.sample_size()
.cmp(&a.sample_format().sample_size())
.then(b.max_sample_rate().cmp(&a.max_sample_rate()))
});

configs
.into_iter()
.filter(|c| c.min_sample_rate().0 <= 48000 && c.max_sample_rate().0 <= 48000)
.filter(|c| c.min_sample_rate().0 <= 48000 && c.max_sample_rate().0 >= 48000)
.find(|c| ffmpeg_sample_format_for(c.sample_format()).is_some())
})
.and_then(|config| {
device
.name()
.ok()
.map(|name| (name, device, config.with_max_sample_rate()))
})
});

result.and_then(|config| {
let final_config = config.with_sample_rate(cpal::SampleRate(48000));
device.name().ok().map(|name| (name, device, final_config))
})
}

#[derive(Reply)]
Expand Down Expand Up @@ -290,13 +293,46 @@ impl Message<SetInput> for MicrophoneFeed {

std::thread::spawn({
let config = config.clone();
let device_name_for_log = device.name().ok();
move || {
// Log all configs for debugging
if let Some(ref name) = device_name_for_log {
info!("Device '{}' available configs:", name);
for config in device.supported_input_configs().into_iter().flatten() {
info!(
" Format: {:?}, Min rate: {}, Max rate: {}, Sample size: {}",
config.sample_format(),
config.min_sample_rate().0,
config.max_sample_rate().0,
config.sample_format().sample_size()
);
}
}

info!(
"🎤 Building stream for '{:?}' with config: rate={}, channels={}, format={:?}",
device_name_for_log,
config.sample_rate().0,
config.channels(),
sample_format
);

let stream = match device.build_input_stream_raw(
&config.into(),
sample_format,
{
let actor_ref = actor_ref.clone();
let mut callback_count = 0u64;
move |data, info| {
if callback_count == 0 {
info!(
"🎤 First audio callback - data size: {} bytes, format: {:?}",
data.bytes().len(),
data.sample_format()
);
}
callback_count += 1;

let _ = actor_ref
.tell(MicrophoneSamples {
data: data.bytes().to_vec(),
Expand Down
Loading