Add Swift API for source separation - #3426
Conversation
dosubot
Bot
added
the
size:L
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly expands the Swift API by integrating offline audio source separation capabilities. It provides developers with tools to easily separate audio into different stems (e.g., vocals and accompaniment) using both Spleeter and UVR models, enhancing the framework's utility for audio processing applications. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds Swift bindings and example executables for offline source separation (Spleeter and UVR), build/run helper scripts, test pipeline integration, and .gitignore updates; implements AudioBuffer, SourceSeparationConfig, and SourceSeparator in Swift to read WAVs, run separation via the C API, and save stems. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Swift App
participant Engine as SourceSeparator (C Engine)
participant Model as ONNX Model (Spleeter/UVR)
participant IO as File I/O
App->>IO: load WAV (qi-feng-le-zh.wav)
IO-->>App: AudioBuffer (interleaved multi-channel)
App->>Engine: process(buffer: AudioBuffer)
Engine->>Model: run separation (ONNX inference)
Model-->>Engine: separated stems (C output)
Engine-->>App: [AudioBuffer] (owned stems)
loop per stem
App->>IO: save stem -> vocals.wav / accompaniment.wav
IO-->>App: write result
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces Swift API wrappers and example scripts for source separation using Spleeter and UVR models. The changes include new Swift classes (SherpaOnnxMultiChannelWaveWrapper, SherpaOnnxSourceSeparationOutputWrapper, SherpaOnnxOfflineSourceSeparationWrapper) to interface with the underlying C library, along with shell scripts to build and run these examples. Review feedback focuses on improving the robustness and idiomatic Swift usage of these wrappers, specifically by advocating for failable initializers and non-optional properties to handle C pointers that can be null, simplifying deinitialization and property access, and updating the example code to properly handle optional return values. Minor improvements were also suggested for the shell scripts to use rm -f for safer file cleanup.
|
|
||
| ./run-source-separation-spleeter.sh | ||
| rm -rf sherpa-onnx-spleeter-* | ||
| rm vocals.wav accompaniment.wav |
There was a problem hiding this comment.
Using rm -f is safer as it won't cause an error if the files don't exist, making the script more robust, especially since set -e is active.
| rm vocals.wav accompaniment.wav | |
| rm -f vocals.wav accompaniment.wav |
|
|
||
| ./run-source-separation-uvr.sh | ||
| rm -rf UVR-MDX-NET-Voc_FT.onnx | ||
| rm uvr-vocals.wav uvr-non-vocals.wav |
There was a problem hiding this comment.
Using rm -f is safer as it won't cause an error if the files don't exist, making the script more robust, especially since set -e is active.
| rm uvr-vocals.wav uvr-non-vocals.wav | |
| rm -f uvr-vocals.wav uvr-non-vocals.wav |
| let wave: UnsafePointer<SherpaOnnxMultiChannelWave>! | ||
|
|
||
| init(wave: UnsafePointer<SherpaOnnxMultiChannelWave>!) { | ||
| self.wave = wave | ||
| } |
There was a problem hiding this comment.
Using an implicitly unwrapped optional (!) for a C pointer that can be nil is unsafe. It's better to use a failable initializer (init?) and a non-optional property. This makes the code safer and more idiomatic in Swift.
| let wave: UnsafePointer<SherpaOnnxMultiChannelWave>! | |
| init(wave: UnsafePointer<SherpaOnnxMultiChannelWave>!) { | |
| self.wave = wave | |
| } | |
| let wave: UnsafePointer<SherpaOnnxMultiChannelWave> | |
| init?(wave: UnsafePointer<SherpaOnnxMultiChannelWave>?) { | |
| guard let wave = wave else { return nil } | |
| self.wave = wave | |
| } |
| deinit { | ||
| if let wave { | ||
| SherpaOnnxFreeMultiChannelWave(wave) | ||
| } | ||
| } | ||
|
|
||
| var numChannels: Int32 { | ||
| guard let wave else { return 0 } | ||
| return wave.pointee.num_channels | ||
| } | ||
|
|
||
| var numSamples: Int32 { | ||
| guard let wave else { return 0 } | ||
| return wave.pointee.num_samples | ||
| } | ||
|
|
||
| var sampleRate: Int32 { | ||
| guard let wave else { return 0 } | ||
| return wave.pointee.sample_rate | ||
| } |
There was a problem hiding this comment.
Following the change to a non-optional wave property, the deinit and computed properties can be simplified. The guard let checks are no longer necessary, which makes the code cleaner and more efficient.
| deinit { | |
| if let wave { | |
| SherpaOnnxFreeMultiChannelWave(wave) | |
| } | |
| } | |
| var numChannels: Int32 { | |
| guard let wave else { return 0 } | |
| return wave.pointee.num_channels | |
| } | |
| var numSamples: Int32 { | |
| guard let wave else { return 0 } | |
| return wave.pointee.num_samples | |
| } | |
| var sampleRate: Int32 { | |
| guard let wave else { return 0 } | |
| return wave.pointee.sample_rate | |
| } | |
| deinit { | |
| SherpaOnnxFreeMultiChannelWave(wave) | |
| } | |
| var numChannels: Int32 { | |
| return wave.pointee.num_channels | |
| } | |
| var numSamples: Int32 { | |
| return wave.pointee.num_samples | |
| } | |
| var sampleRate: Int32 { | |
| return wave.pointee.sample_rate | |
| } |
| let output: UnsafePointer<SherpaOnnxSourceSeparationOutput>! | ||
|
|
||
| init(output: UnsafePointer<SherpaOnnxSourceSeparationOutput>!) { | ||
| self.output = output | ||
| } |
There was a problem hiding this comment.
Similar to SherpaOnnxMultiChannelWaveWrapper, it's safer to use a failable initializer and a non-optional property for output since the underlying C function can return nil.
| let output: UnsafePointer<SherpaOnnxSourceSeparationOutput>! | |
| init(output: UnsafePointer<SherpaOnnxSourceSeparationOutput>!) { | |
| self.output = output | |
| } | |
| let output: UnsafePointer<SherpaOnnxSourceSeparationOutput> | |
| init?(output: UnsafePointer<SherpaOnnxSourceSeparationOutput>?) { | |
| guard let output = output else { return nil } | |
| self.output = output | |
| } |
| deinit { | ||
| if let impl { | ||
| SherpaOnnxDestroyOfflineSourceSeparation(impl) | ||
| } | ||
| } |
There was a problem hiding this comment.
With a non-optional impl property, the deinit can be simplified by removing the if let binding.
deinit {
SherpaOnnxDestroyOfflineSourceSeparation(impl)
}| static func readWave(filename: String) -> SherpaOnnxMultiChannelWaveWrapper { | ||
| let wave = SherpaOnnxReadWaveMultiChannel(toCPointer(filename)) | ||
| return SherpaOnnxMultiChannelWaveWrapper(wave: wave) | ||
| } |
There was a problem hiding this comment.
Since SherpaOnnxReadWaveMultiChannel can return nil on failure, this function should return an optional SherpaOnnxMultiChannelWaveWrapper? to propagate the failure information to the caller. This avoids creating an "empty" wrapper object that might hide errors.
| static func readWave(filename: String) -> SherpaOnnxMultiChannelWaveWrapper { | |
| let wave = SherpaOnnxReadWaveMultiChannel(toCPointer(filename)) | |
| return SherpaOnnxMultiChannelWaveWrapper(wave: wave) | |
| } | |
| static func readWave(filename: String) -> SherpaOnnxMultiChannelWaveWrapper? { | |
| let wave = SherpaOnnxReadWaveMultiChannel(toCPointer(filename)) | |
| return SherpaOnnxMultiChannelWaveWrapper(wave: wave) | |
| } |
| func process( | ||
| wave: SherpaOnnxMultiChannelWaveWrapper | ||
| ) -> SherpaOnnxSourceSeparationOutputWrapper { | ||
| guard let wavePtr = wave.wave else { | ||
| return SherpaOnnxSourceSeparationOutputWrapper(output: nil) | ||
| } | ||
| let output = SherpaOnnxOfflineSourceSeparationProcess( | ||
| impl, | ||
| wavePtr.pointee.samples, | ||
| wavePtr.pointee.num_channels, | ||
| wavePtr.pointee.num_samples, | ||
| wavePtr.pointee.sample_rate | ||
| ) | ||
| return SherpaOnnxSourceSeparationOutputWrapper(output: output) | ||
| } |
There was a problem hiding this comment.
The SherpaOnnxOfflineSourceSeparationProcess function can return nil. To handle this gracefully, the process method should return an optional SherpaOnnxSourceSeparationOutputWrapper?. Also, with the suggested changes to SherpaOnnxMultiChannelWaveWrapper, the guard check for wave.wave is no longer needed here, as the caller would have to unwrap the optional wave object.
func process(
wave: SherpaOnnxMultiChannelWaveWrapper
) -> SherpaOnnxSourceSeparationOutputWrapper? {
let wavePtr = wave.wave
let output = SherpaOnnxOfflineSourceSeparationProcess(
impl,
wavePtr.pointee.samples,
wavePtr.pointee.num_channels,
wavePtr.pointee.num_samples,
wavePtr.pointee.sample_rate
)
return SherpaOnnxSourceSeparationOutputWrapper(output: output)
}| let ss = SherpaOnnxOfflineSourceSeparationWrapper(config: &config) | ||
|
|
||
| let wave = SherpaOnnxOfflineSourceSeparationWrapper.readWave( | ||
| filename: "./qi-feng-le-zh.wav") | ||
| print( | ||
| "Input: channels=\(wave.numChannels), samples=\(wave.numSamples), sampleRate=\(wave.sampleRate)" | ||
| ) | ||
|
|
||
| let output = ss.process(wave: wave) |
There was a problem hiding this comment.
With the suggested changes to make the wrapper initializers and methods failable, this example code needs to be updated to handle the returned optionals. This makes the example more robust by properly handling potential initialization or processing failures.
guard let ss = SherpaOnnxOfflineSourceSeparationWrapper(config: &config) else {
fatalError("Failed to create SherpaOnnxOfflineSourceSeparationWrapper")
}
guard let wave = SherpaOnnxOfflineSourceSeparationWrapper.readWave(
filename: "./qi-feng-le-zh.wav") else {
fatalError("Failed to read wave file ./qi-feng-le-zh.wav")
}
print(
"Input: channels=\(wave.numChannels), samples=\(wave.numSamples), sampleRate=\(wave.sampleRate)"
)
guard let output = ss.process(wave: wave) else {
fatalError("Failed to process wave")
}| let ss = SherpaOnnxOfflineSourceSeparationWrapper(config: &config) | ||
|
|
||
| let wave = SherpaOnnxOfflineSourceSeparationWrapper.readWave( | ||
| filename: "./qi-feng-le-zh.wav") | ||
| print( | ||
| "Input: channels=\(wave.numChannels), samples=\(wave.numSamples), sampleRate=\(wave.sampleRate)" | ||
| ) | ||
|
|
||
| let output = ss.process(wave: wave) |
There was a problem hiding this comment.
With the suggested changes to make the wrapper initializers and methods failable, this example code needs to be updated to handle the returned optionals. This makes the example more robust by properly handling potential initialization or processing failures.
guard let ss = SherpaOnnxOfflineSourceSeparationWrapper(config: &config) else {
fatalError("Failed to create SherpaOnnxOfflineSourceSeparationWrapper")
}
guard let wave = SherpaOnnxOfflineSourceSeparationWrapper.readWave(
filename: "./qi-feng-le-zh.wav") else {
fatalError("Failed to read wave file ./qi-feng-le-zh.wav")
}
print(
"Input: channels=\(wave.numChannels), samples=\(wave.numSamples), sampleRate=\(wave.sampleRate)"
)
guard let output = ss.process(wave: wave) else {
fatalError("Failed to process wave")
}
1 participant
Summary by CodeRabbit
New Features
Tests
Chores