Skip to content

Commit

Permalink
(kinda) bring back extension proxy object support in AAPXSDefinition.
Browse files Browse the repository at this point in the history
I thought we could just eliminate strongly typed extension proxies from
AAPXSDefinition.
It turned out that binder-client-as-plugin still needs to implement
`get_extension()` function, and it needs an extension instance for each URI.

The problem was hidden because `get_extension()` in binder-client-as-plugin
was buggy and was trying to get the extension from AndroidAudioPluginHost,
whose extension retrieval was also buggy and returned plugin extensions.

Fortunately?, TypedAAPXS classes could be used to represent the extension
functions and they could be kinda populated with additional contexts (such
as AAPXSInitiatorInstance and AAPXSSerialization).
  • Loading branch information
atsushieno committed Nov 7, 2023
1 parent a7526e6 commit 436e16b
Show file tree
Hide file tree
Showing 22 changed files with 505 additions and 262 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ class AudioPluginInterfaceImpl : public aidl::org::androidaudioplugin::BnAudioPl
CHECK_INSTANCE(instance, in_instanceID)

instance->completeInstantiation();
instance->setupAAPXS();
instance->setupAAPXSInstances();
instance->startPortConfiguration();
return ndk::ScopedAStatus::ok();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,8 @@ Java_org_androidaudioplugin_hosting_NativeRemotePluginInstance_createRemotePlugi
}
auto instance = dynamic_cast<aap::RemotePluginInstance*>(client->getInstanceById(result.value));
assert(instance);
instance->configurePorts();
instance->scanParametersAndBuildList();
instance->configurePorts();
return (jlong) result.value;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,13 @@ void aap_client_as_plugin_deactivate(AndroidAudioPlugin *plugin)
void* aap_client_as_plugin_get_extension(AndroidAudioPlugin *plugin, const char *uri)
{
auto ctx = (AAPClientContext*) plugin->plugin_specific;
return ctx->host.get_extension(&ctx->host, uri);
auto instance = (aap::RemotePluginInstance*) ctx->host.context;
auto aapxsDefinition = instance->getAAPXSRegistry()->items()->getByUri(uri);
if (!aapxsDefinition->get_plugin_extension_proxy)
return nullptr;
auto aapxsInstance = instance->getAAPXSDispatcher().getPluginAAPXSByUri(uri);
auto proxy = aapxsDefinition->get_plugin_extension_proxy(aapxsDefinition, aapxsInstance, aapxsInstance->serialization);
return proxy.as_plugin_extension(&proxy);
}

