From 6266072641741caffafed68ec304429b0f16a47c Mon Sep 17 00:00:00 2001
From: Robbert van der Helm <mail@robbertvanderhelm.nl>
Date: Thu, 30 Apr 2020 20:55:49 +0200
Subject: [PATCH] Explicitly list opcodes that should return strings

The automatic detection works fine in every case I've tested other than
Fabfilter plugins, but this is probably for the best.
---
 src/common/events.h             | 12 ++++--------
 src/plugin/host-bridge.cpp      | 15 +++++++++++++++
 src/wine-host/plugin-bridge.cpp |  7 +++++++
 3 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/src/common/events.h b/src/common/events.h
index c78403bb..ca304fb9 100644
--- a/src/common/events.h
+++ b/src/common/events.h
@@ -43,14 +43,10 @@ class DefaultDataConverter {
             return nullptr;
         }
 
-        // Assume buffers are zeroed out, this is probably not the case
-        // FIXME: Some plugins, such as Fabfilter plugins, don't zero out their
-        //        string buffers (such as when calling
-        //        `audioMasterGetVendorString` and
-        //        `audioMasterGetProductString`). We'll have to either manually
-        //        specify which opcodes are expected to be strings or always
-        //        write back changed strings. I think the first choice is
-        //        cleaner.
+        // This is a simple fallback that will work in almost every case.
+        // Because some plugins don't zero out their string buffers when sending
+        // host callbacks, we will explicitely list all callbacks that expect a
+        // string in `DispatchDataConverter` adn `HostCallbackDataConverter`.
         const char* c_string = static_cast<const char*>(data);
         if (c_string[0] != 0) {
             return std::string(c_string);
diff --git a/src/plugin/host-bridge.cpp b/src/plugin/host-bridge.cpp
index e5f10780..bf732c25 100644
--- a/src/plugin/host-bridge.cpp
+++ b/src/plugin/host-bridge.cpp
@@ -241,6 +241,21 @@ class DispatchDataConverter : DefaultDataConverter {
                 break;
             case effGetMidiKeyName:
                 return *static_cast<const VstMidiKeyName*>(data);
+            // Any VST host I've encountered has properly zeroed out these their
+            // string buffers, but we'll add a list of opcodes that should
+            // return a string just in case `DefaultDataConverter::read()` can't
+            // figure it out.
+            case effGetProgramName:
+            case effGetParamLabel:
+            case effGetParamDisplay:
+            case effGetParamName:
+            case effGetProgramNameIndexed:
+            case effGetEffectName:
+            case effGetVendorString:
+            case effGetProductString:
+            case effShellGetNextPlugin:
+                return WantsString{};
+                break;
             default:
                 return DefaultDataConverter::read(opcode, index, value, data);
                 break;
diff --git a/src/wine-host/plugin-bridge.cpp b/src/wine-host/plugin-bridge.cpp
index 830a5fbb..fc699900 100644
--- a/src/wine-host/plugin-bridge.cpp
+++ b/src/wine-host/plugin-bridge.cpp
@@ -352,6 +352,13 @@ class HostCallbackDataConverter : DefaultDataConverter {
                 // done inside of `passthrough_event`.
                 return AEffect(*plugin);
                 break;
+            // We detect whether an opcode should return a string by checking
+            // whether there's a zeroed out buffer behind the void pointer. This
+            // works for any host, but not all plugins zero out their buffers.
+            case audioMasterGetVendorString:
+            case audioMasterGetProductString:
+                return WantsString{};
+                break;
             default:
                 return DefaultDataConverter::read(opcode, index, value, data);
                 break;