overdrivenpotato / rust-vst2

VST 2.4 API implementation in rust. Create plugins or hosts.
MIT License
221 stars 23 forks source link

Added process_events() to Host for sending MIDI output to Host #19

Closed Boscop closed 7 years ago

Boscop commented 7 years ago

As requested: https://github.com/overdrivenpotato/rust-vst2/issues/18 I haven't tested it yet but I implemented it because @suhr needed it, and he is gonna test it with his plugin. So we should wait for his confirmation before merging this. (I basically copy-pasted from Plugin to Host, maybe we should factor out the commonalities..)

suhr commented 7 years ago

https://gist.github.com/suhr/0f70358b0f918e64556ec11cdb41afd9

This simple plugin segfaults in vst2::interfaces::dispatch::h706fb186c7e36e92.

Boscop commented 7 years ago

It crashes in this line: https://github.com/overdrivenpotato/rust-vst2/blob/master/src/interfaces.rs#L179

This is in the code that existed before; where the plugin gets MIDI events from the Host. Maybe it's related to the ffi for the VstEvent struct? https://github.com/johnglover/sndobj/blob/master/msvc6.0/vstplug/aeffectx.h#L63 vs. https://github.com/overdrivenpotato/rust-vst2/blob/master/src/api.rs#L397

I'm not sure why events has length 2 there..

Boscop commented 7 years ago

I found out that the VstEvents struct is supposed to have events as a variable sized array that is contiguous in memory after the previous two members, not as a pointer to an array somewhere else! Not sure what the most elegant way is to allocate this in Rust... Also, it would make sense to pre-allocate a struct to reuse between frames, and only reallocate when the number of events exceeds the allocated length! Both on the host and plugin side. Because allocation should be kept to a minimum in the ASIO thread.

suhr commented 7 years ago
  1. MIDI messages doesn't go through the plugin
  2. SIGABRT after sending several messages
#0  0x00007f0b484018f7 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:55
#1  0x00007f0b48402c09 in __GI_abort () at abort.c:89
#2  0x00007f0b484401f8 in __libc_message (do_abort=do_abort@entry=2, fmt=fmt@entry=0x7f0b485351c0 "*** Error in `%s': %s: 0x%s ***\n") at ../sysdeps/posix/libc_fatal.c:175
#3  0x00007f0b484458c5 in malloc_printerr (action=3, str=0x7f0b485352d0 "free(): invalid next size (fast)", ptr=<optimized out>) at malloc.c:4960
#4  0x00007f0b484460a3 in _int_free (av=<optimized out>, p=<optimized out>, have_lock=0) at malloc.c:3831
#5  0x00007f0b189a5c5d in alloc::heap::deallocate (ptr=0x7f0b0c0008e0 "\002", old_size=24, align=1) at /var/tmp/portage/dev-lang/rust-9999/work/rust-9999/src/liballoc/heap.rs:113
#6  0x00007f0b189a67c4 in alloc::raw_vec::{{impl}}::drop<u8> (self=0x7f0b282ec798) at /var/tmp/portage/dev-lang/rust-9999/work/rust-9999/src/liballoc/raw_vec.rs:552
#7  0x00007f0b189a2ff1 in drop::h6a8e117377a463ec () at /var/tmp/portage/dev-lang/rust-9999/work/rust-9999/src/libcore/cmp.rs:812
#8  0x00007f0b18996af9 in drop_contents::hdc70d90ff1131280 () at /var/tmp/portage/dev-lang/rust-9999/work/rust-9999/src/libcollections/vec.rs:1311
#9  0x00007f0b189a344c in drop::hdc70d90ff1131280 () at /var/tmp/portage/dev-lang/rust-9999/work/rust-9999/src/libcore/cmp.rs:812
#10 0x00007f0b189b623d in vst2::plugin::{{impl}}::process_events (self=0x2637cb0, events=...) at /home/suhr/.cargo/git/checkouts/rust-vst2-56ca6cc44fbc6a34/ed784e50484d503138e05c095a4aa48548dca516/src/plugin.rs:889
#11 0x00007f0b18995928 in eter::{{impl}}::process_events (self=0x2637cb0, events=...) at /home/suhr/Projects/eter/src/lib.rs:26
#12 0x00007f0b189b7692 in vst2::interfaces::dispatch (effect=0x837190, opcode=25, index=0, value=0, ptr=0x1f60490, opt=0)
    at /home/suhr/.cargo/git/checkouts/rust-vst2-56ca6cc44fbc6a34/ed784e50484d503138e05c095a4aa48548dca516/src/interfaces.rs:182
#13 0x00007f0b37dce333 in ?? () from /usr/lib/carla/libcarla_standalone2.so
Boscop commented 7 years ago

@suhr Are you on x64?

Boscop commented 7 years ago

I think I fixed it now, please try again :)

suhr commented 7 years ago

Events doesn't go though, but the plugin doesn't crash anymore! Strange. UPDATE: the plugin does receive MIDI events. So, the issue is with sending them.

Are you on x64?

Yep.

Boscop commented 7 years ago

Maybe you have to enable MIDI output for this VST in your host? Could you try with a SampleHost that impls Host? So host and plugin in the same executable, and then send MIDI events to the plugin and compare when they arrive in Host::process_events() ?

suhr commented 7 years ago

Maybe you have to enable MIDI output for this VST in your host?

Nope. Carla works pretty straightforward. Btw, the plugin causes Carla assertion failure: "fIsProcessing" in file CarlaPluginVST2.cpp, line 1766 in it.

Could you try with a SampleHost that impls Host? So host and plugin in the same executable, and then send MIDI events to the plugin and compare when they arrive in Host::process_events() ?

Without any dynamic loading?

Boscop commented 7 years ago

Yea, you don't need dynamic loading, just create a struct SampleHost and impl Host for it (especially process_events()): https://github.com/overdrivenpotato/rust-vst2/blob/18fceb9b5ad60cdc28433e376fd1ca5c5b01e702/examples/simple_host.rs

overdrivenpotato commented 7 years ago

I was able to test this successfully in ableton. If the builds pass on this refactor I'll merge in the changes. I also don't know of any idiomatic way to handle variable-length arrays but this should be fine

suhr commented 7 years ago

Works in Ableton, doesn't work in Carla, and there's no way to test it in Bitwig. Now I really don't know what to do.

suhr commented 7 years ago

cc @falkTX

I definitely need some ideas about how to debug this.

falkTX commented 7 years ago

the carla assertion is because you're trying to send midi events outside of its process function. this is not allowed, as it leads to concurrency errors.

suhr commented 7 years ago

@falkTX Thank you a lot! Now it works.

Btw, does VST documentation actually say that you should send events only in process()?

falkTX commented 7 years ago

not sure, but it seems pretty obvious if you know other apis. stuff that involves processing should go into the processing function.

overdrivenpotato commented 7 years ago

@suhr I've added documentation to process_events which should clarify this issue: 483aee0a29ace721af88cf734d9cc8a1621189ed

Boscop commented 7 years ago

@suhr Yeah, usually you send midi only during process()