Misc questions

[Boscop]

A few questions came up, maybe you could answer some of them? :)

1. If I want my VST that uses the extension API to also work in other DAWs, is it safe to call reaper_vst_plugin!() and PluginContext::from_vst_plugin(&self.host, static_vst_plugin_context()) when running in other DAWs? Do I have to be careful about something? (Also, is it safe to have multiple instances of the same VST loaded in the same project?)
2. Is there a better way to detect if I'm running in Reaper than this?

let host_info = host.get_info();
if host_info.1 == "Cockos" && host_info.2.starts_with("REAPER") {

3. Which value should I return from ext_set_fx_change?
4. I copied MyOnAudioBuffer and MyControlSurface from here, and modified run() to log to my logfile instead, but it's not being called, any idea why?
5. When Reaper starts, set_track_list_change is called twice, but if I add a new track and then rename it, it's not called. Why could that be?
6. It seems I have to wait until the project is fully loaded before calling the extension API, otherwise Reaper can crash when calling the API. Is there a way to check when the project has finished loading?
7. Any idea why GetNumTracks returns 0 when Reaper is not paused? It only seems to returns the right number when playback is paused.

[helgoboss]

A few questions came up, maybe you could answer some of them? :)

I'll try as far as I can. I just sometimes have to refer to the REAPER devs because I don't have the time/capacity to provide REAPER API support.

1. If I want my VST that uses the extension API to also work in other DAWs, is it safe to call reaper_vst_plugin!() and PluginContext::from_vst_plugin(&self.host, static_vst_plugin_context()) when running in other DAWs? Do I have to be careful about something? (Also, is it safe to have multiple instances of the same VST loaded in the same project?)

Good question, I should document this. Both should be okay. It shouldn't fail because it doesn't yet try to interact with REAPER. It's just about preparing the context needed for actually interacting with REAPER.

Things start to get interesting as soon as you use try to obtain a (low-level) Reaper instance by invoking Reaper::load(). In this case, the host callback will be invoked repeatedly to get access to all the REAPER function pointers. Even this should be fine if we can assume that all non-REAPER hosts just return a null pointer in this case. Then the Reaper instance will exist, but all function pointers will be None. You can check individual function pointers if they are None by using the pointers() method on the Reaper struct (documented on the Reaper struct).

Of course you could avoid calling Reaper::load() in the first place by checking before whether the host is actually REAPER.

2. Is there a better way to detect if I'm running in Reaper than this?

