Skip to content

fix: clamp opus rate selection for high input #1268

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Oct 22, 2025
Merged

fix: clamp opus rate selection for high input #1268

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fa35769
fix: clamp opus rate selection for high input
richiemcilroy Oct 21, 2025
e8003f5
fix tests
Brendonovich Oct 22, 2025
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
16 changes: 5 additions & 11 deletions crates/enc-ffmpeg/src/audio/buffered_resampler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
pts += buffer.0.samples() as i64;
}

return remaining_samples;

Check warning on line 51 in crates/enc-ffmpeg/src/audio/buffered_resampler.rs

View workflow job for this annotation

GitHub Actions / Clippy

unneeded `return` statement 

warning: unneeded `return` statement
  --> crates/enc-ffmpeg/src/audio/buffered_resampler.rs:51:9
   |
51 |         return remaining_samples;
   |         ^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_return
   = note: `#[warn(clippy::needless_return)]` on by default
help: remove `return`
   |
51 -         return remaining_samples;
51 +         remaining_samples
   |
}

pub fn output(&self) -> resampling::context::Definition {
Expand All @@ -56,11 +56,11 @@
}

pub fn add_frame(&mut self, mut frame: ffmpeg::frame::Audio) {
if let Some(min_next_pts) = self.min_next_pts {
if let Some(pts) = frame.pts() {
frame.set_pts(Some(pts.max(min_next_pts)));
}
}

Check warning on line 63 in crates/enc-ffmpeg/src/audio/buffered_resampler.rs

View workflow job for this annotation

GitHub Actions / Clippy

this `if` statement can be collapsed 

warning: this `if` statement can be collapsed
  --> crates/enc-ffmpeg/src/audio/buffered_resampler.rs:59:9
   |
59 | /         if let Some(min_next_pts) = self.min_next_pts {
60 | |             if let Some(pts) = frame.pts() {
61 | |                 frame.set_pts(Some(pts.max(min_next_pts)));
62 | |             }
63 | |         }
   | |_________^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_if
   = note: `#[warn(clippy::collapsible_if)]` on by default
help: collapse nested if block
   |
59 ~         if let Some(min_next_pts) = self.min_next_pts
60 ~             && let Some(pts) = frame.pts() {
61 |                 frame.set_pts(Some(pts.max(min_next_pts)));
62 ~             }
   |

let pts = frame.pts().unwrap();

Expand All @@ -75,7 +75,7 @@

self.buffer.push_back((resampled_frame, resampled_pts));

while let Some(_) = self.resampler.delay() {

Check warning on line 78 in crates/enc-ffmpeg/src/audio/buffered_resampler.rs

View workflow job for this annotation

GitHub Actions / Clippy

redundant pattern matching, consider using `is_some()` 

warning: redundant pattern matching, consider using `is_some()`
  --> crates/enc-ffmpeg/src/audio/buffered_resampler.rs:78:19
   |
78 |         while let Some(_) = self.resampler.delay() {
   |         ----------^^^^^^^------------------------- help: try: `while self.resampler.delay().is_some()`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern_matching
   = note: `#[warn(clippy::redundant_pattern_matching)]` on by default
let mut resampled_frame = ffmpeg::frame::Audio::new(
self.resampler.output().format,
0,
Expand All @@ -89,7 +89,7 @@

self.buffer.push_back((resampled_frame, next_pts));

next_pts = next_pts + samples as i64;

Check warning on line 92 in crates/enc-ffmpeg/src/audio/buffered_resampler.rs

View workflow job for this annotation

GitHub Actions / Clippy

manual implementation of an assign operation 

warning: manual implementation of an assign operation
  --> crates/enc-ffmpeg/src/audio/buffered_resampler.rs:92:13
   |
92 |             next_pts = next_pts + samples as i64;
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `next_pts += samples as i64`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#assign_op_pattern
   = note: `#[warn(clippy::assign_op_pattern)]` on by default
}

self.min_next_pts = Some(pts + frame.samples() as i64);
Expand Down Expand Up @@ -279,21 +279,15 @@
const IN_RATE: u32 = 100;

fn create_resampler(out_rate: u32) -> BufferedResampler {
let resampler = ffmpeg::software::resampler(
(
format::Sample::U8(cap_media_info::Type::Packed),
ChannelLayout::MONO,
IN_RATE,
),
(
BufferedResampler::new(
AudioInfo::new_raw(format::Sample::U8(cap_media_info::Type::Packed), IN_RATE, 1),
AudioInfo::new_raw(
format::Sample::U8(cap_media_info::Type::Packed),
ChannelLayout::MONO,
out_rate,
1,
),
)
.unwrap();

BufferedResampler::new(resampler)
.unwrap()
}

fn make_input_frame(samples: usize, pts: i64) -> ffmpeg::frame::Audio {
Expand Down
41 changes: 33 additions & 8 deletions crates/enc-ffmpeg/src/audio/opus.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,8 @@ impl OpusEncoder {
.collect::<Vec<_>>();
rates.sort();

let Some(&rate) = rates
.iter()
.find(|r| **r >= input_config.rate())
.or(rates.first())
else {
return Err(OpusEncoderError::RateNotSupported(input_config.rate()));
};
rate
select_output_rate(input_config.rate(), &rates)
.ok_or(OpusEncoderError::RateNotSupported(input_config.rate()))?
};

let mut output_config = input_config;
Expand Down Expand Up @@ -108,6 +102,37 @@ impl OpusEncoder {
}
}

fn select_output_rate(input_rate: i32, supported_rates: &[i32]) -> Option<i32> {
supported_rates
.iter()
.copied()
.find(|&rate| rate >= input_rate)
.or_else(|| supported_rates.iter().copied().max())
}

#[cfg(test)]
mod tests {
use super::select_output_rate;

#[test]
fn chooses_matching_rate_when_available() {
let supported = [8_000, 12_000, 16_000, 24_000, 48_000];
assert_eq!(select_output_rate(16_000, &supported), Some(16_000));
}

#[test]
fn clamps_to_highest_supported_rate_when_input_is_higher() {
let supported = [8_000, 12_000, 16_000, 24_000, 48_000];
assert_eq!(select_output_rate(96_000, &supported), Some(48_000));
}

#[test]
fn clamps_to_lowest_supported_rate_when_input_is_lower() {
let supported = [8_000, 12_000, 16_000, 24_000, 48_000];
assert_eq!(select_output_rate(4_000, &supported), Some(8_000));
}
}

impl AudioEncoder for OpusEncoder {
fn send_frame(&mut self, frame: frame::Audio, output: &mut format::context::Output) {
let _ = self.queue_frame(frame, Duration::MAX, output);
Expand Down
Loading