Add Hold phase to envelope and API refactoring

This converts 4-stage ADSR to 5-stage AHDSR envelope with Hold phase
between Attack and Decay. The Hold phase maintains peak level for a
configurable duration before decay begins.
- Replace positional parameters with picosynth_env_params_t struct
- Replace picosynth_env_ms_params_t for millisecond-based init
- Add hold/hold_ms fields to both parameter structs

Author: jserv

README.md

@@ -76,7 +76,10 @@ picosynth_node_t *env = picosynth_voice_get_node(v, 0);
 picosynth_node_t *osc = picosynth_voice_get_node(v, 1);
 
 /* Initialize nodes */
-picosynth_init_env(env, NULL, 5000, 500, Q15_MAX/2, 500);
+picosynth_init_env(env, NULL,
+    &(picosynth_env_params_t){.attack=5000, .decay=500, .sustain=Q15_MAX/2,
+                              .release=500,
+                             });
 picosynth_init_osc(osc, &env->out, picosynth_voice_freq_ptr(v),
                    picosynth_wave_sine);
 picosynth_voice_set_out(v, 1);

include/picosynth.h

@@ -11,7 +11,10 @@
  *   picosynth_node_t *osc = picosynth_voice_get_node(v, 1);
  *   picosynth_node_t *flt = picosynth_voice_get_node(v, 2);
  *
- *   picosynth_init_env_ms(env, NULL, 10, 100, 80, 50);
+ *   picosynth_init_env_ms(env, NULL,
+ *       &(picosynth_env_ms_params_t){.atk_ms=10, .dec_ms=100,
+ *                                    .sus_pct=80, .rel_ms=50,
+ *                                   });
  *   picosynth_init_osc(osc, &env->out, picosynth_voice_freq_ptr(v),
  *                      picosynth_wave_sine);
  *   picosynth_init_lp(flt, NULL, &osc->out, 5000);
@@ -81,18 +84,30 @@ static inline q15_t q15_sat(int32_t x)
 /* Waveform generator function pointer */
 typedef q15_t (*picosynth_wave_func_t)(q15_t phase);
 
+/* AHDSR envelope parameters for initialization.
+ * Using a struct avoids long parameter lists and enables named initialization.
+ */
+typedef struct {
+    int32_t attack;  /* Attack rate (higher = faster attack) */
+    int32_t hold;    /* Hold duration in samples (0 = no hold) */
+    int32_t decay;   /* Decay rate (higher = faster decay) */
+    q15_t sustain;   /* Sustain level (negative inverts output) */
+    int32_t release; /* Release rate (higher = faster release) */
+} picosynth_env_params_t;
+
 /* Oscillator state */
 typedef struct {
     const q15_t *freq;          /* Phase increment (frequency control) */
     const q15_t *detune;        /* Optional FM/detune offset */
     picosynth_wave_func_t wave; /* Waveform generator (phase -> sample) */
 } picosynth_osc_t;
 
-/* ADSR envelope state. Rates are step values scaled <<4 internally.
- * Use synth_init_env_ms().
+/* AHDSR envelope state (Attack-Hold-Decay-Sustain-Release).
+ * Rates are step values scaled <<4 internally. Use synth_init_env_ms().
  */
 typedef struct {
     int32_t attack;      /* Ramp-up rate */
+    int32_t hold;        /* Hold duration in samples (at peak before decay) */
     int32_t decay;       /* Ramp-down rate to sustain */
     q15_t sustain;       /* Hold level (negative inverts output) */
     int32_t release;     /* Ramp-down rate after note-off */
@@ -101,6 +116,7 @@ typedef struct {
     /* Block processing state (computed at block boundaries) */
     int32_t block_rate;    /* Current per-sample rate */
     uint8_t block_counter; /* Samples until next rate computation */
+    int32_t hold_counter;  /* Remaining hold samples (runtime state) */
 } picosynth_env_t;
 
 /* Single-pole filter state */
@@ -197,21 +213,29 @@ void picosynth_init_osc(picosynth_node_t *n,
                         const q15_t *freq,
                         picosynth_wave_func_t wave);
 
-/* Initialize envelope node (rates as increments per sample, scaled) */
+/* Initialize AHDSR envelope node with parameter struct.
+ * @params: Pointer to envelope parameters (attack, hold, decay, sustain,
+ * release). Rates are increments per sample, scaled <<4 internally.
+ */
 void picosynth_init_env(picosynth_node_t *n,
                         const q15_t *gain,
-                        int32_t attack,
-                        int32_t decay,
-                        q15_t sustain,
-                        int32_t release);
+                        const picosynth_env_params_t *params);
 
