Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Add encode-microphone example #291

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

Open
morajabi wants to merge 2 commits into webrtc-rs:master
base: master
Choose a base branch
Loading
from morajabi:master
Open

Conversation

@morajabi
Copy link
Contributor

@morajabi morajabi commented Sep 12, 2022

No description provided.

Copy link
Contributor Author

CI failure seems unrelated to this change (?)

lazy_static = "1.4"
rand = "0.8"
# encode-microphone
cpal = "0.14.0"
Copy link
Contributor

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

melekes reacted with thumbs up emoji

#[tokio::main]
async fn main() -> Result<()> {
let mut app = Command::new("encode-microphone")
Copy link
Contributor

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()?;
Copy link
Contributor

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);
Copy link
Contributor

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 || {
Copy link
Contributor

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 || {
Copy link
Contributor

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?

Copy link
Contributor Author

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?

Copy link
Contributor

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
Copy link
Contributor

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.

Copy link
Contributor Author

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();
Copy link
Contributor

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")
))]
Copy link
Contributor

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" {
Copy link
Contributor

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.

Copy link
Member

k0nserv commented Oct 6, 2022

@morajabi I'm holding of on reviewing this until CI passes FYI

Copy link
Contributor

melekes commented Nov 22, 2022

@morajabi friendly ping. Will you have time to address @xnorpx feedback?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@xnorpx xnorpx xnorpx left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /