Lb302 cleanup by rubiefawn · Pull Request #8132 · LMMS/lmms

rubiefawn · 2025-11-13T09:15:14Z

Makes the following changes to the Lb302 plugin for increased maintainability and readability:

The above list is not comprehensive and is destined to expand.

rubiefawn · 2025-12-08T08:58:28Z

Figured I'd run some un-rigorous perf stat benchmarks of release builds as a sanity check to make sure I wasn't destroying performance by changing the mutex for a bunch of atomic operations and busy-wait loops.

Updated for commit 03cca1b

Before, with mutex/dynamically allocated vector:

Performance counter stats for './lmms render /home/fawn/Documents/lmms/projects/torture.mmpz -a -f flac':

   103,084,983,781      task-clock                       #    2.979 CPUs utilized             
           695,800      context-switches                 #    6.750 K/sec                     
               388      cpu-migrations                   #    3.764 /sec                      
             6,173      page-faults                      #   59.883 /sec                      
   267,717,430,944      instructions                     #    1.17  insn per cycle            
   229,289,575,609      cycles                           #    2.224 GHz                       
    61,653,273,269      branches                         #  598.082 M/sec                     
       496,591,833      branch-misses                    #    0.81% of all branches           

      34.602878156 seconds time elapsed

      93.334079000 seconds user
       9.660192000 seconds sys

After, with atomic ringbuffer/statically allocated array:

Performance counter stats for './lmms render /home/fawn/Documents/lmms/projects/torture.mmpz -a -f flac':

   104,963,456,247      task-clock                       #    3.066 CPUs utilized             
           708,920      context-switches                 #    6.754 K/sec                     
                67      cpu-migrations                   #    0.638 /sec                      
             4,237      page-faults                      #   40.366 /sec                      
   267,503,252,259      instructions                     #    1.15  insn per cycle            
   233,160,958,544      cycles                           #    2.221 GHz                       
    61,493,266,240      branches                         #  585.854 M/sec                     
       511,104,882      branch-misses                    #    0.83% of all branches           

      34.239441489 seconds time elapsed

      95.604870000 seconds user
       9.199340000 seconds sys

Project file used for said benchmarks: torture.mmp.xml

rubiefawn · 2025-12-08T22:40:44Z

CACHELINE and busy_wait_hint probably don't belong inside Lb302. Where is the best place for me to move these to?

messmerd · 2026-01-08T06:03:56Z

CACHELINE and busy_wait_hint probably don't belong inside Lb302. Where is the best place for me to move these to?

Maybe add them to a new LmmsPolyfill.h header. And rename CACHELINE to lmms::hardware_destructive_interference_size to match the name the standard library uses.

Makes the following changes to the Lb302 plugin: - Use non-static data member initializers where possible - Find suitable homes for several loose constants - Use std::numbers::pi instead of M_PI - Use std::array and std::unique_ptr instead of manual memory management - Change some plain-old-data classes with only public members to structs - Change some ints to more descriptive types such as f_cnt_t, fpp_t, etc - Attempt to conform any lines I touched to current code conventions

This should be the last of the functions from `<cmath>`

Begone, SIGNAL() SLOT() macro magic!!

...and move them to be inside the classes that own them

- Remove Lb302Note as it is just function parameters with extra steps - Clean up Lb302Synth::initNote() - Clean up Lb302Synth::process() - Move the GET_INC() helper to an anon namespace, rename to phaseInc() - Make computeDecayFactor() a lambda next to its only callsite

To avoid implicit casts, it's all supposed to be floats

Also don't bother checking for __ARM_ACLE

whyyyyyyy

Also don't drop notes when the queue is full, busy-wait a limited number of iterations to see if space gets freed up

- Rename CACHELINE to lmms::hardware_destructive_interference_size - Move lmms::hardware_destructive_interference_size and busy_wait_hint() to Hardware.h

include/Hardware.h

messmerd · 2026-01-29T19:27:32Z

include/Hardware.h

+#endif
+
+
+#if defined(__x86_64__) || defined(_M_AMD64) || defined(__i386__) || defined(_M_IX86)


