Higher-level API

[hasufell]

I was already considering once to write a pulseaudio binding, but more high-level. I still feel this binding exposes a lot of details of dealing with the threading model of pulseaudio. I was wondering if there would be a way to utilize futures, but I am not particularly proficient with those yet.

Especially the callback stuff still requires one to pass C pointers around: https://docs.rs/libpulse-binding/1.0.3/libpulse_binding/context/type.ContextNotifyCb.html

[jnqnfe]

Sorry for the long delay in responding.

It does indeed expose a lot of detail. I am not currently very familiar with the futures crate either, so I can't comment on whether it would be suitable or possible for allowing an even cleaner/more-abstract interface. Also, creating these crates was a small side project of a much broader scope of work I am undertaking, and I've yet to even put them to use myself. I will certainly bare futures in mind and come back to this.

Patches are of course welcome, if anyone wanted to submit a proposed implementation.

fyi, long term I intend to enquire whether the PA project itself would like to take over ownership and maintenance of these PA related crates of mine. Even longer term, let's hope PA itself fully converts to Rust, which would certainly allow use of futures.

[jnqnfe]

I have just published version 2.0. The biggest change is support for closures as callbacks across most of the API.

I have looked into Futures stuff a little; but not yet enough to determine definitely whether it is suitable for use here. My biggest concern currently is how execution of callbacks would work with PA's event loop ("mainloop") construct. I'll put more work into looking at this shortly, now that the callback closure stuff is out of the way (mostly, the mainloop API is the primary bit left undone).

[dianhenglau]

Hi. I would like to add some info to this issue.

I think the "closures as callbacks" does not fit when running the main loop using the callback style.

What is "running the main loop using the callback style"?

Here is how pacat.c run the main loop:

/* Code omitted... */

/* This is called whenever the context status changes */
static void context_state_callback(pa_context *c, void *userdata) {
    pa_assert(c);

    switch (pa_context_get_state(c)) {
        /* Take action based on context state... */
    }

    /* Code omitted... */
}

/* Code omitted... */

int main(int argc, char *argv[]) {
    pa_mainloop* m = NULL;

    /* Code omitted... */

    /* Set up a new main loop */
    if (!(m = pa_mainloop_new())) {
        pa_log(_("pa_mainloop_new() failed."));
        goto quit;
    }

    /* Code omitted... */

    /* Create a new connection context */
    if (!(context = pa_context_new_with_proplist(mainloop_api, NULL, proplist))) {
        /* Log error and quit... */
    }

    pa_context_set_state_callback(context, context_state_callback, NULL);

    /* Connect the context */
    if (pa_context_connect(context, server, 0, NULL) < 0) {
        /* Log error and quit... */
    }

    /* Run the main loop */
    if (pa_mainloop_run(m, &ret) < 0) {
        /* Log error and quit... */
    }

    /* Code omitted... */
}

Basically it performs major tasks in callbacks. It then registers those callbacks, and lets the main loop run by itself.

What is the problem?

I tried to follow the pattern above using libpulse_binding:

use libpulse_binding::{
    context::{self, Context},
    mainloop::standard::Mainloop,
    proplist::Proplist,
};
use std::{cell::RefCell, rc::Rc};

fn main() {
    // Set up a new main loop
    let mut mainloop = Mainloop::new().unwrap();

    // Create an empty proplist
    let proplist = Proplist::new().unwrap();

    // Create a new connection context
    let context = Rc::new(RefCell::new(
        Context::new_with_proplist(&mainloop, "Try Pulse", &proplist).unwrap(),
    ));

    context
        .borrow_mut()
        .set_state_callback(Some(context_state_callback(Rc::clone(&context))));

    // Connect the context
    context
        .borrow_mut()
        .connect(None, context::flags::NOFLAGS, None)
        .unwrap();

    // Run the main loop
    mainloop.run().unwrap();
}

