Skip to content

Commit

Permalink
Add some realtime safety arributes and design notes.
Browse files Browse the repository at this point in the history
This technically fixes issue #35 (implementation would be needed but
that should be tracked somewhere else).
  • Loading branch information
atsushieno committed Mar 8, 2023
1 parent e93ecc2 commit 27102e9
Show file tree
Hide file tree
Showing 14 changed files with 120 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@

namespace aap {

int32_t getMidiSettingsFromLocalConfig(std::string pluginId) {
return AAPJniFacade::getInstance()->getMidiSettingsFromLocalConfig(pluginId);
}

// ----------------------------------

std::vector<PluginInformation*> convertPluginList(jobjectArray jPluginInfos)
Expand Down
5 changes: 5 additions & 0 deletions androidaudioplugin/src/main/cpp/core/AAPJniFacade.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#ifndef AAP_CORE_AAPJNIFACADE_H
#define AAP_CORE_AAPJNIFACADE_H

#if ANDROID

#include "aap/core/plugin-information.h"
#include "aap/core/host/plugin-connections.h"
#include "jni.h"
Expand Down Expand Up @@ -64,4 +66,7 @@ namespace aap {
};

}

#endif // ANDROID

#endif //AAP_CORE_AAPJNIFACADE_H
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ class MidiPluginServiceExtension : public PluginServiceExtensionImplBase {
assert(len < MAX_PLUGIN_ID_SIZE);
char* pluginId = (char*) calloc(len, 1);
strncpy(pluginId, (const char*) ((int32_t*) extensionInstance->data + 1), len);
*((int32_t*) extensionInstance->data) = AAPJniFacade::getInstance()->getMidiSettingsFromLocalConfig(pluginId);
*((int32_t*) extensionInstance->data) = getMidiSettingsFromLocalConfig(pluginId);
return;
}
assert(false); // should not happen
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,6 @@ AndroidAudioPluginFactory *GetDesktopAudioPluginFactoryClientBridge(aap::PluginC
#endif

namespace aap {
int32_t getMidiSettingsFromLocalConfig(std::string pluginId);
}
#endif // AAP_CORE_AUDIO_PLUGIN_HOST_INTERNALS_H
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,17 @@

#if !ANDROID

extern "C"
int32_t getMidiSettingsFromLocalConfig(std::string pluginId) {
assert(false); // FIXME: implement
}

extern "C"
int32_t createGui(std::string pluginId, int32_t instanceId, void* audioPluginView) {
assert(false); // It is not implemented (not even suposed to be).
}

namespace aap {

int32_t getMidiSettingsFromLocalConfig(std::string pluginId) {
assert(false); // FIXME: implement
}

PluginClientSystem* PluginClientSystem::getInstance() {
assert(false); // FIXME: implement
}
Expand Down
55 changes: 55 additions & 0 deletions docs/design/REALTIME_SAFETY.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# AAP and requirements for realtime audio processing

Realtime audio processing is a must in generic audio processing in audio plugin framework, and AAP should not be exceptional. Currently there are some however some inevitable realtime blockers as long as we are on Android (which is the almost only reason why we use AAP). This documentation clarifies how we do and should tuckle realtime audio processing problems.

As explained shortly later, it is in theory impossible to get AAP to process audio in truly realtime manner. But we should try to minimize the impact of realtime blockes. It is why this documentation is done.

## Where we need realtime safety

### within audio processing loop in the audio thread

AAP is not very different from typical audio plugin formats in that it has the "realtime ready" state and the state otherwise. Whenever a host and plugin is at ACTIVE state, the audio processing is supposed to be RT-safe.

For both host and plugin, `process()` needs to be implemented in RT-safe manner. `activate()` and `deactivate()` switches those states, but they themselves do not necessarily have to be RT-safe (they would mostly be though).

### extensions

Each extension function is attributed as either RT-safe or non-RT-safe. (NOTE: even with C23 we cannot really define custom attributes, so we would only put code comments `RT_SAFE` or `RT_UNSAFE`.

Regardless of the attributed safety, each extension function should perform in RT-safe manner. Those marked as RT-safe should be simply implemented in RT-safe manner. Those extension functions that are marked as non-RT-safe still need to execute in asynchronous manner, i.e. :

- at INACTIVE state, we do not have to worry about RT-safety, thus Binder `extension()` method can be (synchronously) invoked and they can be implemented in AAPXS in simply synchronous way.
- at ACTIVE state, those non-RT-safe functions has to be transmitted as asynchronous MIDI Universal SysEx UMP. There might or might not be a reply Universal SysEx UMP to it in the returning processing result. When it was not performed within the same `process()` time slot, then response will be sent back to host in another `process()` cycle later.

Although note that this does not mean that typical end users of AAP API (i.e. plugin developers) should care; they can implement AAP extensions in synchronous manner. AAPXS implementors (like ourselves as libandroidaudioplugin developers) should take care e.g. dispatching client requests to another non-RT thread, and correlate Universal SysEx responses.

(We do not consider other channels between host and plugin than Binder; it is possible to establish another channel that is not based on Binder, or establish non-realtime Service connection, but that would complicate the entire plugin connection management. )

## Inevitable realtime blockers

### (Ndk)Binder

As long as we need AAP plugin hosts (DAWs) and AAP plugins in different application processes, it is inevitable that we have to resort to some IPC framework, and we use Binder as the best optimal IPC framework on Android.

Unfortunately, we cannot use Binder in realtime manner whereas [binder itself is designed to be realtime ready](https://source.android.com/docs/core/architecture/hidl/binder-ipc#rt-priority). We will be suffered from its mutex locks.

While the entire Service framework is Dalvik only (it should not matter anyways), use of NdkBinder and native code is (almost) must. Any audio framework in native code (such as JUCE) would invoke audio processing from within Oboe audio callback, which is supposed to be RT-safe.

## realtime blockers that we could avoid

- JNI: `JavaVM` and `JNIEnv` brings in the entire Dalvik VM which is not RT-safe (GC, JIT, threads in mutex etc.). RT audio thread should not attach to JavaVM, which will stop the attached thread in blocking manner.
- (most of) system calls such as I/O or `std::mutex`
- excessive > O(1) operations; they make WCET unpredictable
- non-wait-free locks
- blocking thread dispatchers: calling async operation should be made wait-free. (For example we use `ALooper` with pipe `write()` which should be rt-safe.)

## external references

- Real-time audio programming 101: time waits for nothing http://www.rossbencina.com/code/real-time-audio-programming-101-time-waits-for-nothing
- (ADC '19) Real-time 101 - part I: Investigating the real-time problem space https://www.youtube.com/watch?v=Q0vrQFyAdWI and part II: https://www.youtube.com/watch?v=PoZAo2Vikbo&t=6s
- https://github.com/hogliux/farbot/ , referenced from the session talk
- (ADC '22) Thread synchronisation in real-time audio processing with RCU (Read-Copy-Update) https://www.youtube.com/watch?v=7fKxIZOyBCE
- https://github.com/crill-dev/crill , referenced from the session talk
- https://github.com/cradleapps/realtime_memory std::pmr implementation (referenced from the relevant ADC22 session talk, not published yet)


10 changes: 5 additions & 5 deletions include/aap/ext/gui.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,21 +38,21 @@ typedef struct aap_gui_extension_t {
// Note that audioPluginView parameter is used only between AAPXS service and the plugin.
// The plugin client process has no access to the View in the AudioPluginService process.
// The actual instantiation could be asynchronously done.
gui_extension_create_func_t create;
RT_UNSAFE gui_extension_create_func_t create;

// shows the view (using `WindowManager.addView()`).
// returns AAP_GUI_RESULT_OK for success, or non-zero error code. e.g. AAP_GUI_ERROR_NO_DETAILS.
gui_extension_show_func_t show;
RT_UNSAFE gui_extension_show_func_t show;

// hides the view (using `WindowManager.removeView()`).
gui_extension_hide_func_t hide;
RT_UNSAFE gui_extension_hide_func_t hide;

// resizes the View (by using `WindowManager.updateViewLayout()`.
// returns AAP_GUI_RESULT_OK for success, or non-zero error code. e.g. AAP_GUI_ERROR_NO_DETAILS.
gui_extension_resize_func_t resize;
RT_UNSAFE gui_extension_resize_func_t resize;

// frees the view.
gui_extension_destroy_func_t destroy;
RT_UNSAFE gui_extension_destroy_func_t destroy;
} aap_gui_extension_t;

#ifdef __cplusplus
Expand Down
2 changes: 1 addition & 1 deletion include/aap/ext/midi.h
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ typedef struct aap_midi_extension_t {
// Returns the MIDI mapping policy flags that indicates the message types that the plugin may directly
// consume and thus cannot be mapped to other purposes (parameter changes and preset changes).
// The actual callee is most likely audio-plugin-host, not the plugin.
midi_extension_get_mapping_policy_func_t get_mapping_policy;
RT_UNSAFE midi_extension_get_mapping_policy_func_t get_mapping_policy;
} aap_midi_extension_t;

#ifdef __cplusplus
Expand Down
6 changes: 3 additions & 3 deletions include/aap/ext/parameters.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,18 +55,18 @@ typedef aap_parameter_info_t* (*parameters_extension_get_parameter_func_t) (Andr
typedef struct aap_parameters_extension_t {
// Returns the number of parameters.
// If the plugin does not provide the parameter list on aap_metadata, it is supposed to provide them here.
parameters_extension_get_parameter_count_func_t get_parameter_count;
RT_SAFE parameters_extension_get_parameter_count_func_t get_parameter_count;
// Returns the parameter information by parameter index (NOT by ID).
// If the plugin does not provide the parameter list on aap_metadata, it is supposed to provide them here.
parameters_extension_get_parameter_func_t get_parameter;
RT_SAFE parameters_extension_get_parameter_func_t get_parameter;
} aap_parameters_extension_t;

typedef void (*parameters_host_extension_on_parameter_list_changed_func_t) (AndroidAudioPluginHost* host, AndroidAudioPlugin *plugin);

typedef struct aap_host_parameters_extension_t {
// Notifies host that parameter layout is being changed.
// THe actual parameter list needs to be queried by host (it will need to refresh the list anyways).
parameters_host_extension_on_parameter_list_changed_func_t on_parameters_changed;
RT_SAFE parameters_host_extension_on_parameter_list_changed_func_t on_parameters_changed;
} aap_host_parameters_extension_t;

#ifdef __cplusplus
Expand Down
46 changes: 26 additions & 20 deletions include/aap/ext/plugin-info.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ typedef float (*get_float_parameter_property_func)(aap_plugin_info_parameter_t*
typedef const char* (*get_string_parameter_property_func)(aap_plugin_info_parameter_t* parameter);

struct aap_plugin_info_parameter {
void *context;
get_uint32_parameter_property_func id;
get_string_parameter_property_func name;
get_float_parameter_property_func min_value;
get_float_parameter_property_func max_value;
get_float_parameter_property_func default_value;
RT_SAFE void *context;
RT_SAFE get_uint32_parameter_property_func id;
RT_UNSAFE get_string_parameter_property_func name;
RT_SAFE get_float_parameter_property_func min_value;
RT_SAFE get_float_parameter_property_func max_value;
RT_SAFE get_float_parameter_property_func default_value;
};

typedef struct aap_plugin_info_t aap_plugin_info_t;
Expand All @@ -68,26 +68,32 @@ typedef aap_plugin_info_port_t (*get_plugin_info_get_port_func)(aap_plugin_info_

struct aap_plugin_info_t {
void *context;
get_string_plugin_property_func plugin_package_name;
get_string_plugin_property_func plugin_local_name;
get_string_plugin_property_func display_name;
get_string_plugin_property_func developer_name;
get_string_plugin_property_func version;
get_string_plugin_property_func primary_category;
get_string_plugin_property_func identifier_string;
get_string_plugin_property_func plugin_id;

get_uint32_plugin_property_func get_port_count;
get_plugin_info_get_port_func get_port;
get_uint32_plugin_property_func get_parameter_count;
get_plugin_info_get_parameter_func get_parameter;
RT_UNSAFE get_string_plugin_property_func plugin_package_name;
RT_UNSAFE get_string_plugin_property_func plugin_local_name;
RT_UNSAFE get_string_plugin_property_func display_name;
RT_UNSAFE get_string_plugin_property_func developer_name;
RT_UNSAFE get_string_plugin_property_func version;
RT_UNSAFE get_string_plugin_property_func primary_category;
RT_UNSAFE get_string_plugin_property_func identifier_string;
RT_UNSAFE get_string_plugin_property_func plugin_id;

RT_SAFE get_uint32_plugin_property_func get_port_count;
RT_SAFE get_plugin_info_get_port_func get_port;
RT_SAFE get_uint32_plugin_property_func get_parameter_count;
RT_SAFE get_plugin_info_get_parameter_func get_parameter;
};

typedef void (*aap_notify_plugin_info_update_func_t)(AndroidAudioPluginExtensionTarget target);

typedef aap_plugin_info_t (*aap_host_get_plugin_info_func_t)(
AndroidAudioPluginHost* host, const char *pluginId);

typedef struct aap_plugin_info_extension_t {
RT_SAFE aap_notify_plugin_info_update_func_t notify_plugin_info_update;
} aap_plugin_info_extension_t;

typedef struct aap_host_plugin_info_extension_t {
aap_host_get_plugin_info_func_t get;
RT_UNSAFE aap_host_get_plugin_info_func_t get;
} aap_host_plugin_info_extension_t;

#ifdef __cplusplus
Expand Down
6 changes: 4 additions & 2 deletions include/aap/ext/port-config.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,10 @@ typedef void (*port_config_extension_get_options_t) (AndroidAudioPluginExtension
typedef void (*port_config_extension_select_t) (AndroidAudioPluginExtensionTarget target, const char* configuration);

typedef struct aap_port_config_extension_t {
port_config_extension_get_options_t get_options;
port_config_extension_select_t select;
// It is supposed to be stored/cached in memory
RT_SAFE port_config_extension_get_options_t get_options;
// It is supposed to be stored/cached in memory
RT_SAFE port_config_extension_select_t select;
} aap_port_config_extension_t;

#ifdef __cplusplus
Expand Down
10 changes: 5 additions & 5 deletions include/aap/ext/presets.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ typedef int32_t (*presets_extension_get_preset_index_func_t) (AndroidAudioPlugin
typedef void (*presets_extension_set_preset_index_func_t) (AndroidAudioPluginExtensionTarget target, int32_t index);

typedef struct aap_presets_extension_t {
presets_extension_get_preset_count_func_t get_preset_count;
presets_extension_get_preset_data_size_func_t get_preset_data_size;
presets_extension_get_preset_func_t get_preset;
presets_extension_get_preset_index_func_t get_preset_index;
presets_extension_set_preset_index_func_t set_preset_index;
RT_UNSAFE presets_extension_get_preset_count_func_t get_preset_count;
RT_UNSAFE presets_extension_get_preset_data_size_func_t get_preset_data_size;
RT_UNSAFE presets_extension_get_preset_func_t get_preset;
RT_UNSAFE presets_extension_get_preset_index_func_t get_preset_index;
RT_UNSAFE presets_extension_set_preset_index_func_t set_preset_index;
} aap_presets_extension_t;

#ifdef __cplusplus
Expand Down
6 changes: 3 additions & 3 deletions include/aap/ext/state.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ typedef void (*state_extension_get_state_t) (AndroidAudioPluginExtensionTarget t
typedef void (*state_extension_set_state_t) (AndroidAudioPluginExtensionTarget target, aap_state_t* source);

typedef struct aap_state_extension_t {
state_extension_get_state_size_t get_state_size;
state_extension_get_state_t get_state;
state_extension_set_state_t set_state;
RT_UNSAFE state_extension_get_state_size_t get_state_size;
RT_UNSAFE state_extension_get_state_t get_state;
RT_UNSAFE state_extension_set_state_t set_state;
} aap_state_extension_t;

#ifdef __cplusplus
Expand Down
3 changes: 3 additions & 0 deletions include/aap/unstable/aap-core.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
extern "C" {
#endif

#define RT_SAFE [[clang::annotate("AAP_RT_SAFE")]]
#define RT_UNSAFE [[clang::annotate("AAP_RT_UNSAFE")]]

// FIXME: there should be some definition for this constant elsewhere
#define DEFAULT_CONTROL_BUFFER_SIZE 8192

Expand Down

0 comments on commit 27102e9

Please sign in to comment.