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
2 changes: 0 additions & 2 deletions sherpa-onnx/rust/sherpa-onnx/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ mod online_asr;
mod online_punctuation;
mod online_speech_denoiser;
mod speaker_embedding;
mod speech_denoiser;
mod tts;
mod utils;
mod vad;
Comment on lines 7 to 12

Copilot AI Mar 20, 2026

Copy link

Choose a reason for hiding this comment

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

offline_speech_denoiser.rs exists and online_speech_denoiser.rs now imports from crate::offline_speech_denoiser::..., but lib.rs does not declare mod offline_speech_denoiser; in the module list shown here. This will fail to compile with an unresolved import unless the module is declared elsewhere. Add mod offline_speech_denoiser; (and consider whether it needs to be pub/re-exported depending on intended public API).

Copilot uses AI. Check for mistakes.
Expand All @@ -22,7 +21,6 @@ pub use online_asr::*;
pub use online_punctuation::*;
pub use online_speech_denoiser::*;
pub use speaker_embedding::*;
pub use speech_denoiser::*;
pub use tts::*;
pub use utils::*;
pub use vad::*;
Comment on lines 21 to 26

Copilot AI Mar 20, 2026

Copy link

Choose a reason for hiding this comment

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

This PR removes pub use speech_denoiser::*; (and the entire speech_denoiser module), which is a breaking change for downstream crates importing sherpa_onnx::speech_denoiser::* or relying on those re-exports. If the intent is a non-breaking refactor, consider keeping a compatibility shim (e.g., reintroduce speech_denoiser as a module that re-exports the moved items, possibly marked #[deprecated]), or re-export the moved items from the crate root to preserve prior paths.

Copilot uses AI. Check for mistakes.
Expand Down
100 changes: 99 additions & 1 deletion sherpa-onnx/rust/sherpa-onnx/src/offline_speech_denoiser.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,105 @@
use crate::speech_denoiser::{DenoisedAudio, OfflineSpeechDenoiserModelConfig};
use crate::utils::to_c_ptr;
use sherpa_onnx_sys as sys;
use std::ffi::CString;
use std::ptr;
use std::slice;

#[derive(Clone, Debug, Default)]
pub struct OfflineSpeechDenoiserGtcrnModelConfig {
pub model: Option<String>,
}

impl OfflineSpeechDenoiserGtcrnModelConfig {
pub(crate) fn to_sys(
&self,
cstrings: &mut Vec<CString>,
) -> sys::OfflineSpeechDenoiserGtcrnModelConfig {
sys::OfflineSpeechDenoiserGtcrnModelConfig {
model: to_c_ptr(&self.model, cstrings),
}
}
}

#[derive(Clone, Debug, Default)]
pub struct OfflineSpeechDenoiserDpdfNetModelConfig {
pub model: Option<String>,
}

impl OfflineSpeechDenoiserDpdfNetModelConfig {
pub(crate) fn to_sys(
&self,
cstrings: &mut Vec<CString>,
) -> sys::OfflineSpeechDenoiserDpdfNetModelConfig {
sys::OfflineSpeechDenoiserDpdfNetModelConfig {
model: to_c_ptr(&self.model, cstrings),
}
}
}
Comment on lines +8 to +37

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The structs OfflineSpeechDenoiserGtcrnModelConfig and OfflineSpeechDenoiserDpdfNetModelConfig and their to_sys implementations are identical. You can reduce code duplication by using a macro to generate these structs and their implementations. This will make the code more concise and easier to maintain.

macro_rules! define_denoiser_model_config {
    ($name:ident, $sys_name:ty) => {
        #[derive(Clone, Debug, Default)]
        pub struct $name {
            pub model: Option<String>,
        }

        impl $name {
            pub(crate) fn to_sys(
                &self,
                cstrings: &mut Vec<CString>,
            ) -> $sys_name {
                <$sys_name> {
                    model: to_c_ptr(&self.model, cstrings),
                }
            }
        }
    };
}

define_denoiser_model_config!(
    OfflineSpeechDenoiserGtcrnModelConfig,
    sys::OfflineSpeechDenoiserGtcrnModelConfig
);

define_denoiser_model_config!(
    OfflineSpeechDenoiserDpdfNetModelConfig,
    sys::OfflineSpeechDenoiserDpdfNetModelConfig
);


#[derive(Clone, Debug)]
pub struct OfflineSpeechDenoiserModelConfig {
pub gtcrn: OfflineSpeechDenoiserGtcrnModelConfig,
pub dpdfnet: OfflineSpeechDenoiserDpdfNetModelConfig,
pub num_threads: i32,
pub debug: bool,
pub provider: Option<String>,
}

impl Default for OfflineSpeechDenoiserModelConfig {
fn default() -> Self {
Self {
gtcrn: Default::default(),
dpdfnet: Default::default(),
num_threads: 1,
debug: false,
provider: Some("cpu".to_string()),
}
}
}

impl OfflineSpeechDenoiserModelConfig {
pub(crate) fn to_sys(
&self,
cstrings: &mut Vec<CString>,
) -> sys::OfflineSpeechDenoiserModelConfig {
sys::OfflineSpeechDenoiserModelConfig {
gtcrn: self.gtcrn.to_sys(cstrings),
num_threads: self.num_threads,
debug: self.debug as i32,
provider: to_c_ptr(&self.provider, cstrings),
dpdfnet: self.dpdfnet.to_sys(cstrings),
}
}
}

#[derive(Clone, Debug, Default)]
pub struct DenoisedAudio {
pub samples: Vec<f32>,
pub sample_rate: i32,
}

impl DenoisedAudio {
pub(crate) fn from_ptr(ptr: *const sys::DenoisedAudio) -> Self {
if ptr.is_null() {
return Self::default();
}

unsafe {
let n = (*ptr).n.max(0) as usize;
let samples = if (*ptr).samples.is_null() || n == 0 {
vec![]
} else {
slice::from_raw_parts((*ptr).samples, n).to_vec()
};
let sample_rate = (*ptr).sample_rate;
sys::SherpaOnnxDestroyDenoisedAudio(ptr);
Self {
samples,
sample_rate,
}
}
}
Comment on lines +82 to +101

Copilot AI Mar 20, 2026

Copy link

Choose a reason for hiding this comment

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

from_ptr always calls SherpaOnnxDestroyDenoisedAudio(ptr), so it takes ownership of the incoming pointer. That ownership contract isn’t obvious from the name/signature (especially since it accepts *const). To reduce the risk of accidental double-free/use-after-free by future callers, consider renaming to something that encodes ownership (e.g., from_owned_ptr), adding a brief doc comment stating it consumes/frees the pointer, and/or switching the parameter type to *mut sys::DenoisedAudio if the C API expects a mutable pointer for destroy.

Copilot uses AI. Check for mistakes.
}

#[derive(Clone, Debug, Default)]
pub struct OfflineSpeechDenoiserConfig {
Expand Down
2 changes: 1 addition & 1 deletion sherpa-onnx/rust/sherpa-onnx/src/online_speech_denoiser.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::speech_denoiser::{DenoisedAudio, OfflineSpeechDenoiserModelConfig};
use crate::offline_speech_denoiser::{DenoisedAudio, OfflineSpeechDenoiserModelConfig};
use sherpa_onnx_sys as sys;
use std::ffi::CString;
use std::ptr;
Expand Down
98 changes: 0 additions & 98 deletions sherpa-onnx/rust/sherpa-onnx/src/speech_denoiser.rs

This file was deleted.

Loading