/* Return a closure that will be called whenever the context status changes */
fn context_state_callback(context: Rc<RefCell<Context>>) -> Box<dyn FnMut()> {
    Box::new(move || {
        match context.borrow().get_state() {
            // Take action based on context state...
            _ => (),
        }
    })
}

When running it with backtrace enabled, I got

thread 'main' panicked at 'already mutably borrowed: BorrowError', /build/rust/src/rustc-1.46.0-src/src/libcore/cell.rs:792:27
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /build/rust/src/rustc-1.46.0-src/vendor/backtrace-0.3.46/src/backtrace/libunwind.rs:86
   1: backtrace::backtrace::trace_unsynchronized
             at /build/rust/src/rustc-1.46.0-src/vendor/backtrace-0.3.46/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print_fmt
             at src/libstd/sys_common/backtrace.rs:78
   3: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
             at src/libstd/sys_common/backtrace.rs:59
   4: core::fmt::write
             at src/libcore/fmt/mod.rs:1076
   5: std::io::Write::write_fmt
             at src/libstd/io/mod.rs:1537
   6: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:62
   7: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:49
   8: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:198
   9: std::panicking::default_hook
             at src/libstd/panicking.rs:217
  10: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:526
  11: rust_begin_unwind
             at src/libstd/panicking.rs:437
  12: core::panicking::panic_fmt
             at src/libcore/panicking.rs:85
  13: core::option::expect_none_failed
             at src/libcore/option.rs:1269
  14: core::result::Result<T,E>::expect
             at /build/rust/src/rustc-1.46.0-src/src/libcore/result.rs:963
  15: core::cell::RefCell<T>::borrow
             at /build/rust/src/rustc-1.46.0-src/src/libcore/cell.rs:792
  16: try_pulse_binding::context_state_callback::{{closure}}
             at src/bin/try-pulse-binding.rs:37
  17: <alloc::boxed::Box<F> as core::ops::function::FnMut<A>>::call_mut
             at /build/rust/src/rustc-1.46.0-src/src/liballoc/boxed.rs:1088
  18: libpulse_binding::context::notify_cb_proxy_multi::{{closure}}
             at /path/to/cargo/registry/src/github.com-1ecc6299db9ec823/libpulse-binding-2.16.1/src/context/mod.rs:640
  19: std::panicking::try::do_call
             at /build/rust/src/rustc-1.46.0-src/src/libstd/panicking.rs:348
  20: __rust_try
  21: std::panicking::try
             at /build/rust/src/rustc-1.46.0-src/src/libstd/panicking.rs:325
  22: std::panic::catch_unwind
             at /build/rust/src/rustc-1.46.0-src/src/libstd/panic.rs:394
  23: libpulse_binding::context::notify_cb_proxy_multi
             at /path/to/cargo/registry/src/github.com-1ecc6299db9ec823/libpulse-binding-2.16.1/src/context/mod.rs:638
  24: <unknown>
  25: pa_context_connect
  26: libpulse_binding::context::Context::connect
             at /path/to/cargo/registry/src/github.com-1ecc6299db9ec823/libpulse-binding-2.16.1/src/context/mod.rs:295
  27: try_pulse_binding::main
             at src/bin/try-pulse-binding.rs:25
  28: std::rt::lang_start::{{closure}}
             at /build/rust/src/rustc-1.46.0-src/src/libstd/rt.rs:67
  29: std::rt::lang_start_internal::{{closure}}
             at src/libstd/rt.rs:52
  30: std::panicking::try::do_call
             at src/libstd/panicking.rs:348
  31: std::panicking::try
             at src/libstd/panicking.rs:325
  32: std::panic::catch_unwind
             at src/libstd/panic.rs:394
  33: std::rt::lang_start_internal
             at src/libstd/rt.rs:51
  34: std::rt::lang_start
             at /build/rust/src/rustc-1.46.0-src/src/libstd/rt.rs:67
  35: main
  36: __libc_start_main
  37: _start

