From f53f8b62364a60f78b9b67beec48ba64aff8e05a Mon Sep 17 00:00:00 2001 From: Zeyu Yang <40936154+zeyuyang42@users.noreply.github.com> Date: Wed, 26 Aug 2026 18:28:33 +0200 Subject: [PATCH] Make the combination mode switchable at runtime The mode was bound once in EnvelopeCombinator::initialize(), which resolved a string to a member-function pointer. Nothing could change it afterwards, so "add a message for mode change" needed the core to grow a setter first. Replace the member-function pointer with a CombMode enum held in a std::atomic, dispatched by a switch in perform(). Not std::atomic of the pointer-to-member: that is 16 bytes on the Itanium ABI and would not be lock-free, so set_mode() could block the audio thread on an internal lock. A static_assert pins the lock-free requirement. perform() loads the mode once per block, so a block is never processed partly in one mode and partly in another, and set_mode() is safe to call from a control thread with DSP running. This also removes existing UB. processFunc had no in-class initializer, so the `if (processFunc)` guard in perform() was reading an indeterminate value rather than testing for null. The mode now has a valid default and a `prepared` flag records whether initialize() actually ran. The constructor signature is unchanged and initialize() still returns false on an unknown mode name, which is what the Pd wrapper gates on, so Pd is unaffected -- verified by building it. CombMode, tryParseCombMode/parseCombMode, toString and combModeNames live in types.h next to the existing GenMode parser. That answers the review point on the previous commit: the Max wrapper had its own copy of the mode-name table so that a typo could be caught at object-creation time rather than at DSP start. Both the table and the hardcoded "add, root or max" error text are gone, and adding a mode is now a one-place edit in types.h. The two parse entry points are deliberate -- the throwing one for construction, the noexcept one for the message handler, which runs on a host's C stack where an escaping exception would terminate the process. On the Max side: a "mode" message plus a matching "mode" attribute, both routed through one helper so they cannot diverge, so `mode root`, `@mode root` and the inspector all work. No deferral -- the core publishes through a lock-free atomic, so switching mid-stream is safe and deferring would only add latency. EnvelopeCombinator::perform now takes and returns by reference. Routing Max through the core had reintroduced a per-block copy in each direction; the Max wrapper reads the core's output buffer directly and no longer keeps an output buffer of its own. The remaining input copy is elementwise and allocation-free. Help patch: mode message boxes wired into the left inlet of the `2 max` example. Refs Zerr* board ID 136. Co-Authored-By: Claude Opus 5 (1M context) --- core/include/modules/envelopecombinator.h | 51 ++++++++++-- core/include/utils/types.h | 72 +++++++++++++++++ core/src/modules/envelopecombinator.cpp | 60 ++++++++++---- maxmsp/help/mc.zerr.combinator~.maxhelp | 72 +++++++++++++++++ .../mc.zerr.combinator_tilde.cpp | 52 ++++++++++++- .../zerr_combinator.hpp | 78 ++++++++++--------- 6 files changed, 323 insertions(+), 62 deletions(-) diff --git a/core/include/modules/envelopecombinator.h b/core/include/modules/envelopecombinator.h index 7d04cdb..b4560ab 100644 --- a/core/include/modules/envelopecombinator.h +++ b/core/include/modules/envelopecombinator.h @@ -12,7 +12,7 @@ #include "logger.h" #include "types.h" #include "utils.h" -#include +#include namespace zerr { /** @@ -45,19 +45,56 @@ class EnvelopeCombinator { /** * @brief Process input envelope blocks and combine using selected mode * @param in Input envelope blocks to process - * @return Combined output envelope blocks + * @return Reference to the internal output buffer, valid until the next perform() + * + * Taken and returned by reference so that nothing is allocated on the audio thread. */ - Blocks perform(Blocks in); + const Blocks& perform(const Blocks& in); + /** + * @brief Change the combination mode + * @param mode Mode name: "add", "root" or "max" + * @return false, leaving the mode unchanged, if the name is not a known mode + * + * Safe to call from a control thread while perform() runs on an audio thread: the + * mode is a lock-free atomic that perform() loads once per block, so a block is + * never processed partly in one mode and partly in another. + */ + bool set_mode(const std::string& mode) noexcept; + /** + * @brief Change the combination mode + * @param mode The mode to switch to + */ + void set_mode(CombMode mode) noexcept; + /** + * @brief Get the combination mode currently in use + */ + [[nodiscard]] CombMode get_mode() const noexcept + { + return combMode.load(std::memory_order_relaxed); + } + /** + * @brief Get the name of the combination mode currently in use + * @return "add", "root" or "max" + */ + [[nodiscard]] const char* get_mode_name() const noexcept { return toString(get_mode()); } ~EnvelopeCombinator() = default; private: - using ProcessFunction = void (EnvelopeCombinator::*)(); - ProcessFunction processFunc; - int numSource; /**< Number of envelope sources to combine */ int numChannel; /**< Number of channels per source */ zerr::SystemConfigs systemCfgs; /**< system configuration: sample_rate, block_size */ - std::string combMode; /**< Mode for combining envelopes */ + + /**< Mode for combining envelopes. Atomic because a control thread may change it while + the audio thread is inside perform(). */ + std::atomic combMode{CombMode::Max}; + static_assert(std::atomic::is_always_lock_free, + "CombMode must be lock-free so set_mode() never blocks the audio thread"); + + bool modeValid{true}; /**< False when the constructor was given an unknown mode name */ + /**< The mode name as given to the constructor, kept only so initialize() can name it in + its error message. Control thread only -- never read while perform() runs. */ + std::string requestedModeName; + bool prepared{false}; /**< True once initialize() has sized the buffers */ Logger logger; /**< Logger instance for debug/error messages */ diff --git a/core/include/utils/types.h b/core/include/utils/types.h index 4c54043..c5d1d3e 100644 --- a/core/include/utils/types.h +++ b/core/include/utils/types.h @@ -60,6 +60,78 @@ inline GenMode parseGenMode(const std::string& s) throw std::invalid_argument("Unknown GenMode: " + s); } +/**< Strategy for combining envelopes coming from several sources */ +enum class CombMode { + Add, /**< Sum the envelopes across sources */ + Root, /**< Geometric mean: the Nth root of the N sources multiplied together */ + Max, /**< Largest envelope value across sources */ +}; + +/** + * @brief Parse a string into a CombMode enum value without throwing + * @param s The string to parse ("add", "root" or "max") + * @param out Receives the parsed value; left untouched when the string is not a mode + * @return true if the string named a mode + * + * The non-throwing counterpart of parseCombMode, for callers on a host's C callback + * stack where an escaping exception would terminate the process. + */ +inline bool tryParseCombMode(const std::string& s, CombMode& out) noexcept +{ + if (s == "add") { + out = CombMode::Add; + return true; + } + if (s == "root") { + out = CombMode::Root; + return true; + } + if (s == "max") { + out = CombMode::Max; + return true; + } + return false; +} + +/** + * @brief Parse a string into a CombMode enum value + * @param s The string to parse ("add", "root" or "max") + * @return CombMode The corresponding enum value + * @throws std::invalid_argument if the string is not a valid mode + */ +inline CombMode parseCombMode(const std::string& s) +{ + CombMode mode; + if (!tryParseCombMode(s, mode)) { + throw std::invalid_argument("Unknown CombMode: " + s); + } + return mode; +} + +/** + * @brief Render a CombMode as the string that names it + */ +inline const char* toString(CombMode mode) noexcept +{ + switch (mode) { + case CombMode::Add: + return "add"; + case CombMode::Root: + return "root"; + case CombMode::Max: + return "max"; + } + return "max"; +} + +/** + * @brief The supported combination modes as a human-readable list, for error messages + * + * Single source of truth for wrappers reporting a bad mode, so a new mode only has to be + * added here and in tryParseCombMode. + */ +inline const char* combModeNames() noexcept { return "add, root or max"; } + using ConfigPath = std::string; /**< Path string for configuration files */ using Params = std::vector; /**< Vector container for parameter values */ diff --git a/core/src/modules/envelopecombinator.cpp b/core/src/modules/envelopecombinator.cpp index 679eef4..35d19d7 100644 --- a/core/src/modules/envelopecombinator.cpp +++ b/core/src/modules/envelopecombinator.cpp @@ -15,7 +15,15 @@ EnvelopeCombinator::EnvelopeCombinator(int numSource, int numChannel, SystemConf this->numSource = numSource; this->numChannel = numChannel; this->systemCfgs = systemCfgs; - this->combMode = combMode; + + // Parse now, report in initialize(). The constructor cannot fail, and wrappers + // already gate on initialize()'s return value. + CombMode parsed; + modeValid = tryParseCombMode(combMode, parsed); + if (modeValid) { + this->combMode.store(parsed, std::memory_order_relaxed); + } + this->requestedModeName = combMode; numInlet = numSource * numChannel; numOutlet = numChannel; @@ -30,29 +38,51 @@ bool EnvelopeCombinator::initialize() inputBuffer.resize(numInlet, Samples(systemCfgs.block_size, 0.0f)); outputBuffer.resize(numOutlet, Samples(systemCfgs.block_size, 0.0f)); - if (combMode == "add") { - processFunc = &EnvelopeCombinator::_process_add; - } - else if (combMode == "root") { - processFunc = &EnvelopeCombinator::_process_root; - } - else if (combMode == "max") { - processFunc = &EnvelopeCombinator::_process_max; - } - else { - logger.logError("EnvelopeCombinator::initialize Unknown combination mode: " + combMode); + if (!modeValid) { + logger.logError("EnvelopeCombinator::initialize Unknown combination mode: " + + requestedModeName); return false; } + prepared = true; + return true; +} + +bool EnvelopeCombinator::set_mode(const std::string& mode) noexcept +{ + CombMode parsed; + if (!tryParseCombMode(mode, parsed)) { + return false; + } + set_mode(parsed); return true; } -Blocks EnvelopeCombinator::perform(Blocks in) +void EnvelopeCombinator::set_mode(CombMode mode) noexcept +{ + // Relaxed is enough: the enum is the only datum published, and it guards nothing else. + combMode.store(mode, std::memory_order_relaxed); +} + +const Blocks& EnvelopeCombinator::perform(const Blocks& in) { + if (!prepared) { + return outputBuffer; + } + inputBuffer = in; - if (processFunc) { - (this->*processFunc)(); + // Load once per block so a block is never processed partly in one mode. + switch (combMode.load(std::memory_order_relaxed)) { + case CombMode::Add: + _process_add(); + break; + case CombMode::Root: + _process_root(); + break; + case CombMode::Max: + _process_max(); + break; } return outputBuffer; diff --git a/maxmsp/help/mc.zerr.combinator~.maxhelp b/maxmsp/help/mc.zerr.combinator~.maxhelp index acdbdb5..6369805 100644 --- a/maxmsp/help/mc.zerr.combinator~.maxhelp +++ b/maxmsp/help/mc.zerr.combinator~.maxhelp @@ -478,6 +478,57 @@ "sig" : 0.0 } + } +, { + "box" : { + "fontsize" : 14.0, + "id" : "obj-60", + "maxclass" : "comment", + "numinlets" : 1, + "numoutlets" : 0, + "patching_rect" : [ 560.0, 378.0, 280.0, 20.0 ], + "text" : "change mode while DSP is running:" + } + + } +, { + "box" : { + "fontsize" : 14.0, + "id" : "obj-61", + "maxclass" : "message", + "numinlets" : 2, + "numoutlets" : 1, + "outlettype" : [ "" ], + "patching_rect" : [ 560.0, 404.0, 78.0, 24.0 ], + "text" : "mode max" + } + + } +, { + "box" : { + "fontsize" : 14.0, + "id" : "obj-62", + "maxclass" : "message", + "numinlets" : 2, + "numoutlets" : 1, + "outlettype" : [ "" ], + "patching_rect" : [ 646.0, 404.0, 84.0, 24.0 ], + "text" : "mode root" + } + + } +, { + "box" : { + "fontsize" : 14.0, + "id" : "obj-63", + "maxclass" : "message", + "numinlets" : 2, + "numoutlets" : 1, + "outlettype" : [ "" ], + "patching_rect" : [ 738.0, 404.0, 78.0, 24.0 ], + "text" : "mode add" + } + } ], "lines" : [ { @@ -493,6 +544,27 @@ "source" : [ "obj-28", 0 ] } + } +, { + "patchline" : { + "destination" : [ "obj-28", 0 ], + "source" : [ "obj-61", 0 ] + } + + } +, { + "patchline" : { + "destination" : [ "obj-28", 0 ], + "source" : [ "obj-62", 0 ] + } + + } +, { + "patchline" : { + "destination" : [ "obj-28", 0 ], + "source" : [ "obj-63", 0 ] + } + } , { "patchline" : { diff --git a/maxmsp/source/projects/mc.zerr.combinator_tilde/mc.zerr.combinator_tilde.cpp b/maxmsp/source/projects/mc.zerr.combinator_tilde/mc.zerr.combinator_tilde.cpp index dab26bd..1f97b52 100644 --- a/maxmsp/source/projects/mc.zerr.combinator_tilde/mc.zerr.combinator_tilde.cpp +++ b/maxmsp/source/projects/mc.zerr.combinator_tilde/mc.zerr.combinator_tilde.cpp @@ -57,6 +57,10 @@ void zerr_combinator_perform64(t_zerr_combinator* x, t_object* dsp64, double** i void zerr_combinator_bang(t_zerr_combinator* x); +void zerr_combinator_mode(t_zerr_combinator* x, t_symbol* msg, long argc, t_atom* argv); + +t_max_err zerr_combinator_mode_set(t_zerr_combinator* x, t_object* attr, long argc, t_atom* argv); + long zerr_combinator_multichanneloutputs(t_zerr_combinator* x, long outletindex); long zerr_combinator_inputchanged(t_zerr_combinator* x, long index, long count); @@ -82,6 +86,7 @@ C74_EXPORT void ext_main(void* r) 0); class_addmethod(c, (method)zerr_combinator_inputchanged, "inputchanged", A_CANT, 0); class_addmethod(c, (method)zerr_combinator_bang, "bang", 0); + class_addmethod(c, (method)zerr_combinator_mode, "mode", A_GIMME, 0); // Attributes. "chans" is derived from the inlets' channel count in dsp64, so it is // readable but not settable -- writing it would only desync it from the real signal. @@ -89,6 +94,14 @@ C74_EXPORT void ext_main(void* r) CLASS_ATTR_LABEL(c, "chans", 0, "Output Channels (read-only)"); CLASS_ATTR_BASIC(c, "chans", 0); + // The same switch as the "mode" message, so it also works from the inspector and as + // an @mode attribute at object creation. + CLASS_ATTR_SYM(c, "mode", 0, t_zerr_combinator, mode_sym); + CLASS_ATTR_ACCESSORS(c, "mode", NULL, zerr_combinator_mode_set); + CLASS_ATTR_ENUM(c, "mode", 0, "\"add\" \"root\" \"max\""); + CLASS_ATTR_LABEL(c, "mode", 0, "Combination Mode"); + CLASS_ATTR_BASIC(c, "mode", 0); + // Initialize DSP and register the class class_dspinit(c); class_register(CLASS_BOX, c); @@ -144,13 +157,14 @@ void* zerr_combinator_new(t_symbol* s, long argc, t_atom* argv) return NULL; } const char* requested = atom_getsym(argv + 1)->s_name; - if (ZerrCombinator::isValidMode(requested)) { + zerr::CombMode parsed; + if (zerr::tryParseCombMode(requested, parsed)) { mode = requested; } else { object_error((t_object*)x, "unknown combination mode '%s'; expected %s. Using '%s'.", requested, - ZerrCombinator::modeNames(), mode); + zerr::combModeNames(), mode); } } x->mode_sym = gensym(mode); @@ -217,6 +231,40 @@ void zerr_combinator_bang(t_zerr_combinator* x) x->mode_sym ? x->mode_sym->s_name : "unknown", x->input_count, x->channel_count); } +/** + * Applies a combination mode by name. Shared by the "mode" message and the "mode" + * attribute so the two can never diverge. Control thread only -- the core publishes the + * change through a lock-free atomic, so no deferral is needed and DSP can keep running. + */ +static bool zerr_combinator_apply_mode(t_zerr_combinator* x, t_symbol* mode) +{ + if (!mode || !x->zc || !x->zc->setMode(mode->s_name)) { + object_error((t_object*)x, "mode: unknown combination mode '%s'; expected %s", + mode ? mode->s_name : "", zerr::combModeNames()); + return false; + } + x->mode_sym = mode; + return true; +} + +void zerr_combinator_mode(t_zerr_combinator* x, t_symbol* msg, long argc, t_atom* argv) +{ + if (argc < 1 || atom_gettype(argv) != A_SYM) { + object_error((t_object*)x, "mode: expects one symbol (%s)", zerr::combModeNames()); + return; + } + zerr_combinator_apply_mode(x, atom_getsym(argv)); +} + +t_max_err zerr_combinator_mode_set(t_zerr_combinator* x, t_object* attr, long argc, t_atom* argv) +{ + if (argc < 1 || !argv || atom_gettype(argv) != A_SYM) { + object_error((t_object*)x, "mode: expects one symbol (%s)", zerr::combModeNames()); + return MAX_ERR_GENERIC; + } + return zerr_combinator_apply_mode(x, atom_getsym(argv)) ? MAX_ERR_NONE : MAX_ERR_GENERIC; +} + //------------------------------------------------------------------------------ // Multichannel Methods //------------------------------------------------------------------------------ diff --git a/maxmsp/source/projects/mc.zerr.combinator_tilde/zerr_combinator.hpp b/maxmsp/source/projects/mc.zerr.combinator_tilde/zerr_combinator.hpp index 1a9854d..081401a 100644 --- a/maxmsp/source/projects/mc.zerr.combinator_tilde/zerr_combinator.hpp +++ b/maxmsp/source/projects/mc.zerr.combinator_tilde/zerr_combinator.hpp @@ -42,15 +42,12 @@ class ZerrCombinator { * @param mode Combination mode: "add", "root" or "max" * @throws std::invalid_argument if inputCount < 1 or mode is not a known mode */ - ZerrCombinator(int inputCount, std::string mode) - : inputCount{inputCount}, combMode{std::move(mode)} + ZerrCombinator(int inputCount, const std::string& mode) + : inputCount{inputCount}, combMode{zerr::parseCombMode(mode)} { if (inputCount < 1) { throw std::invalid_argument("ZerrCombinator: inputCount must be at least 1"); } - if (!isValidMode(combMode)) { - throw std::invalid_argument("ZerrCombinator: unknown combination mode: " + combMode); - } } // Neither copyable nor movable: holds an atomic and is owned by a raw pointer in the @@ -60,25 +57,6 @@ class ZerrCombinator { ZerrCombinator(ZerrCombinator&&) = delete; ZerrCombinator& operator=(ZerrCombinator&&) = delete; - /** - * @brief Checks a combination mode name without constructing anything - * @param mode Mode name to check - * @return true if the core supports this mode - * - * Duplicates the mode names in zerr::EnvelopeCombinator::initialize() so that a typo in - * an object argument can be reported at creation time rather than at DSP start. Replace - * this with a core-side parser once zerr::EnvelopeCombinator exposes one. - */ - [[nodiscard]] static bool isValidMode(const std::string& mode) noexcept - { - return mode == "add" || mode == "root" || mode == "max"; - } - - /** - * @brief Human-readable list of the supported modes, for error messages - */ - [[nodiscard]] static const char* modeNames() noexcept { return "add, root or max"; } - /** * @brief Builds (or rebuilds) the core module for a given signal shape * @param cfg Sample rate and block size, as reported by dsp64 @@ -100,14 +78,13 @@ class ZerrCombinator { return true; } - auto next = - std::make_unique(inputCount, numChannel, cfg, combMode); + auto next = std::make_unique(inputCount, numChannel, cfg, + zerr::toString(combMode)); if (!next->initialize()) { return false; } zerr::Blocks nextIn(next->numInlet, zerr::Samples(cfg.block_size, 0.0)); - zerr::Blocks nextOut(next->numOutlet, zerr::Samples(cfg.block_size, 0.0)); // MSP rebuilds the DSP chain around dsp64, so perform() is not running for this // object while we swap. The flag covers the states MSP does not: never prepared, @@ -115,7 +92,6 @@ class ZerrCombinator { ready.store(false, std::memory_order_release); combinator.swap(next); inputBuffer.swap(nextIn); - outputBuffer.swap(nextOut); systemConfigs = cfg; this->numChannel = numChannel; ready.store(true, std::memory_order_release); @@ -137,7 +113,7 @@ class ZerrCombinator { void perform(double** ins, long numins, double** outs, long numouts, long sampleframes) noexcept { const long numIn = static_cast(inputBuffer.size()); - const long numOut = static_cast(outputBuffer.size()); + const long numOut = numChannel; if (!ready.load(std::memory_order_acquire) || !ins || !outs || numins < numIn || numouts < numOut) { @@ -157,13 +133,18 @@ class ZerrCombinator { std::copy_n(ins[i], count, inputBuffer[i].begin()); } - outputBuffer = combinator->perform(inputBuffer); + // Read the core's buffer directly rather than copying it into one of our own. + const zerr::Blocks& combined = combinator->perform(inputBuffer); + if (static_cast(combined.size()) < numOut) { + silence(outs, numouts, sampleframes); + return; + } for (long i = 0; i < numOut; ++i) { if (!outs[i]) { continue; } - std::copy_n(outputBuffer[i].begin(), count, outs[i]); + std::copy_n(combined[i].begin(), count, outs[i]); if (count < sampleframes) { std::memset(outs[i] + count, 0, (sampleframes - count) * sizeof(double)); } @@ -171,9 +152,31 @@ class ZerrCombinator { } /** - * @brief Gets the combination mode this object was created with + * @brief Changes the combination mode, with or without DSP running + * @param mode Mode name: "add", "root" or "max" + * @return false, leaving the mode unchanged, if the name is not a known mode + * + * Control thread only. Nothing is rebuilt: the core swaps a lock-free atomic, and the + * mode is remembered here too so that a change made before DSP ever started is applied + * by the next prepare(). + */ + bool setMode(const std::string& mode) noexcept + { + zerr::CombMode parsed; + if (!zerr::tryParseCombMode(mode, parsed)) { + return false; + } + combMode = parsed; + if (combinator) { + combinator->set_mode(parsed); + } + return true; + } + + /** + * @brief Gets the combination mode currently in use */ - [[nodiscard]] const std::string& getMode() const noexcept { return combMode; } + [[nodiscard]] const char* getMode() const noexcept { return zerr::toString(combMode); } /** * @brief Gets the number of envelope sets being combined @@ -203,16 +206,15 @@ class ZerrCombinator { } } - int inputCount; /**< Number of envelope sets, one multichannel inlet each */ - int numChannel{0}; /**< Envelopes per set; known only once prepare() has run */ - std::string combMode; /**< Combination mode: "add", "root" or "max" */ + int inputCount; /**< Number of envelope sets, one multichannel inlet each */ + int numChannel{0}; /**< Envelopes per set; known only once prepare() has run */ + zerr::CombMode combMode; /**< Combination mode; control thread only, mirrored into the core */ zerr::SystemConfigs systemConfigs{0, 0}; /**< System configuration settings */ std::atomic ready{false}; /**< True once the core module is built and buffers sized */ - zerr::Blocks inputBuffer; /**< Buffer for storing incoming audio samples */ - zerr::Blocks outputBuffer; /**< Multi-channel buffer for storing processed audio samples */ + zerr::Blocks inputBuffer; /**< Buffer for storing incoming audio samples */ std::unique_ptr combinator; /**< Core combination algorithms */ };