Skip to content
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

An advice about rust bindings #344

Closed
alisomay opened this issue Jan 24, 2022 · 12 comments
Closed

An advice about rust bindings #344

alisomay opened this issue Jan 24, 2022 · 12 comments

Comments

@alisomay
Copy link
Contributor

I've generated rust bindings for libpd. It worked pretty straightforward with no issues.

The only issue I have is about the hooks.
It would be amazing if I could write a safe wrapper where a user can pass a closure and in the crate I can pass it as a hook to libpd.

Although the function pointer type in libpd is a bit limiting.

Ex.

typedef void (*t_libpd_printhook)(const char *s);

If the definition in C would be something along the lines of,

typedef void (*t_libpd_printhook)(const char *s, void *extra_data);

I could use some of the techniques here,

Trampoline Technique
Similar Trampoline Technique

There is a question I've asked in stack overflow for addressing a fallback solution.

I assume there are no plans to change the C API.

If you have any ideas to implement a nice API for the hooks in Rust, I'd love to hear.

@danomatika
Copy link
Member

danomatika commented Jan 24, 2022

There is some work on making all of the hooks per-instance which includes adding access to a per-instance pointer:

https://github.com/libpd/libpd/pull/282/files#diff-51ce01cd8a0f2a0249dc73e318ccfb430fbe0e341edfd69a8a83ccd81f58e29aR502

You could store implementation-specific stuff there ala a struct with the requisite per-hook data.

Note: The PR is not done and may have issues.

@alisomay
Copy link
Contributor Author

Thanks for the quick answer!
That sounds promising, after reading the PR a little bit, (I'm not very very familiar with the code base) the only part which looks relevant to what you suggest looks like this one to me

EXTERN void libpd_set_instancedata(t_pdinstance *pd, void *data);

Although I didn't see any way of accessing the instance or the data in the hook function pointers.
Am I missing something? Maybe you may point me to the right direction.

@danomatika
Copy link
Member

danomatika commented Jan 24, 2022

You would need to call libpd_get_instancedata in the wrapper callback then pass it to the rust implementation etc. libpd's C API is meant to be minimal and the various language wrappers implement whatever additional interface is required to be more "language friendly." This would be beyond the FFI approach, so I would suggest just using separate callbacks for now. I have no ETA if/when the PR would be merged.

@alisomay
Copy link
Contributor Author

alisomay commented Jan 25, 2022

By the way, I've also managed to resolve it in the context of libpd not compiled with multi instance support.

Ex. of a print hook.

pub fn on_print<F: Fn(&str) + Send + Sync + 'static>(f: F) {
    let closure: &'static _ = Box::leak(Box::new(move |string: *const std::os::raw::c_char| {
        let string = unsafe { CStr::from_ptr(string).to_str().unwrap() };
        f(string);
    }));
    let callback = Closure1::new(closure);
    let code = callback.code_ptr();
    let ptr: &_ = unsafe { std::mem::transmute(code) };
    std::mem::forget(callback);
    unsafe { libpd_sys::libpd_set_printhook(Some(*ptr)) };
}

Now the user can use it with,

on_print(|message|{
    println!("Finally: {message}")
});

It leaks the closure and it's all captured state, since drop will never be called.
I can not yet fully comprehend what inconveniences that may bring yet :)
At least it works fine.

@alisomay
Copy link
Contributor Author

alisomay commented Jan 25, 2022

By the way if I release libpd-rs later, would it be more appropriate to move it to this repository or keep it in my own github?

@danomatika
Copy link
Member

danomatika commented Jan 26, 2022 via email

@danomatika
Copy link
Member

danomatika commented Jan 26, 2022 via email

@alisomay
Copy link
Contributor Author

Sounds great actually. When I'm happy with the API. I'll write back to you.

@alisomay
Copy link
Contributor Author

alisomay commented Apr 15, 2022

After a break, I came back to this project and updated the closure implementation a bit.
I'm currently happy about how it ended up. So maybe it would be beneficial to share it here and close the issue for other readers also.

Using libffi-3.0.0

The implementation looks like this.

Example

pub fn on_float<F: FnMut(&str, f32) + Send + Sync + 'static>(mut user_provided_closure: F) {
    let closure: &'static mut _ = Box::leak(Box::new(
        move |source: *const std::os::raw::c_char, float: f32| {
            let source = unsafe { CStr::from_ptr(source).to_str().expect("You may panic or handle the error here.") };
            user_provided_closure(source, float);
        },
    ));
    let callback = ClosureMut2::new(closure);
    let code = callback.code_ptr();
    let ptr: &_ = unsafe {
        &*(code as *const libffi::high::FnPtr2<*const i8, f32, ()>)
            .cast::<unsafe extern "C" fn(*const i8, f32)>()
    };
    std::mem::forget(callback);
    unsafe { libpd_sys::libpd_set_queued_floathook(Some(*ptr)) };
} 

And it may be used like this,

    // Init pd..
    
    let my_float = Arc::new(Mutex::new(0.0_f32));
    on_float(move |source, value| {
        assert_eq!(source, "float_from_pd");
        *my_float.lock().unwrap() += value;
    });
    
    // Open patch..
    
    // This is the rust version of `libpd_bind`
    let receiver_handle = start_listening_from("float_from_pd").unwrap();
    
    // Run pd..
    // Poll messages..

A full example could be found here.

Thanks for your help @danomatika.

@danomatika
Copy link
Member

danomatika commented Apr 15, 2022 via email

@danomatika
Copy link
Member

Also note: I plan to merge the feature/multi-hooks branch soon, so you may need to make some updates. I will respond here when I do.

@alisomay
Copy link
Contributor Author

Thanks for letting me know!
I'll do the wiki page soon!

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

No branches or pull requests

2 participants