The key stacks here are number 16 and 27. The problem is that we have already borrowed context mutably when calling context.borrow_mut().connect(), and connect() will call the callback for context-state-changed, which make context.borrow().get_state() inside the callback failed, because of the borrowing rules. Similar problem arises when using the callback pattern with stream and mainloop.

I think this problem cannot be solved using only safe rust. Currently, to workaround it, one way is avoiding callback, like the example shown in the libpulse_binding::mainloop::standard documentation. Another way is using a pointer, thus involving unsafe rust. Perhaps with a higher level API that utilizes future, we can avoid the callback pattern thus avoiding the problem.

[jnqnfe]

@dianhenglau, yes, I'm aware that there are some limitations with the design, forcing use of small bits of unsafe user code, and simple user mistakes not being caught at compile time. There is no easy solution.

What is "running the main loop using the callback style"?

I'm not sure what you're asking here - why you've quoted what appears to be a question...Or if you're asking a question at all...

For the standard mainloop, the mainloop and callbacks are all executed within the user thread. For the threaded mainloop, they are executed in a separate thread dedicated to the mainloop.

What is the problem?

I tried to follow the pattern above using libpulse_binding:

...

...The problem is that we have already borrowed context mutably when calling context.borrow_mut().connect()...

Yes....

I think this problem cannot be solved using only safe rust. Currently, to workaround it, one way is avoiding callback, like the example shown in the libpulse_binding::mainloop::standard documentation. Another way is using a pointer, thus involving unsafe rust.

Yep. You've got to hack around the borrow checker in this situation. The only way to do things without any unsafe user code would be to create wrappers such as alternatives to Rc which drop the borrow-checking that Rc has, but providing something like that is dangerous.

Example pointer based solution, if it helps:

use libpulse_binding::{
    context::{self, Context},
    mainloop::standard::Mainloop,
    proplist::Proplist,
};
use std::{cell::RefCell, rc::Rc};

fn main() {
    // Set up a new main loop
    let mut mainloop = Mainloop::new().unwrap();

    // Create an empty proplist
    let proplist = Proplist::new().unwrap();

    // Create a new connection context
    let context = Rc::new(RefCell::new(
        Context::new_with_proplist(&mainloop, "Try Pulse", &proplist).unwrap(),
    ));

    context
        .borrow_mut()
        .set_state_callback(Some(context_state_callback(Rc::clone(&context))));

    // Connect the context
    context
        .borrow_mut()
        .connect(None, context::flags::NOFLAGS, None)
        .unwrap();

    // Run the main loop
    mainloop.run().unwrap();
}

/* Return a closure that will be called whenever the context status changes */
fn context_state_callback(context: Rc<RefCell<Context>>) -> Box<dyn FnMut()> {
    Box::new(move || {
        let state = unsafe { &*context.as_ptr() }.get_state();
        match state {
            // Take action based on context state...
            _ => (),
        }
    })
}

Perhaps with a higher level API that utilizes future, we can avoid the callback pattern thus avoiding the problem.

It's been a while since I looked at this, so I can't comment off hand exactly how much better off we'd be with futures in terms of avoiding the problems, but getting futures integrated remains a key goal. Unfortunately it is very much non-trivial to wire up to the PulseAudio API correctly, hence why I've not gotten it done thus far.

[agraven]

I am currently looking into potentially writing an async/await wrapper for libpulse-binding. After a small amount of initial experimentation, doing so seems to be viable. Certain parts are easier than others to wrap, with e.g. simply waiting for the result of a callback being more or less trivial, and making the mainloop's poll non-blocking being somewhat complex

[agraven]

Also, for a non-async high level API, there is rust-pulsectl, though the maintenance state of that crate is shaky at best.

[agraven]

I've pushed some of my work to my repository at https://github.com/agraven/libpulse-binding-async