I would use the host architecture macros from lmmsconfig.h instead

Mostly resolved as of 6fccf95 (MSVC is a pain in the ass as usual)

include/Hardware.h

plugins/Lb302/Lb302.cpp

messmerd · 2026-01-30T02:29:29Z

plugins/Lb302/Lb302.h

+	float vcf_c0 = 0.f; // c0=e1 on retrigger; c0*=ed every sample; cutoff=e0+c0
+	float vcf_e0 = 0.f; // e0 and e1 for interpolation
+	float vcf_e1 = 0.f;
+	float vcf_rescoeff; //!< Resonance coefficient [0.30, 9.54]


These could be renamed to follow the naming conventions for data members

For stuff like this where it's all vcf_foo, vcf_bar, with coefficient names that are supposed to be lowercase, I'm considering sticking them all in a POD struct like so:

struct { float c0 = 0.f; // c0 = e[1] on retrigger; c0 *='ed every sample; cutoff = e[0] + c0 std::array<float, 2> e = { 0.f, 0.f }; // e[0] and e[1] for interpolation float resCoeff; //!< Resonance coefficient [0.30, 9.54] } m_vcf;

plugins/Lb302/Lb302.h

messmerd · 2026-01-30T02:33:23Z

plugins/Lb302/Lb302.h

-	float vcf_d1,           //   d1 and d2 are added back into the sample with
-	      vcf_d2;           //   vcf_a and b as coefficients. IIR2 resonance
-	                        //   loop.
+protected:


The data members of this class could also be renamed to follow the conventions

messmerd · 2026-01-30T02:34:44Z

plugins/Lb302/Lb302.h

+	float kfcn;
+	float kp;
+	float kp1;
+	float kp1h;
+	float kres;
+	float ay1 = 0.f;
+	float ay2 = 0.f;
+	float aout = 0.f;
+	float lastin = 0.f;
+	float value;


messmerd · 2026-01-30T02:38:06Z

plugins/Lb302/Lb302.h

+	static constexpr float DIST_RATIO = 4.f;
+	static constexpr fpp_t ENVINC = 64; //!< Envelope Recalculation period
+	static constexpr float vca_attack = 1.f - 0.96406088f; //!< Amp attack
+	static constexpr float vca_a0 = 0.5f; //!< Initial amplifier coefficient


Suggested change

static constexpr float DIST_RATIO = 4.f;

static constexpr fpp_t ENVINC = 64; //!< Envelope Recalculation period

static constexpr float vca_attack = 1.f - 0.96406088f; //!< Amp attack

static constexpr float vca_a0 = 0.5f; //!< Initial amplifier coefficient

static constexpr float s_distRatio = 4.f;

static constexpr fpp_t s_envInc = 64; //!< Envelope Recalculation period

static constexpr float s_vcaAttack = 1.f - 0.96406088f; //!< Amp attack

static constexpr float s_vcaA0 = 0.5f; //!< Initial amplifier coefficient

Following the conventions for private static data members.

The other data members in this class could also be updated.

Fixed static members in ac820f4, the rest still todo.

include/Hardware.h

- #include <new> *before* testing macro defined within it (lol) - Make #ifdef chain for hardware_destructive_interference_size more readable - Use LMMS_HOST_* macros from lmmsconfig.h where possible - Move #includes for busy_wait_hint() to top of file - Make busy_wait_hint() a function rather than a macro - Minor formatting changes

15 is the only allowed value, so that is used regardless of whatever is passed in

rubiefawn · 2026-02-04T01:14:59Z

Ugh, I just noticed a bug I must have introduced in this PR with slide behavior. Notes continue to slide even after the toggle is disabled, but the slide-from note is whatever the last note was before the toggle was disabled.

Edit: Bug introduced in 1b563ed, is fixed by 0fc7d88. Hooray for git bisect

- Restored a line that resets the slide-from note when a slide is done, the accidental removal of which was the cause of a weird bug - Actually remove initSlide() and initNote() which were supposed to be fully inlined

- Remove redundant `db24Toggled()` calls on init and preset load - Move filter envelope decay stuff that does not require `recalcFilter()` from `filterChanged()` into new slot `decayChanged()`

rubiefawn self-assigned this Nov 13, 2025

rubiefawn added the enhancement label Nov 13, 2025

rubiefawn added performance needs style review A style review is currently required for this PR needs code review A functional code review is currently required for this PR needs testing This pull request needs more testing labels Dec 8, 2025

rubiefawn marked this pull request as ready for review December 8, 2025 22:38

rubiefawn added 20 commits January 27, 2026 23:09

Combine and clean up extern "C" blocks

dcf4bf8

Prefix tanh() with std::

67582b5

This should be the last of the functions from `<cmath>`

Change floats that are audio to sample_t

b65c779

Modernize Qt connect() invocations

7315fd9

Begone, SIGNAL() SLOT() macro magic!!

Change remaining macro constants to constexpr vars

b38b6c8

...and move them to be inside the classes that own them

Change double literals to float literals

1fa9fa9

To avoid implicit casts, it's all supposed to be floats

Start to rearrange the layout of Lb302Synth

c89e885

Code conventions tomfoolery

2d16014

Make some more things constexpr

09c6ff4

Minor coding conventions conformity

cce4ddd

Code conventions applied to Lb302SynthView

4ae13ac

Replace mutex with realtime-safe mpsc channel

e2bc4ef

Add missing headers

2948743

Also don't bother checking for __ARM_ACLE

Fallback hardware_destructive_interference_size

cf8a14c

GCC is missing some ARM intrinsics, attempt asm

e3372bd

whyyyyyyy

Remove mutex header include

b7020c8

Process new notes last (previous behavior)

03cca1b

Also don't drop notes when the queue is full, busy-wait a limited number of iterations to see if space gets freed up

Add documentation for all the ringbuffer stuff

6450822

rubiefawn added 2 commits January 27, 2026 23:09

Move hardware-specific stuff to Hardware.h

50e4d13

- Rename CACHELINE to lmms::hardware_destructive_interference_size - Move lmms::hardware_destructive_interference_size and busy_wait_hint() to Hardware.h

Inline some functions only used in one place

1b563ed

rubiefawn force-pushed the refac/lb302 branch from e21515b to 1b563ed Compare January 28, 2026 06:10

messmerd reviewed Jan 30, 2026

View reviewed changes

rubiefawn added 4 commits February 3, 2026 13:56

Ignore parameter for __isb() polyfill

3f878e7

15 is the only allowed value, so that is used regardless of whatever is passed in

Change static member names to fit conventions

ac820f4

Remove weird float pass-by-reference

7e031da

rubiefawn force-pushed the refac/lb302 branch from d3615aa to 7e031da Compare February 3, 2026 21:35

rubiefawn added 2 commits February 3, 2026 14:48

Oops, missed one static member rename

c349ec3

Move phaseInc() helper into lmms namespace

8498015

Fix bug introduced in 1b563ed

0fc7d88

- Restored a line that resets the slide-from note when a slide is done, the accidental removal of which was the cause of a weird bug - Actually remove initSlide() and initNote() which were supposed to be fully inlined

rubiefawn force-pushed the refac/lb302 branch from 788dda5 to 0fc7d88 Compare February 5, 2026 21:10

Resolve two TODOs

9a01a26

- Remove redundant `db24Toggled()` calls on init and preset load - Move filter envelope decay stuff that does not require `recalcFilter()` from `filterChanged()` into new slot `decayChanged()`

		#endif


		#if defined(__x86_64__) \|\| defined(_M_AMD64) \|\| defined(__i386__) \|\| defined(_M_IX86)

Uh oh!

Conversation

rubiefawn commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rubiefawn commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rubiefawn commented Dec 8, 2025

Uh oh!

messmerd commented Jan 8, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rubiefawn commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rubiefawn commented Nov 13, 2025 •

edited

Loading

rubiefawn commented Dec 8, 2025 •

edited

Loading

rubiefawn commented Feb 4, 2026 •

edited

Loading