Skip to content

Conversation

@ashleyz
Copy link
Contributor

@ashleyz ashleyz commented Jun 28, 2022

Patch to expose async logging for backends. Adds cubeb_alog! / cubeb_alogv! macros to make use of it.

@ashleyz ashleyz requested a review from a team June 28, 2022 21:54
@ashleyz
Copy link
Contributor Author

ashleyz commented Jun 28, 2022

Not sure why this isn't building... I've been testing with cmake -DBUILD_RUST_LIBS=ON . and have used the new macros successfully in cubeb-pulse-rs with the WIP patch I have available here. However, on seeing the build errors here I checked cargo build from directly inside my cubeb-rs directory and was able to repro them locally, so I'll make sure I check that next time.

@ChunMinChang
Copy link
Member

Glad to see you are making progress!

I guess the corresponding cubeb changes (to expose cubeb_async_log and its friend) is on the way?

@ashleyz ashleyz marked this pull request as draft June 29, 2022 22:23
cstr
}

match($use_async) {
Copy link
Member

Choose a reason for hiding this comment

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

Since $use_async is an ident, could we use a literal for $use_async to distinguish whether the call is an async log or not (see https://doc.rust-lang.org/reference/macros-by-example.html#transcribing), which should have better readability? What do you think?

@ashleyz ashleyz requested a review from kinetiknz July 14, 2022 23:48
@ashleyz
Copy link
Contributor Author

ashleyz commented Jul 15, 2022

Build errors in nightly should be fixed with PR #72

@ashleyz ashleyz marked this pull request as ready for review July 15, 2022 00:11
@ashleyz ashleyz requested a review from padenot July 15, 2022 16:59
@ashleyz
Copy link
Contributor Author

ashleyz commented Jul 15, 2022

Related PR: mozilla/cubeb#714

Related WIP branch to use async calls with PulseAudio: ashleyz/cubeb-pulse-rs

Copy link
Contributor

@kinetiknz kinetiknz left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants