Also cache CLAP parameter infos

src/common/logging/clap.cpp

@@ -60,8 +60,9 @@ bool ClapLogger::log_request(bool is_host_plugin,
     });
 }
 
-bool ClapLogger::log_request(bool is_host_plugin,
-                             const clap::factory::plugin_factory::Create& request) {
+bool ClapLogger::log_request(
+    bool is_host_plugin,
+    const clap::factory::plugin_factory::Create& request) {
     return log_request_base(is_host_plugin, [&](auto& message) {
         message << "clap_plugin_factory::create(host = <clap_host_t*>, "
                    "plugin_id = \""
@@ -328,20 +329,12 @@ bool ClapLogger::log_request(
     });
 }
 
-bool ClapLogger::log_request(bool is_host_plugin,
-                             const clap::ext::params::plugin::Count& request) {
-    return log_request_base(is_host_plugin, [&](auto& message) {
-        message << request.instance_id << ": clap_plugin_params::count()";
-    });
-}
-
 bool ClapLogger::log_request(
     bool is_host_plugin,
-    const clap::ext::params::plugin::GetInfo& request) {
+    const clap::ext::params::plugin::GetInfos& request) {
     return log_request_base(is_host_plugin, [&](auto& message) {
         message << request.instance_id
-                << ": clap_plugin_params::get_info(param_index = "
-                << request.param_index << ", *param_info)";
+                << ": clap_plugin_params::get_info(..., *param_info) (batched)";
     });
 }
 
@@ -942,13 +935,13 @@ void ClapLogger::log_response(
 
 void ClapLogger::log_response(
     bool is_host_plugin,
-    const clap::ext::params::plugin::GetInfoResponse& response) {
+    const clap::ext::params::plugin::GetInfosResponse& response,
+    bool from_cache) {
     log_response_base(is_host_plugin, [&](auto& message) {
-        if (response.result) {
-            message << "true, <clap_param_info_t* for \""
-                    << response.result->name << "\">";
-        } else {
-            message << "false";
+        message << "<clap_param_info_t*> for " << response.infos.size()
+                << " parameters";
+        if (from_cache) {
+            message << " (from cache)";
         }
     });
 }

src/common/logging/clap.h

@@ -80,8 +80,10 @@ class ClapLogger {
     // log message for the response together with the request.
 
     // Main thread control messages
-    bool log_request(bool is_host_plugin, const clap::factory::plugin_factory::List&);
-    bool log_request(bool is_host_plugin, const clap::factory::plugin_factory::Create&);
+    bool log_request(bool is_host_plugin,
+                     const clap::factory::plugin_factory::List&);
+    bool log_request(bool is_host_plugin,
+                     const clap::factory::plugin_factory::Create&);
     bool log_request(bool is_host_plugin, const clap::plugin::Init&);
     bool log_request(bool is_host_plugin, const clap::plugin::Destroy&);
     bool log_request(bool is_host_plugin, const clap::plugin::Activate&);
@@ -127,9 +129,7 @@ class ClapLogger {
     bool log_request(bool is_host_plugin,
                      const clap::ext::note_ports::plugin::Get&);
     bool log_request(bool is_host_plugin,
-                     const clap::ext::params::plugin::Count&);
-    bool log_request(bool is_host_plugin,
-                     const clap::ext::params::plugin::GetInfo&);
+                     const clap::ext::params::plugin::GetInfos&);
     bool log_request(bool is_host_plugin,
                      const clap::ext::params::plugin::GetValue&);
     bool log_request(bool is_host_plugin,
@@ -229,7 +229,8 @@ class ClapLogger {
     void log_response(bool is_host_plugin,
                       const clap::ext::note_ports::plugin::GetResponse&);
     void log_response(bool is_host_plugin,
-                      const clap::ext::params::plugin::GetInfoResponse&);
+                      const clap::ext::params::plugin::GetInfosResponse&,
+                      bool from_cache = false);
     void log_response(bool is_host_plugin,
                       const clap::ext::params::plugin::GetValueResponse&);
     void log_response(bool is_host_plugin,

src/common/serialization/clap.h

@@ -83,8 +83,7 @@ using ClapMainThreadControlRequest =
                  clap::ext::note_name::plugin::Get,
                  clap::ext::note_ports::plugin::Count,
                  clap::ext::note_ports::plugin::Get,
-                 clap::ext::params::plugin::Count,
-                 clap::ext::params::plugin::GetInfo,
+                 clap::ext::params::plugin::GetInfos,
                  clap::ext::params::plugin::GetValue,
                  clap::ext::params::plugin::ValueToText,
                  clap::ext::params::plugin::TextToValue,

src/common/serialization/clap/ext/params.h

@@ -79,45 +79,41 @@ struct ParamInfo {
 namespace plugin {
 
 /**
- * Message struct for `clap_plugin_params::count()`.
- */
-struct Count {
-    using Response = PrimitiveResponse<uint32_t>;
-
-    native_size_t instance_id;
-
-    template <typename S>
-    void serialize(S& s) {
-        s.value8b(instance_id);
-    }
-};
-
-/**
- * The response to the `clap::ext::params::plugin::GetInfo` message defined
+ * The response to the `clap::ext::params::plugin::GetInfos` message defined
  * below.
  */
-struct GetInfoResponse {
-    std::optional<ParamInfo> result;
+struct GetInfosResponse {
+    /**
+     * All of the plugin's parameter infos. If the plugin somehow returned an
+     * error for a parameter that should be in range, then this contains a
+     * nullopt value.
+     */
+    std::vector<std::optional<ParamInfo>> infos;
 
     template <typename S>
     void serialize(S& s) {
-        s.ext(result, bitsery::ext::InPlaceOptional());
+        s.container(infos, 1 << 16, [](S& s, auto& v) {
+            s.ext(v, bitsery::ext::InPlaceOptional{});
+        });
     }
 };
 
 /**
- * Message struct for `clap_plugin_params::get_info()`.
+ * Message struct for querying the information for all parameters using
+ * `clap_plugin_params::count()` and `clap_plugin_params::get_info()`. This
+ * information is then cached until the plugin tells the host that the
+ * parameters have changed. No specific plugins or hosts seem to require this at
+ * the moment, but this mimics the behavior of the VST3 bridge which needed this
+ * to work around a Kontakt bug.
  */
-struct GetInfo {
-    using Response = GetInfoResponse;
+struct GetInfos {
+    using Response = GetInfosResponse;
 
     native_size_t instance_id;
-    uint32_t param_index;
 
     template <typename S>
     void serialize(S& s) {
         s.value8b(instance_id);
-        s.value4b(param_index);
     }
 };
 

src/plugin/bridges/clap-impls/plugin-proxy.cpp

@@ -146,6 +146,11 @@ clap_plugin_proxy::clap_plugin_proxy(ClapPluginBridge& bridge,
       // getting that many of them
       pending_callbacks_(128) {}
 
+void clap_plugin_proxy::clear_param_info_cache() {
+    std::lock_guard lock(param_info_cache_mutex_);
+    param_info_cache_.clear();
+}
+
 bool CLAP_ABI clap_plugin_proxy::plugin_init(const struct clap_plugin* plugin) {
     assert(plugin && plugin->plugin_data);
     auto self = static_cast<clap_plugin_proxy*>(plugin->plugin_data);
@@ -719,10 +724,17 @@ clap_plugin_proxy::ext_note_ports_get(const clap_plugin_t* plugin,
 uint32_t CLAP_ABI
 clap_plugin_proxy::ext_params_count(const clap_plugin_t* plugin) {
     assert(plugin && plugin->plugin_data);
-    auto self = static_cast<const clap_plugin_proxy*>(plugin->plugin_data);
+    auto self = static_cast<clap_plugin_proxy*>(plugin->plugin_data);
 
-    return self->bridge_.send_main_thread_message(
-        clap::ext::params::plugin::Count{.instance_id = self->instance_id()});
+    // The infos for all of a plugin's parameters is queried in a batch and then
+    // cached. This was needed in the VST3 bridge to work around a bug in
+    // Kontakt (see the similarly named function there for more information),
+    // and the CAP bridge does the same thing for consistency's sake. This cache
+    // is cleared when the plugin asks the host to rescan its parameters.
+    self->maybe_query_parameter_info();
+
+    std::lock_guard lock(self->param_info_cache_mutex_);
+    return self->param_info_cache_.size();
 }
 
 bool CLAP_ABI
@@ -730,16 +742,18 @@ clap_plugin_proxy::ext_params_get_info(const clap_plugin_t* plugin,
                                        uint32_t param_index,
                                        clap_param_info_t* param_info) {
     assert(plugin && plugin->plugin_data && param_info);
-    auto self = static_cast<const clap_plugin_proxy*>(plugin->plugin_data);
+    auto self = static_cast<clap_plugin_proxy*>(plugin->plugin_data);
 
-    const clap::ext::params::plugin::GetInfoResponse response =
-        self->bridge_.send_main_thread_message(
-            clap::ext::params::plugin::GetInfo{
-                .instance_id = self->instance_id(),
-                .param_index = param_index});
-    if (response.result) {
-        response.result->reconstruct(*param_info);
+    // See above
+    self->maybe_query_parameter_info();
 
+    std::lock_guard lock(self->param_info_cache_mutex_);
+    if (param_index > self->param_info_cache_.size()) {
+        return false;
+    }
+
+    if (const auto& info = self->param_info_cache_[param_index]) {
+        info->reconstruct(*param_info);
         return true;
     } else {
         return false;
@@ -912,3 +926,18 @@ bool CLAP_ABI clap_plugin_proxy::ext_voice_info_get(const clap_plugin_t* plugin,
         return false;
     }
 }
+
+void clap_plugin_proxy::maybe_query_parameter_info() {
+    std::lock_guard lock(param_info_cache_mutex_);
+
+    // We'll assume that the plugin has at least one parameter. If it does not
+    // have any parameters then everything will work as expected, except that
+    // the parameter count is not cached.
+    if (param_info_cache_.empty()) {
+        const clap::ext::params::plugin::GetInfosResponse response =
+            bridge_.send_main_thread_message(
+                clap::ext::params::plugin::GetInfos{.instance_id =
+                                                        instance_id()});
+        param_info_cache_ = std::move(response.infos);
+    }
+}

src/plugin/bridges/clap-impls/plugin-proxy.h

@@ -36,6 +36,7 @@
 #include <rigtorp/MPMCQueue.h>
 #include <function2/function2.hpp>
 
+#include "../../common/serialization/clap/ext/params.h"
 #include "../../common/serialization/clap/plugin.h"
 
 // Forward declaration to avoid circular includes
@@ -157,6 +158,13 @@ class clap_plugin_proxy {
         return response_future;
     }
 
+    /**
+     * Clear the parameter information cache. Needs to be called when the plugin
+     * calls `clap_host_params::rescan()`. This cache is used to fetch
+     * information for all parameters at once.
+     */
+    void clear_param_info_cache();
+
     /**
      * The `clap_host_t*` passed when creating the instance. Any callbacks made
      * by the proxied plugin instance must go through here.
@@ -298,6 +306,14 @@ class clap_plugin_proxy {
                                             clap_voice_info_t* info);
 
    private:
+    /**
+     * Query information for all of the plugin's parameters and writes the
+     * results to `param_info_cache_` if necessary. Otherwise does nothing.
+     * Acquires a lock on the struct in the process, so it must not be locked
+     * before calling this function.
+     */
+    void maybe_query_parameter_info();
+
     ClapPluginBridge& bridge_;
     size_t instance_id_;
     clap::plugin::Descriptor descriptor_;
@@ -377,4 +393,14 @@ class clap_plugin_proxy {
      * socket needs to make a main thread function call, it will
      */
     rigtorp::MPMCQueue<HostCallback> pending_callbacks_;
+
+    /**
+     * Caches the info structs for all of a plugin's parameters. This is queried
+     * all at once the first time the interacts with the param extension. When
+     * the plugin asks the host to rescan its parameters, this cache is cleared.
+     * The `std::optional` is used to handle the case where a plugin may return
+     * `false` when querying info for a parameter that should be in range.
+     */
+    std::vector<std::optional<clap::ext::params::ParamInfo>> param_info_cache_;
+    std::mutex param_info_cache_mutex_;
 };

src/plugin/bridges/clap.cpp

@@ -251,8 +251,13 @@ ClapPluginBridge::ClapPluginBridge(const ghc::filesystem::path& plugin_path)
 
                     run_on_main_thread(
                         plugin_proxy,
-                        [&, host = plugin_proxy.host_,
+                        [&, plugin_proxy = &plugin_proxy,
+                         host = plugin_proxy.host_,
                          params = plugin_proxy.host_extensions_.params]() {
+                            // Parameter information is cached and fetched in
+                            // bulk as an optimization
+                            plugin_proxy->clear_param_info_cache();
+
                             params->rescan(host, request.flags);
                         })
                         .wait();

src/wine-host/bridges/clap.cpp

@@ -241,11 +241,11 @@ void ClapBridge::run() {
                         if (plugin) {
                             register_plugin_instance(plugin,
                                                      std::move(host_proxy));
-                            return clap::factory::plugin_factory::CreateResponse{
-                                .instance_id = instance_id};
+                            return clap::factory::plugin_factory::
+                                CreateResponse{.instance_id = instance_id};
                         } else {
-                            return clap::factory::plugin_factory::CreateResponse{
-                                .instance_id = std::nullopt};
+                            return clap::factory::plugin_factory::
+                                CreateResponse{.instance_id = std::nullopt};
                         }
                     })
                     .get();
@@ -716,30 +716,40 @@ void ClapBridge::run() {
                         .result = std::nullopt};
                 }
             },
-            [&](const clap::ext::params::plugin::Count& request)
-                -> clap::ext::params::plugin::Count::Response {
+            [&](const clap::ext::params::plugin::GetInfos& request)
+                -> clap::ext::params::plugin::GetInfos::Response {
                 const auto& [instance, _] = get_instance(request.instance_id);
 
                 // We'll ignore the main thread requirement for simple array
-                // lookups to avoid the synchronisation costs in hot code paths
-                return instance.extensions.params->count(instance.plugin.get());
-            },
-            [&](const clap::ext::params::plugin::GetInfo& request)
-                -> clap::ext::params::plugin::GetInfo::Response {
-                const auto& [instance, _] = get_instance(request.instance_id);
+                // lookups to avoid the synchronisation costs in hot code paths.
+                // We also deviate from yabridge's usual bridging approach by
+                // querying the infos for all parameters at once and then
+                // caching them until told to invalidate that cache. This was
+                // required to get acceptable performance when loading certain
+                // patches in Kontakt's VST3 version. To be consistent, CLAP
+                // bridging uses a similar caching mechanism.
+                // TODO: Also do this for VST2, will require replacing the VST2
+                //       bridging with an approach similar to VST3 and CLAP
+                const uint32_t num_parameters =
+                    instance.extensions.params->count(instance.plugin.get());
 
-                // We'll ignore the main thread requirement for simple array
-                // lookups to avoid the synchronisation costs in hot code paths
-                clap_param_info_t param_info{};
-                if (instance.extensions.params->get_info(instance.plugin.get(),
-                                                         request.param_index,
-                                                         &param_info)) {
-                    return clap::ext::params::plugin::GetInfoResponse{
-                        .result = param_info};
-                } else {
-                    return clap::ext::params::plugin::GetInfoResponse{
-                        .result = std::nullopt};
+                std::vector<std::optional<clap::ext::params::ParamInfo>> infos;
+                infos.reserve(num_parameters);
+                for (uint32_t i = 0; i < num_parameters; i++) {
+                    // This should never fail, but we can't make things up and
+                    // we don't want to change parameter orders around so we'll
+                    // store a nullopt if the plugin returns an error here
+                    clap_param_info_t info{};
+                    if (instance.extensions.params->get_info(
+                            instance.plugin.get(), i, &info)) {
+                        infos.push_back(std::move(info));
+                    } else {
+                        infos.push_back(std::nullopt);
+                    }
                 }
+
+                return clap::ext::params::plugin::GetInfosResponse{
+                    .infos = std::move(infos)};
             },
             [&](const clap::ext::params::plugin::GetValue& request)
                 -> clap::ext::params::plugin::GetValue::Response {