devices/liquidsfz/liquidsfz.cc: track instrument changes while loading

[swesterfeld]

See also #44.

I've tested this by adding a sleep (30) inside the liquidsfz loading code. Instrument changes while busy loading another instrument are no longer lost now, and the event queue gets unconditionally processed, although two branches are necessary since in one branch (when the loader is busy) we cannot access synth_.

One issue that I noticed while implementing this is that currently the code uses sem_.post() (which is effectively sem_post()) to wake up the loader thread from the audio thread. I am not sure if this is hard RT safe, and if not, if we already have some kind of alternative available that is hare RT safe.

[tim-janik]

@greptile

[greptile-apps]

Greptile Overview

Greptile Summary

This PR fixes a critical race condition in the LiquidSFZ synthesizer device where instrument changes were being lost if they occurred while the loader thread was busy loading another instrument. The original implementation only processed parameter events (including instrument changes) when the loader was idle, causing updates to be dropped during loading operations.

The fix introduces an instrument_changed_ flag to track deferred instrument changes and restructures the event processing logic to run unconditionally. Now there are two branches: when the loader is idle, it processes events normally and can access the synth_ object; when the loader is busy, it still processes events to track parameter changes but defers actual instrument loading until the loader becomes available. This ensures that no instrument changes are lost while maintaining thread safety by avoiding synth_ access during loading operations.

Important Files Changed

Changed Files

Filename	Score	Overview
devices/liquidsfz/liquidsfz.cc	4/5	Refactored event processing to prevent race conditions during async instrument loading by adding deferred loading logic and unconditional event processing

Confidence score: 4/5

• This PR addresses a real race condition with a well-thought-out solution that maintains thread safety
• Score reflects solid logic but potential concerns about real-time safety of semaphore operations and complexity of dual-branch event processing
• Pay close attention to the event processing branches and ensure the deferred loading logic works correctly under all conditions

Sequence Diagram

sequenceDiagram
    participant User
    participant AudioProcessor as LiquidSFZ AudioProcessor
    participant Loader as LiquidSFZLoader
    participant LoaderThread as Loader Thread
    participant Synth as LiquidSFZ Synth

    User->>AudioProcessor: "Change instrument parameter"
    AudioProcessor->>AudioProcessor: "Set instrument_changed_ = true"
    
    Note over AudioProcessor: "In render() method"
    AudioProcessor->>Loader: "Check idle() state"
    
    alt Loader is idle
        Loader-->>AudioProcessor: "Returns true"
        AudioProcessor->>Loader: "load(instrument_file)"
        Loader->>Loader: "Set want_sfz_ = instrument_file"
        Loader->>Loader: "Set state_ = STATE_LOAD"
        Loader->>LoaderThread: "sem_.post() to wake thread"
        
        LoaderThread->>Synth: "synth_.load(want_sfz_)"
        Synth-->>LoaderThread: "Load result"
        LoaderThread->>LoaderThread: "Set have_sfz_ = want_sfz_"
        LoaderThread->>LoaderThread: "Set state_ = STATE_IDLE"
        
        AudioProcessor->>Synth: "Process MIDI events"
        AudioProcessor->>Synth: "synth_.process(output, n_frames)"
        Synth-->>AudioProcessor: "Audio output"
        
    else Loader is busy
        Loader-->>AudioProcessor: "Returns false"
        Note over AudioProcessor: "Still track parameter changes"
        AudioProcessor->>AudioProcessor: "Process PARAM_VALUE events only"
        AudioProcessor->>AudioProcessor: "Output silence (floatfill 0.f)"
    end

Loading

_{1 file reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}