-
-
Notifications
You must be signed in to change notification settings - Fork 368
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
Add encode-microphone example #291
base: master
Are you sure you want to change the base?
Conversation
CI failure seems unrelated to this change (?) |
@@ -31,6 +31,11 @@ serde_json = "1.0" | |||
bytes = "1.1" | |||
lazy_static = "1.4" | |||
rand = "0.8" | |||
# encode-microphone | |||
cpal = "0.14.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would drop the patch and stick to major and minor like most of the other lines
|
||
#[tokio::main] | ||
async fn main() -> Result<()> { | ||
let mut app = Command::new("encode-microphone") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check out clap derive, it's pretty nice.
// Create a MediaEngine object to configure the supported codec | ||
let mut m = MediaEngine::default(); | ||
|
||
m.register_default_codecs()?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since this is very opus specific maybe only register Opus
Result::<()>::Ok(()) | ||
}); | ||
|
||
let (sender, frame_receiver) = flume::bounded::<AudioFrame>(3); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you need to add extra dependency on flume here or can you just use tokio channels?
let (encoded_sender, encoded_receiver) = flume::bounded::<AudioEncodedFrame>(3); | ||
|
||
// Encoder thread | ||
thread::spawn(move || { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe add some commandline option here with default 48000kHz but it can be overriden in case there is stereo mic with 16kHz sampling rate for example?
let (encoded_sender, encoded_receiver) = flume::bounded::<AudioEncodedFrame>(3); | ||
|
||
// Encoder thread | ||
thread::spawn(move || { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you need a separate thread for encoder only? can't this run on the same thread as rtp send?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’d like that too, but since Encoder didn’t impl Send I guess it wasn’t possible to use it across awaits in an async thread. Is there a solution?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure, (I am not a very good rust async programmer :) )
eprintln!("an error occurred on stream: {}", err); | ||
}; | ||
|
||
// until it is 960 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make a comment what 960 is, my guess it's 20ms frame with 48kHz sampled data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, I should replace it with a const and a comment.
}; | ||
|
||
// until it is 960 | ||
let mut buffer: Vec<f32> = Vec::new(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with_capacity to preallocate all memory
#[cfg(any( | ||
not(any(target_os = "linux", target_os = "dragonfly", target_os = "freebsd")), | ||
not(feature = "jack") | ||
))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can probably be removed
let host = cpal::default_host(); | ||
|
||
// Set up the input device and stream with the default input config. | ||
let device = if device == "default" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since you already hardcode device on 316 the else is not relevant here.
@morajabi I'm holding of on reviewing this until CI passes FYI |
No description provided.