-/* Initialize envelope with millisecond timings and percentage sustain */
+/* Millisecond-based envelope parameters for picosynth_init_env_ms(). */
+typedef struct {
+    uint16_t atk_ms;  /* Attack time in milliseconds */
+    uint16_t hold_ms; /* Hold time in milliseconds (0 = no hold) */
+    uint16_t dec_ms;  /* Decay time in milliseconds */
+    uint8_t sus_pct;  /* Sustain level as percentage (0-100) */
+    uint16_t rel_ms;  /* Release time in milliseconds */
+} picosynth_env_ms_params_t;
+
+/* Initialize envelope with millisecond timings and percentage sustain.
+ * Converts timing parameters to internal rate values.
+ */
 void picosynth_init_env_ms(picosynth_node_t *n,
                            const q15_t *gain,
-                           uint16_t atk_ms,
-                           uint16_t dec_ms,
-                           uint8_t sus_pct,
-                           uint16_t rel_ms);
+                           const picosynth_env_ms_params_t *params);
 
 /* Initialize low-pass filter node */
 void picosynth_init_lp(picosynth_node_t *n,

src/picosynth.c

@@ -6,9 +6,16 @@
 #include "dsp-math.h"
 #include "picosynth.h"
 
-/* Envelope state: bit 31 = decay mode, bits 0-30 = value */
-#define ENVELOPE_STATE_MODE_BIT 0x80000000u
-#define ENVELOPE_STATE_VALUE_MASK 0x7FFFFFFF
+/* Envelope state: bits 30-31 = mode, bits 0-29 = value
+ * Mode 0 (0x00): Attack - ramp up to peak
+ * Mode 1 (0x40): Hold - maintain peak level
+ * Mode 2 (0x80): Decay/Sustain - exponential decay to sustain
+ */
+#define ENVELOPE_MODE_ATTACK 0x00000000u
+#define ENVELOPE_MODE_HOLD 0x40000000u
+#define ENVELOPE_MODE_DECAY 0x80000000u
+#define ENVELOPE_MODE_MASK 0xC0000000u
+#define ENVELOPE_STATE_VALUE_MASK 0x3FFFFFFF
 
 /* DC blocker coefficient: alpha ≈ 0.995 in Q15 = 32604
  * Removes DC offset from waveshaping with ~4 cycles/sample overhead.
@@ -67,6 +74,7 @@ static void voice_note_on(picosynth_voice_t *v, uint8_t note)
         if (n->type == PICOSYNTH_NODE_ENV) {
             n->env.block_counter = 0;
             n->env.block_rate = 0;
+            n->env.hold_counter = 0;
         }
     }
 }
@@ -403,33 +411,32 @@ void picosynth_init_osc(picosynth_node_t *n,
 
 void picosynth_init_env(picosynth_node_t *n,
                         const q15_t *gain,
-                        int32_t attack,
-                        int32_t decay,
-                        q15_t sustain,
-                        int32_t release)
+                        const picosynth_env_params_t *params)
 {
     memset(n, 0, sizeof(picosynth_node_t));
     n->gain = gain;
     n->type = PICOSYNTH_NODE_ENV;
-    n->env.attack = attack;
-    n->env.decay = decay;
-    n->env.sustain = sustain;
-    n->env.release = release;
+    n->env.attack = params->attack;
+    n->env.hold = params->hold;
+    n->env.decay = params->decay;
+    n->env.sustain = params->sustain;
+    n->env.release = params->release;
+    n->env.hold_counter = 0;
     env_update_exp_coeffs(&n->env);
 }
 
 void picosynth_init_env_ms(picosynth_node_t *n,
                            const q15_t *gain,
-                           uint16_t atk_ms,
-                           uint16_t dec_ms,
-                           uint8_t sus_pct,
-                           uint16_t rel_ms)
+                           const picosynth_env_ms_params_t *params)
 {
-    int32_t atk = (int32_t) PICOSYNTH_ENV_RATE_FROM_MS(atk_ms);
-    int32_t dec = (int32_t) PICOSYNTH_ENV_RATE_FROM_MS(dec_ms);
-    q15_t sus = (q15_t) (((int32_t) sus_pct * Q15_MAX) / 100);
-    int32_t rel = (int32_t) PICOSYNTH_ENV_RATE_FROM_MS(rel_ms);
-    picosynth_init_env(n, gain, atk, dec, sus, rel);
+    picosynth_env_params_t p = {
+        .attack = (int32_t) PICOSYNTH_ENV_RATE_FROM_MS(params->atk_ms),
+        .hold = (int32_t) PICOSYNTH_MS(params->hold_ms),
+        .decay = (int32_t) PICOSYNTH_ENV_RATE_FROM_MS(params->dec_ms),
+        .sustain = (q15_t) (((int32_t) params->sus_pct * Q15_MAX) / 100),
+        .release = (int32_t) PICOSYNTH_ENV_RATE_FROM_MS(params->rel_ms),
+    };
+    picosynth_init_env(n, gain, &p);
 }
 
 void picosynth_init_lp(picosynth_node_t *n,
@@ -712,29 +719,30 @@ q15_t picosynth_process(picosynth_t *s)
                     (int32_t) (((uint32_t) n->state) & (uint32_t) Q15_MAX);
                 break;
             case PICOSYNTH_NODE_ENV: {
-                /* Block-based envelope: compute rate at block boundaries,
-                 * check for phase transitions per-sample. */
+                /* Block-based AHDSR envelope: compute rate at block
+                 * boundaries, check for phase transitions per-sample. */
+                uint32_t mode = ((uint32_t) n->state) & ENVELOPE_MODE_MASK;
 
                 /* Recompute rate at block boundary */
                 if (n->env.block_counter == 0) {
                     n->env.block_counter = PICOSYNTH_BLOCK_SIZE;
                     if (!v->gate) {
                         n->env.block_rate = -n->env.release; /* Informational */
-                    } else if (((uint32_t) n->state) &
-                               ENVELOPE_STATE_MODE_BIT) {
+                    } else if (mode == ENVELOPE_MODE_DECAY) {
                         n->env.block_rate = -n->env.decay; /* Informational */
+                    } else if (mode == ENVELOPE_MODE_HOLD) {
+                        n->env.block_rate = 0; /* Hold at peak */
                     } else {
                         n->env.block_rate = n->env.attack;
                     }
                 }
                 n->env.block_counter--;
 
-                /* Apply rate */
+                /* Apply rate based on mode */
                 int32_t val = n->state & ENVELOPE_STATE_VALUE_MASK;
                 if (v->gate) {
-                    uint32_t mode =
-                        ((uint32_t) n->state) & ENVELOPE_STATE_MODE_BIT;
-                    if (mode) {
+                    if (mode == ENVELOPE_MODE_DECAY) {
+                        /* Decay/Sustain phase */
                         q15_t sus_abs = n->env.sustain < 0 ? -n->env.sustain
                                                            : n->env.sustain;
                         int32_t sus_level = sus_abs << 4;
@@ -746,24 +754,40 @@ q15_t picosynth_process(picosynth_t *s)
                                        15);
                         if (val < sus_level)
                             val = sus_level;
+                    } else if (mode == ENVELOPE_MODE_HOLD) {
+                        /* Hold phase: maintain peak, count down */
+                        val = (int32_t) Q15_MAX << 4; /* Stay at peak */
+                        if (n->env.hold_counter > 0)
+                            n->env.hold_counter--;
+                        if (n->env.hold_counter == 0) {
+                            /* Transition to decay mode */
+                            mode = ENVELOPE_MODE_DECAY;
+                            n->env.block_counter = 0;
+                        }
                     } else {
-                        /* Attack: check for transition to decay */
+                        /* Attack phase: ramp up to peak */
                         val += n->env.block_rate;
                         if (val >= (int32_t) Q15_MAX << 4) {
                             val = (int32_t) Q15_MAX << 4;
-                            mode = ENVELOPE_STATE_MODE_BIT;
+                            /* Check if hold phase is configured */
+                            if (n->env.hold > 0) {
+                                mode = ENVELOPE_MODE_HOLD;
+                                n->env.hold_counter = n->env.hold;
+                            } else {
+                                mode = ENVELOPE_MODE_DECAY;
+                            }
                             /* Force rate recalculation next sample */
                             n->env.block_counter = 0;
                         }
                     }
                     n->state = (int32_t) (((uint32_t) val) | mode);
                 } else {
-                    /* Exponential release */
+                    /* Exponential release (mode cleared) */
                     val = (int32_t) (((int64_t) val * n->env.release_coeff) >>
                                      15);
                     if (val < 16)
                         val = 0;
-                    n->state = val;
+                    n->state = val; /* mode bits clear during release */
                 }
                 break;
             }

