Open bitbegin opened 4 years ago
I did some digging on this and it seems our options for turning this logging off altogether are pretty limited. A proposed solution would be to wrap the relevant calls to ASLA lib with something like https://crates.io/crates/gag
https://chromium.googlesource.com/chromium/src/media/+/master/audio/alsa/audio_manager_alsa.cc#209
should be fixed like this way
What about snd_lib_error_set_handler
?
I gave snd_lib_error_set_handler a quick test.
#[no_mangle]
unsafe extern "C" fn error_handler(
file: *const ::std::os::raw::c_char,
line: ::std::os::raw::c_int,
function: *const ::std::os::raw::c_char,
err: ::std::os::raw::c_int,
fmt: *const ::std::os::raw::c_char,
mut args: ...
) {
dbg!(file, line, function, err, fmt);
}
unsafe {
alsa_sys::snd_lib_error_set_handler(Some(error_handler));
}
[src/main.rs:64:9] file = 0x00007a90c2fe2009
[src/main.rs:64:9] line = 5703
[src/main.rs:64:9] function = 0x00007a90c2fe26a0
[src/main.rs:64:9] err = 0
[src/main.rs:64:9] fmt = 0x00007a90c2fe2356
But that requires c_variadic from nightly.
The following seems to work in stable by transmuting the function pointer. I don't understand enough about unsafe rust or c FFI to know whether that's safe. It also limits access to the variadic argumets. The latter doesn't matter if it's only used for silencing the warnings.
pub fn silence_alsa() {
#[cfg(target_os = "linux")]
{
unsafe extern "C" fn error_handler(
file: *const ::std::os::raw::c_char,
line: ::std::os::raw::c_int,
function: *const ::std::os::raw::c_char,
err: ::std::os::raw::c_int,
fmt: *const ::std::os::raw::c_char,
// mut args: ...
) {
let file_str = unsafe { std::ffi::CStr::from_ptr(file).to_string_lossy() };
let function_str = unsafe { std::ffi::CStr::from_ptr(function).to_string_lossy() };
let fmt_str = unsafe { std::ffi::CStr::from_ptr(fmt).to_string_lossy() };
log::warn!("{file_str}:{line} {function_str} {err} {fmt_str}");
}
unsafe {
let handler: alsa_sys::snd_lib_error_handler_t =
Some(std::mem::transmute(error_handler as *const ()));
alsa_sys::snd_lib_error_set_handler(handler);
}
}
}
It's also global, so I'm not sure if it's sensible to be included in a crate like cpal to begin with. Still might be useful for someone else coming across this.
It looks like there is also snd_lib_error_set_local which is at least thread local and takes precedence over the global handler, but other than the Stream all other alsa operations seem to be executed from the calling thread and spawning random threads just to set a local error handler seems silly as well.
The following seems to work in stable by transmuting the function pointer.
I don't think this is sound, at least portably. What should work is writing a wrapper in C with a non-variadic interface.
It's also global, so I'm not sure if it's sensible to be included in a crate like cpal to begin with.
I think it's reasonable to have. You don't see a lot of applications using multiple independent sound output libraries concurrently. Could always include some sort of escape hatch to disable it if someone really wants.
@Ralith I think you are right. It might just be ok with the system-v x86_64 calling convention but in general all bets are off. I replaced it with a little C shim in my project now. Mostly as an excuse to gain some FFI experience with rust.
With this approach there is a build time dependency on the CC crate and some unsafe code. So it's all a bit messy.
I see three ways forward:
Either way I would need someone with more current C experience and experience with unsafe rust and C FFI to give me a review. I'm currently not confident enough that what I do is sound to release it.
@mitchmindtree since you've triaged this issue in the past, any input on how to move forward?
when the device can't be opened, it will print logs like
ALSA lib