let host_info = host.get_info();
if host_info.1 == "Cockos" && host_info.2.starts_with("REAPER") {

Not sure. I don't put my REAPER-only VST plug-ins in the usual VST plug-in directoy anymore but into REAPER_RESOURCE_DIR/UserPlugins/FX, so it's not picked up by other hosts in the first place.

If you find the best possible way to do this, it would be cool if you could add it to the docs.

1. Which value should I return from ext_set_fx_change?

Return something different than 0 if you handle this callback. I usually return 1.

The only difference this return values makes for reaper-rs is that extended() will only be called if you return 0 (which does the default method implementation). This is the backward compatibility mechanism I came up with to make it possible to safely add more of those type-safe ext_ methods in future without danger of breaking existing code that implements the (non-type-safe) extended method.

The difference for REAPER itself is similar: return 0 if unsupported the docs say. There might be other differences, either now or potentially in future. That's why I left the return value be an interger, not make it a boolean or something (even it would be more descriptive). Otherwise it would be a backwards compatibility nightmare.

2. I copied MyOnAudioBuffer and MyControlSurface from here, and modified run() to log to my logfile instead, but it's not being called, any idea why?

Could be many reasons. The most likely one maybe: The medium-level Reaper struct uses RAII to automatically unregister your hooks on drop. So you must keep this struct around, e.g. as member of your VST plug-in struct.

3. When Reaper starts, set_track_list_change is called twice, but if I add a new track and then rename it, it's not called. Why could that be?

Probably same reason as above.

4. It seems I have to wait until the project is fully loaded before calling the extension API, otherwise Reaper can crash when calling the API. Is there a way to check when the project has finished loading?

I do this by defering closure execution until the next main loop cycle, this works. So just execute your code next time the control surface run method is called.

5. Any idea why GetNumTracks returns 0 when Reaper is not paused? It only seems to returns the right number when playback is paused.

I don't know this function, I use count_tracks.

[Boscop]

Thanks, everything is working fine now.
(Yeah, the problem was that I had dropped the session.)
And count_tracks indeed works when Reaper is not paused. (Weird that GetNumTracks doesn't.)

Btw, would it be possible to make methods like set_track_list_change and ext_set_fx_change take &mut self or would that be unsafe? (I'm currently working around this by sending msgs to self that are then handled in run.)

And I noticed something:
Why is set_track_list_change not called when a track is renamed (and ext_set_fx_change is not called when a FX is renamed)?

[helgoboss]

Thanks, everything is working fine now.

Good to hear :)

(Yeah, the problem was that I had dropped the session.)
And count_tracks indeed works when Reaper is not paused. (Weird that GetNumTracks doesn't.)

Strange indeed.

Btw, would it be possible to make methods like set_track_list_change and ext_set_fx_change take &mut self or would that be unsafe? (I'm currently working around this by sending msgs to self that are then handled in run.)

That would be unsafe in terms of reentrancy. run never is reentered while running (makes sense because it's a main loop invocation), so we can guarantee exclusive access. All remaining callback methods might be reentered while running. E.g. what might happen with set_track_list_change:

1. REAPER calls our set_track_list_change
2. Our set _track_list_change calls REAPER to add a track.
3. REAPER reacts to that and calls our set_track_list_change. BOOM, reentrancy! If we would take self as mutable in this case, we would have violated one of Rust's most important safety measures.

There might be other callback methods except run which don't suffer this possible reentracy problem but I wouldn't rely on it. I asked Justin once and he only guaranteed run to be non-reentrant.

If you want to modify something synchronously (without a channel), you can use interior mutability.

And I noticed something:
Why is set_track_list_change not called when a track is renamed (and ext_set_fx_change is not called when a FX is renamed)?

1. Because there's a dedicated callback for this: set_track_title.
2. Don't know. I guess it's just not implemented. I would like to know the answer as well.

[Boscop]

Thanks, everything is working fine now.
Btw, why does ControlSurface derive from Debug?
It forces me to exclude almost every field:

#[derive(Derivative)]
#[derivative(Debug)]
pub struct MyControlSurface<FC> {
	#[derivative(Debug = "ignore")]
	pub reaper: Reaper,
	#[derivative(Debug = "ignore")]
	pub tx_action: Sender<SurfaceAction>,
	#[derivative(Debug = "ignore")]
	pub rx_action: Receiver<SurfaceAction>,
	#[derivative(Debug = "ignore")]
	pub action_queue: DebounceQueue<SurfaceAction>,
	#[derivative(Debug = "ignore")]
	pub reload_cfg_and_apply_if_changed: Box<FC>,
	#[derivative(Debug = "ignore")]
	pub file_watcher: Hotwatch,
	#[derivative(Debug = "ignore")]
	pub host: HostCallback,
	pub my_track_name: String,
}

[helgoboss]

I added this type bound because it becomes part of DelegatingControlSurface as delegate: Box<dyn ControlSurface> and this one becomes part of the ReaperSession struct ... and latter I want to support Debug for deep inspection of the object graph. Because it's useful and also part of the Rust API design guidelines.

Maybe it would be good to remove this requirement for ControlSurface so that it's up to the consumer to decide if he wants Debug in this depth. However, I'm not sure how I would use the Debug implementation if it's there and not use it if it's not there. Any suggestions?

On the other hand ... why does it force you to exclude so many fields? E.g. the Reaper struct supports Debug, Hotwatch also, and the channels as well - if you make SurfaceAction implement Debug.

[Boscop]

Ah, makes sense.
Yeah, I realized I didn't have to exclude so many fields..

[helgoboss]

Btw, would it be possible to make methods like set_track_list_change and ext_set_fx_change take &mut self or would that be unsafe? (I'm currently working around this by sending msgs to self that are then handled in run.)

@Boscop FYI, I realized there's a soundness issue even with the current restriction of allowing read access only in set_track_list_change and similar callbacks. If we are currently in the call stack of ControlSurface::run() and REAPER invokes one of the callback methods, then even giving read-only access to the control surface is wrong. We shouldn't grant any access at all in this case. I think the only sound way (from a Rust perspective) would be to defer the callback method invocation to a point after the run() method has completed. So either to execute right after run() has ended or wait until the next run() cycle ... when doing this consequently, one could even grant write access.