aap_plugin_info_t aap_client_as_plugin_get_plugin_info(AndroidAudioPlugin *plugin)
Expand Down Expand Up @@ -214,7 +220,7 @@ AndroidAudioPlugin* aap_client_as_plugin_new(

// Set up shared memory FDs for plugin extension services.
// We make use of plugin metadata that should list up required and optional extensions.
if (!instance->setupAAPXSInstances(instance->getAAPXSRegistry(), [&](const char* uri, AAPXSSerializationContext *serialization) {
if (!instance->setupAAPXSInstances([&](const char* uri, AAPXSSerializationContext *serialization) {
// create asharedmem and add as an extension FD, keep it until it is destroyed.
auto fd = ASharedMemory_create(nullptr, serialization->data_capacity);
auto shm = instance->getSharedMemoryStore();
Expand Down
20 changes: 9 additions & 11 deletions androidaudioplugin/src/main/cpp/core/aapxs/aapxs-runtime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

// Client setup

aap::xs::AAPXSClientDispatcher::AAPXSClientDispatcher(AAPXSDefinitionClientRegistry *registry)
: AAPXSDispatcher(registry->items()->getUridMapping()), registry(registry) {
aap::xs::AAPXSClientDispatcher::AAPXSClientDispatcher(AAPXSDefinitionRegistry *registry)
: AAPXSDispatcher(registry->getUridMapping()), registry(registry) {
}

bool aap::xs::AAPXSClientDispatcher::setupInstances(void* hostContext,
Expand All @@ -15,11 +15,10 @@ bool aap::xs::AAPXSClientDispatcher::setupInstances(void* hostContext,
initiator_get_new_request_id_func initiatorGetNewRequestId) {
assert(!already_setup);

auto items = registry->items();
if (!std::all_of(items->begin(), items->end(), [&](AAPXSDefinition& f) {
if (!std::all_of(registry->begin(), registry->end(), [&](AAPXSDefinition& f) {
if (!f.uri)
return true; // skip
int32_t urid = registry->items()->getUridMapping()->getUrid(f.uri);
int32_t urid = registry->getUridMapping()->getUrid(f.uri);
// allocate SerializationContext
auto serialization = std::make_unique<AAPXSSerializationContext>();
serialization->data_capacity = f.data_capacity;
Expand Down Expand Up @@ -63,14 +62,14 @@ aap::xs::AAPXSClientDispatcher::populateAAPXSRecipientInstance(
}

AAPXSSerializationContext *aap::xs::AAPXSClientDispatcher::getSerialization(const char *uri) {
auto& shm = serialization_store[registry->items()->getUridMapping()->getUrid(uri)];
auto& shm = serialization_store[registry->getUridMapping()->getUrid(uri)];
return shm ? shm.get() : nullptr;
}

// Service setup

aap::xs::AAPXSServiceDispatcher::AAPXSServiceDispatcher(AAPXSDefinitionServiceRegistry *registry)
: AAPXSDispatcher(registry->items()->getUridMapping()), registry(registry) {
aap::xs::AAPXSServiceDispatcher::AAPXSServiceDispatcher(AAPXSDefinitionRegistry *registry)
: AAPXSDispatcher(registry->getUridMapping()), registry(registry) {
}

void aap::xs::AAPXSServiceDispatcher::setupInstances(void* hostContext,
Expand All @@ -80,11 +79,10 @@ void aap::xs::AAPXSServiceDispatcher::setupInstances(void* hostContext,
initiator_get_new_request_id_func initiatorGetNewRequestId) {
assert(!already_setup);

auto items = registry->items();
std::for_each(items->begin(), items->end(), [&](AAPXSDefinition& f) {
std::for_each(registry->begin(), registry->end(), [&](AAPXSDefinition& f) {
if (!f.uri)
return; // skip
int32_t urid = registry->items()->getUridMapping()->getUrid(f.uri);
int32_t urid = registry->getUridMapping()->getUrid(f.uri);
// allocate SerializationContext
auto serialization = std::make_unique<AAPXSSerializationContext>();
// host extensions
Expand Down
16 changes: 14 additions & 2 deletions androidaudioplugin/src/main/cpp/core/aapxs/midi-aapxs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,18 @@ void aap::xs::AAPXSDefinition_Midi::aapxs_midi_process_incoming_host_aapxs_reply
request->callback(request->callback_user_data, host, request->request_id);
}

int32_t aap::xs::MidiClientAAPXS::getMidiMappingPolicy() {
return callTypedFunctionSynchronously<int32_t>(OPCODE_GET_MAPPING_POLICY);
AAPXSExtensionClientProxy
aap::xs::AAPXSDefinition_Midi::aapxs_midi_get_plugin_proxy(struct AAPXSDefinition *feature,
AAPXSInitiatorInstance *aapxsInstance,
AAPXSSerializationContext *serialization) {
auto client = (AAPXSDefinition_Midi*) feature->aapxs_context;
if (!client->typed_client)
client->typed_client = std::make_unique<MidiClientAAPXS>(aapxsInstance, serialization);
*client->typed_client = MidiClientAAPXS(aapxsInstance, serialization);
client->client_proxy = AAPXSExtensionClientProxy{client->typed_client.get(), aapxs_midi_as_plugin_extension};
return client->client_proxy;
}

enum aap_midi_mapping_policy aap::xs::MidiClientAAPXS::getMidiMappingPolicy() {
return callTypedFunctionSynchronously<enum aap_midi_mapping_policy>(OPCODE_GET_MAPPING_POLICY);
}
30 changes: 26 additions & 4 deletions androidaudioplugin/src/main/cpp/core/aapxs/parameters-aapxs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,28 @@ void aap::xs::AAPXSDefinition_Parameters::aapxs_parameters_process_incoming_host
request->callback(request->callback_user_data, host, request->request_id);
}

AAPXSExtensionClientProxy aap::xs::AAPXSDefinition_Parameters::aapxs_parameters_get_plugin_proxy(
struct AAPXSDefinition *feature, AAPXSInitiatorInstance *aapxsInstance,
AAPXSSerializationContext *serialization) {
auto client = (AAPXSDefinition_Parameters*) feature->aapxs_context;
if (!client->typed_client)
client->typed_client = std::make_unique<ParametersClientAAPXS>(aapxsInstance, serialization);
*client->typed_client = ParametersClientAAPXS(aapxsInstance, serialization);
client->client_proxy = AAPXSExtensionClientProxy{client->typed_client.get(), aapxs_parameters_as_plugin_extension};
return client->client_proxy;
}

AAPXSExtensionServiceProxy aap::xs::AAPXSDefinition_Parameters::aapxs_parameters_get_host_proxy(
struct AAPXSDefinition *feature, AAPXSInitiatorInstance *aapxsInstance,
AAPXSSerializationContext *serialization) {
auto service = (AAPXSDefinition_Parameters*) feature->aapxs_context;
if (!service->typed_service)
service->typed_service = std::make_unique<ParametersServiceAAPXS>(aapxsInstance, serialization);
*service->typed_service = ParametersServiceAAPXS(aapxsInstance, serialization);
service->service_proxy = AAPXSExtensionServiceProxy{&service->typed_service, aapxs_parameters_as_host_extension};
return service->service_proxy;
}

// AAPXSParametersClient

int32_t aap::xs::ParametersClientAAPXS::getParameterCount() {
Expand All @@ -95,26 +117,26 @@ int32_t aap::xs::ParametersClientAAPXS::getParameterCount() {
aap_parameter_info_t aap::xs::ParametersClientAAPXS::getParameter(int32_t index) {
*((int32_t*) serialization->data) = index;
return callTypedFunctionSynchronously<aap_parameter_info_t>(
OPCODE_PARAMETERS_GET_PARAMETER_COUNT);
OPCODE_PARAMETERS_GET_PARAMETER);
}

double aap::xs::ParametersClientAAPXS::getProperty(int32_t index, int32_t propertyId) {
*((int32_t*) serialization->data) = index;
*((int32_t*) serialization->data + 1) = propertyId;
return callTypedFunctionSynchronously<double>(OPCODE_PARAMETERS_GET_PARAMETER_COUNT);
return callTypedFunctionSynchronously<double>(OPCODE_PARAMETERS_GET_PROPERTY);
}

int32_t aap::xs::ParametersClientAAPXS::getEnumerationCount(int32_t index) {
*((int32_t*) serialization->data) = index;
return callTypedFunctionSynchronously<int32_t>(OPCODE_PARAMETERS_GET_PARAMETER_COUNT);
return callTypedFunctionSynchronously<int32_t>(OPCODE_PARAMETERS_GET_ENUMERATION_COUNT);
}

aap_parameter_enum_t
aap::xs::ParametersClientAAPXS::getEnumeration(int32_t index, int32_t enumIndex) {
*((int32_t*) serialization->data) = index;
*((int32_t*) serialization->data + 1) = enumIndex;
return callTypedFunctionSynchronously<aap_parameter_enum_t>(
OPCODE_PARAMETERS_GET_PARAMETER_COUNT);
OPCODE_PARAMETERS_GET_ENUMERATION);
}

void aap::xs::ParametersClientAAPXS::completeWithParameterCallback (void* callbackData, void* pluginOrHost, int32_t requestId) {
Expand Down
81 changes: 53 additions & 28 deletions androidaudioplugin/src/main/cpp/core/aapxs/presets-aapxs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,51 +5,52 @@ void aap::xs::AAPXSDefinition_Presets::aapxs_presets_process_incoming_plugin_aap
struct AAPXSDefinition *feature, AAPXSRecipientInstance *aapxsInstance,
AndroidAudioPlugin *plugin, AAPXSRequestContext *request) {
auto ext = (aap_presets_extension_t*) plugin->get_extension(plugin, AAP_PRESETS_EXTENSION_URI);
if (!ext)
return; // FIXME: should there be any global error handling?
switch(request->opcode) {
case OPCODE_GET_PRESET_COUNT:
*((int32_t*) request->serialization->data) = ext->get_preset_count(ext, plugin);
*((int32_t*) request->serialization->data) = ext ? ext->get_preset_count(ext, plugin) : 0;
// RT_SAFE. Send reply now.
aapxsInstance->send_aapxs_reply(aapxsInstance, request);
break;
case OPCODE_GET_PRESET_INDEX:
*((int32_t*) request->serialization->data) = ext->get_preset_index(ext, plugin);
*((int32_t*) request->serialization->data) = ext ? ext->get_preset_index(ext, plugin) : 0;
// RT_SAFE. Send reply now.
aapxsInstance->send_aapxs_reply(aapxsInstance, request);
break;
case OPCODE_SET_PRESET_INDEX: {
int32_t index = *((int32_t *) request->serialization->data);
if (ext) {
int32_t index = *((int32_t *) request->serialization->data);

// FIXME: should this be calling it asynchronously?
// Async invoker foundation should be backed by AAPXSRecipientInstance though.
// FIXME: should this be calling it asynchronously?
// Async invoker foundation should be backed by AAPXSRecipientInstance though.

ext->set_preset_index(ext, plugin, index);
ext->set_preset_index(ext, plugin, index);
}

aapxsInstance->send_aapxs_reply(aapxsInstance, request);
break;
}
case OPCODE_GET_PRESET_DATA: {
// request (offset-range: content)
// - 0..3 : int32_t index
// - 4..7 : bool skip binary or not
aap_preset_t preset;
int32_t index = *((int32_t *) request->serialization->data);

// FIXME: should this be calling it asynchronously?
// Async invoker foundation should be backed by AAPXSRecipientInstance though.

ext->get_preset(ext, plugin, index, &preset, nullptr,
nullptr); // we can pass null callback as plugins implement it in synchronous way
// response (offset-range: content)
// - 0..3 : stable ID
// - 4..259 : name (fixed length char buffer)
*((int32_t *) request->serialization->data) = preset.id;
size_t len = strlen(preset.name);
strncpy((char *) request->serialization->data + sizeof(int32_t),
const_cast<char *const>(preset.name), len);
((char *) request->serialization->data)[len + sizeof(int32_t)] = 0;

if (ext) {
// request (offset-range: content)
// - 0..3 : int32_t index
// - 4..7 : bool skip binary or not
aap_preset_t preset;
int32_t index = *((int32_t *) request->serialization->data);

// FIXME: should this be calling it asynchronously?
// Async invoker foundation should be backed by AAPXSRecipientInstance though.

ext->get_preset(ext, plugin, index, &preset, nullptr,
nullptr); // we can pass null callback as plugins implement it in synchronous way
// response (offset-range: content)
// - 0..3 : stable ID
// - 4..259 : name (fixed length char buffer)
*((int32_t *) request->serialization->data) = preset.id;
size_t len = strlen(preset.name);
strncpy((char *) request->serialization->data + sizeof(int32_t),
const_cast<char *const>(preset.name), len);
((char *) request->serialization->data)[len + sizeof(int32_t)] = 0;
}
aapxsInstance->send_aapxs_reply(aapxsInstance, request);
break;
}
Expand Down Expand Up @@ -90,6 +91,30 @@ void aap::xs::AAPXSDefinition_Presets::aapxs_presets_process_incoming_host_aapxs
request->callback(request->callback_user_data, host, request->request_id);
}

AAPXSExtensionClientProxy
aap::xs::AAPXSDefinition_Presets::aapxs_presets_get_plugin_proxy(struct AAPXSDefinition *feature,
AAPXSInitiatorInstance *aapxsInstance,
AAPXSSerializationContext *serialization) {
auto client = (AAPXSDefinition_Presets*) feature->aapxs_context;
if (!client->typed_client)
client->typed_client = std::make_unique<PresetsClientAAPXS>(aapxsInstance, serialization);
*client->typed_client = PresetsClientAAPXS(aapxsInstance, serialization);
client->client_proxy = AAPXSExtensionClientProxy{client->typed_client.get(), aapxs_presets_as_plugin_extension};
return client->client_proxy;
}

AAPXSExtensionServiceProxy
aap::xs::AAPXSDefinition_Presets::aapxs_presets_get_host_proxy(struct AAPXSDefinition *feature,
AAPXSInitiatorInstance *aapxsInstance,
AAPXSSerializationContext *serialization) {
auto service = (AAPXSDefinition_Presets*) feature->aapxs_context;
if (!service->typed_service)
service->typed_service = std::make_unique<PresetsServiceAAPXS>(aapxsInstance, serialization);
*service->typed_service = PresetsServiceAAPXS(aapxsInstance, serialization);
service->service_proxy = AAPXSExtensionServiceProxy{&service->typed_service, aapxs_presets_as_host_extension};
return service->service_proxy;
}

// Strongly-typed client implementation (plugin extension functions)

int32_t aap::xs::PresetsClientAAPXS::getPresetCount() {
Expand Down
14 changes: 13 additions & 1 deletion androidaudioplugin/src/main/cpp/core/aapxs/state-aapxs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,19 @@ void aap::xs::AAPXSDefinition_State::aapxs_state_process_incoming_host_aapxs_rep
request->callback(request->callback_user_data, host, request->request_id);
}

int32_t aap::xs::StateClientAAPXS::getStateSize() {
AAPXSExtensionClientProxy
aap::xs::AAPXSDefinition_State::aapxs_state_get_plugin_proxy(struct AAPXSDefinition *feature,
AAPXSInitiatorInstance *aapxsInstance,
AAPXSSerializationContext *serialization) {
auto client = (AAPXSDefinition_State*) feature->aapxs_context;
if (!client->typed_client)
client->typed_client = std::make_unique<StateClientAAPXS>(aapxsInstance, serialization);
*client->typed_client = StateClientAAPXS(aapxsInstance, serialization);
client->client_proxy = AAPXSExtensionClientProxy{client->typed_client.get(), aapxs_parameters_as_plugin_extension};
return client->client_proxy;
}

size_t aap::xs::StateClientAAPXS::getStateSize() {
return callTypedFunctionSynchronously<int32_t>(OPCODE_GET_STATE_SIZE);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ aap::PluginClient::Result<int32_t> aap::PluginClient::instantiateRemotePlugin(co
descriptor, pluginFactory,
sampleRate, event_midi2_input_buffer_size);
instances.emplace_back(instance);
instance->setupAAPXS();
instance->completeInstantiation();
instance->scanParametersAndBuildList();
return Result<int32_t>{instance->getInstanceId(), ""};
}
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ aap::LocalPluginInstance::LocalPluginInstance(
host(host),
aapxs_host_session(eventMidi2InputBufferSize),
feature_registry(new xs::AAPXSDefinitionServiceRegistry(aapxsRegistry)),
aapxs_dispatcher(xs::AAPXSServiceDispatcher(feature_registry.get()))
aapxs_dispatcher(aapxsRegistry)
{
shared_memory_store = new aap::ServicePluginSharedMemoryStore();
instance_id = instanceId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ aap::RemotePluginInstance::RemotePluginInstance(PluginClient* client,
client(client),
aapxs_session(eventMidi2InputBufferSize),
feature_registry(new xs::AAPXSDefinitionClientRegistry(aapxsRegistry)),
aapxs_dispatcher(xs::AAPXSClientDispatcher(feature_registry.get()))
aapxs_dispatcher(aapxsRegistry)
{
shared_memory_store = new ClientPluginSharedMemoryStore();

Expand Down Expand Up @@ -201,13 +201,15 @@ static inline void staticSendAAPXSReply(AAPXSRecipientInstance* instance, AAPXSR
((aap::RemotePluginInstance*) instance->host_context)->sendHostAAPXSReply(context);
}

bool aap::RemotePluginInstance::setupAAPXSInstances(xs::AAPXSDefinitionClientRegistry *registry,
std::function<bool(const char*, AAPXSSerializationContext*)> sharedMemoryAllocatingRequester) {
return aapxs_dispatcher.setupInstances(this,
bool aap::RemotePluginInstance::setupAAPXSInstances(std::function<bool(const char*, AAPXSSerializationContext*)> sharedMemoryAllocatingRequester) {
if (!aapxs_dispatcher.setupInstances(this,
sharedMemoryAllocatingRequester,
staticSendAAPXSRequest,
staticSendAAPXSReply,
staticGetNewRequestId);
staticGetNewRequestId))
return false;
standards->initialize(&aapxs_dispatcher);
return true;
}

bool
Expand Down Expand Up @@ -278,5 +280,5 @@ aap::xs::AAPXSDefinitionClientRegistry *aap::RemotePluginInstance::getAAPXSRegis
}

void aap::RemotePluginInstance::setupAAPXS() {
standards = std::make_unique<xs::ClientStandardExtensions>(&aapxs_dispatcher);
standards = std::make_unique<xs::ClientStandardExtensions>();
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,6 @@ void aap::PluginInstance::completeInstantiation()
assert(plugin);

instantiation_state = PLUGIN_INSTANTIATION_STATE_UNPREPARED;

setupAAPXS();
}

void aap::PluginInstance::setupPortConfigDefaults() {
Expand Down
Loading

0 comments on commit 436e16b

Please sign in to comment.