tests/example.c

@@ -155,11 +155,14 @@ int main(void)
     /* Fundamental envelope: VERY slow decay for sustained body
      * Real piano fundamental sustains for seconds
      */
-    picosynth_init_env(
-        v0_env, NULL, 10000,          /* attack - smooth onset */
-        60,                           /* decay - VERY slow (sustains long) */
-        (q15_t) (Q15_MAX * 15 / 100), /* sustain (15%) - strong body */
-        40);                          /* release - gentle fade */
+    picosynth_init_env(v0_env, NULL,
+                       &(picosynth_env_params_t) {
+                           .attack = 10000, /* smooth onset */
+                           .hold = 0,       /* none (immediate decay) */
+                           .decay = 60,     /* VERY slow (sustains long) */
+                           .sustain = (q15_t) (Q15_MAX * 15 / 100), /* 15% */
+                           .release = 40, /* gentle fade */
+                       });
 
     /* Pure sine at fundamental frequency */
     picosynth_init_osc(v0_osc, &v0_env->out, picosynth_voice_freq_ptr(v),
@@ -184,21 +187,29 @@ int main(void)
     picosynth_node_t *v1_mix = picosynth_voice_get_node(v, 5);
 
     /* 2nd partial envelope: medium decay (faster than fundamental) */
-    picosynth_init_env(v1_env1, NULL, 8000,         /* attack */
-                       150,                         /* decay - MEDIUM rate */
-                       (q15_t) (Q15_MAX * 8 / 100), /* sustain (8%) */
-                       50);                         /* release */
+    picosynth_init_env(v1_env1, NULL,
+                       &(picosynth_env_params_t) {
+                           .attack = 8000,
+                           .hold = 0,
+                           .decay = 150, /* MEDIUM rate */
+                           .sustain = (q15_t) (Q15_MAX * 8 / 100), /* 8% */
+                           .release = 50,
+                       });
 
     /* 2nd partial: sine at 2*base_freq with inharmonicity stretch */
     picosynth_init_osc(v1_osc1, &v1_env1->out, picosynth_voice_freq_ptr(v),
                        picosynth_wave_sine);
     v1_osc1->osc.detune = &partial2_offset; /* offset = f1, so total = 2*f1 */
 
     /* 3rd partial envelope: slightly faster decay than 2nd */
-    picosynth_init_env(v1_env2, NULL, 7000, /* attack */
-                       300, /* decay - faster than 2nd partial */
-                       (q15_t) (Q15_MAX * 4 / 100), /* sustain (4%) */
-                       40);                         /* release */
+    picosynth_init_env(v1_env2, NULL,
+                       &(picosynth_env_params_t) {
+                           .attack = 7000,
+                           .hold = 0,
+                           .decay = 300, /* faster than 2nd partial */
+                           .sustain = (q15_t) (Q15_MAX * 4 / 100), /* 4% */
+                           .release = 40,
+                       });
 
     /* 3rd partial: sine at 3*base_freq with inharmonicity stretch */
     picosynth_init_osc(v1_osc2, &v1_env2->out, picosynth_voice_freq_ptr(v),
@@ -225,11 +236,14 @@ int main(void)
     /* Upper partials envelope: FAST decay, LOW level
      * Subtle contribution that fades quickly
      */
-    picosynth_init_env(
-        v2_env, NULL, 5000,          /* attack - soft onset */
-        800,                         /* decay - fast fade */
-        (q15_t) (Q15_MAX * 1 / 100), /* sustain (1%) - nearly silent */
-        20);                         /* release - quick */
+    picosynth_init_env(v2_env, NULL,
+                       &(picosynth_env_params_t) {
+                           .attack = 5000, /* soft onset */
+                           .hold = 0,
+                           .decay = 800, /* fast fade */
+                           .sustain = (q15_t) (Q15_MAX * 1 / 100), /* 1% */
+                           .release = 20,                          /* quick */
+                       });
 
     /* Sine for clean sound - no harsh harmonics */
     picosynth_init_osc(v2_osc, &v2_env->out, picosynth_voice_freq_ptr(v),
@@ -253,10 +267,14 @@ int main(void)
     /* Hammer noise: very subtle, almost imperceptible
      * Just adds slight "thump" texture, not harsh attack
      */
-    picosynth_init_env(v3_env, NULL, 8000, /* attack - soft */
-                       6000,               /* decay - very fast */
-                       0,                  /* sustain - none */
-                       50);                /* release */
+    picosynth_init_env(v3_env, NULL,
+                       &(picosynth_env_params_t) {
+                           .attack = 8000, /* soft */
+                           .hold = 0,
+                           .decay = 6000, /* very fast */
+                           .sustain = 0,  /* none */
+                           .release = 50,
+                       });
 
     /* White noise source */
     picosynth_init_osc(v3_noise, &v3_env->out, picosynth_voice_freq_ptr(v),