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

alphabet medium-level covering #67

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

Levitanus
Copy link
Contributor

@Levitanus Levitanus commented Nov 14, 2022

I'll move alphabetically through functions which can be straightforwardly wrapped. So, basically, this PR can be merged in master time to time without big concerns.

@Levitanus Levitanus force-pushed the alphabet_covering branch 2 times, most recently from 7165cf1 to 394c24b Compare November 15, 2022 09:51
@Levitanus
Copy link
Contributor Author

With this commit I'm wondering if there is more idiomatic way of unwrapping misc types, when underlying API wants 0, -1 or else.

For example:

let cmd: i32;
match command {
    None => cmd = 0,
    Some(id) => cmd = id.get() as i32,
}

I would like to write something like: command.unwrap_or(0) in low level function call body.

@Levitanus
Copy link
Contributor Author

Levitanus commented Nov 17, 2022

I've realized, that I not really understand the concept of usage scope. I mean, what should be in the main thread, and what — not. And should be this checked or not (are there any costs for this).

`count_automation_items`
`count_envelope_points`
`count_envelope_points_ex`
`count_take_envelopes`
`count_takes`
`count_selected_media_items`
`count_selected_media_items_unchecked`
`count_tempo_time_sig_markers`
`count_tempo_time_sig_markers_unchecked`
`delete_envelope_point_ex`
`delete_envelope_point_range`
`delete_envelope_point_range_ex`
`delete_project_marker`
`delete_project_marker_unchecked`
`delete_project_marker_by_index`
`delete_project_marker_by_index_unchecked`
`delete_take_marker`
`delete_take_stretch_markers`
`delete_tempo_time_signature_marker`
`delete_tempo_time_signature_marker_unchecked`
@helgoboss
Copy link
Owner

I've realized, that I not really understand the concept of usage scope. I mean, what should be in the main thread, and what — not. And should be this checked or not (are there any costs for this).

In general, you need to know that most functions of the REAPER API are main-thread only. If you call them in any other thread, it's undefined behavior ... and that should be avoided at all costs because undefined behavior often means crash. There are a few functions which can be called from any thread and others that must be called from a real-time thread.

The medium-level API attempts to provide safe functions (that "vow" not to exhibit undefined behavior), but of course only if that's possible without deviating from the original REAPER API function cut. That's often not possible. Fortunately, when it comes to avoiding undefined behavior due to usage from the wrong thread, it is possible 👍. That's why you find the self.require_main_thread() calls all over the place. It's an overhead, but it's super tiny and extremely worth it. I used wrong threading many years ago when I started writing Playtime because it worked on my machine. Later, I realized I got it all wrong and had to completely redesign parts of my architecture ... not good. A panic as built into reaper-rs would have saved me that effort (a panic is not nice but it's better than a crash or other undefined behavior). And I'm not the only one who fell into that trap: helgoboss/helgobox#705 ... in fact, I have the feeling that most extension writers are not aware of the threading restrictions in the beginning.

That's why I consider the panic generated by self.require_main_thread() as absolutely essential and you should use that as well whenever you build a main-thread only function. UsageScope is a modest attempt to fail at compilation time when attempting to call a medium-level function from the wrong thread. It also helps with auto-completion and is a kind of "documentation in code". However, it can't be more than that, it's quite limited. You can easily circumvent it, e.g. by consciously passing an instance of Reaper<MainThreadScope> to the audio thread and using it there. And in the high-level APIs where I use different structs and statics, this kind of type safety doesn't work at all. So it's less important than require_main_thread().

The real challenge for you as medium-level API contributor :) is to figure out if a function is main-thread only or not. Because there are exceptions. And they are not documented by Cockos :( Usually, I always assume main-thread only and later make the usage scope more broad if I realize that was wrong. It's safer and better in terms of staying backward compatible. If I need it in the real-time thread, I usually write Justin an email and ask him if it's okay to use it from there.

@Levitanus
Copy link
Contributor Author

Wow. It became much clearer! Thank you.

But, where do we really going out of the MainThread? Do I understand corretly, that all API listeners (ActionHook, CSurf) are still in the main thread? And shit can happen, mostly in three cases:

  • When a separate GUI thread spawned (egui, for example)
  • When you spawn an AudioAccessor for the hardware
  • When you want to run a server inside

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

Successfully merging this pull request may close these issues.

2 participants