moved audio input/output to it's own thread, wallpapers with music elements work now properly

src/WallpaperEngine/Application/WallpaperApplication.cpp

@@ -33,8 +33,10 @@
 
 #include <algorithm>
 #include <cctype>
+#include <chrono>
 #include <climits>
 #include <csignal>
+#include <ctime>
 #include <fstream>
 #include <numeric>
 #include <string_view>

src/WallpaperEngine/Audio/Drivers/Recorders/PlaybackRecorder.h

@@ -7,6 +7,10 @@ public:
 
     virtual void update ();
 
+    /** Guards audio16/32/64 for recorders that fill them from a background capture thread; no-op by default */
+    virtual void lock () const { }
+    virtual void unlock () const { }
+
     float audio16[16] = { 0 };
     float audio32[32] = { 0 };
     float audio64[64] = { 0 };

src/WallpaperEngine/Audio/Drivers/Recorders/PulseAudioPlaybackRecorder.cpp

@@ -1,25 +1,36 @@
 #include "PulseAudioPlaybackRecorder.h"
 #include "WallpaperEngine/Logging/Log.h"
+#include <chrono>
 #include <cmath>
 #include <cstring>
-#include <glm/common.hpp>
+#include <ctime>
+#include <iomanip>
+#include <sstream>
 
-float movetowards (float current, float target, float maxDelta) {
-    if (abs (target - current) <= maxDelta) {
-	return target;
-    }
-
-    return current + glm::sign (target - current) * maxDelta;
+namespace WallpaperEngine::Audio::Drivers::Recorders {
+namespace {
+// Timestamp helper backing the debug-only capture markers below - useful for tracking down
+// audio-to-visual delay regressions in the future.
+std::string wallClockTimestamp () {
+    const auto now = std::chrono::system_clock::now ();
+    const auto ms = std::chrono::duration_cast<std::chrono::milliseconds> (now.time_since_epoch ()) % 1000;
+    const std::time_t t = std::chrono::system_clock::to_time_t (now);
+    std::tm tmBuf {};
+    localtime_r (&t, &tmBuf);
+
+    std::ostringstream oss;
+    oss << std::put_time (&tmBuf, "%H:%M:%S") << '.' << std::setfill ('0') << std::setw (3) << ms.count ();
+    return oss.str ();
 }
+} // namespace
 
-namespace WallpaperEngine::Audio::Drivers::Recorders {
 void pa_stream_notify_cb (pa_stream* stream, void* /*userdata*/) {
     switch (pa_stream_get_state (stream)) {
 	case PA_STREAM_FAILED:
 	    sLog.error ("Cannot open stream for capture. Audio processing is disabled");
 	    break;
 	case PA_STREAM_READY:
-	    sLog.debug ("Audio processing: capture stream ready");
+	    sLog.debug ("[", wallClockTimestamp (), "] Audio processing: capture stream ready");
 	    break;
 	default:
 	    break;
@@ -124,8 +135,8 @@ void pa_server_info_cb (pa_context* ctx, const pa_server_info* info, void* userd
 
     // 10 = latency msecs, 750 = max msecs to store
     size_t bytesPerSec = pa_bytes_per_second (&spec);
-    attr.fragsize = bytesPerSec * 10 / 100;
-    attr.maxlength = attr.fragsize + bytesPerSec * 750 / 100;
+    attr.fragsize = bytesPerSec * 10 / 1000;
+    attr.maxlength = attr.fragsize + bytesPerSec * 750 / 1000;
 
     sLog.debug ("Audio processing: capturing from monitor source '", monitor_name, "' (default sink)");
 
@@ -182,6 +193,7 @@ PulseAudioPlaybackRecorder::PulseAudioPlaybackRecorder () :
 	  .audioBuffer = new uint8_t[WAVE_BUFFER_SIZE],
 	  .audioBufferTmp = new uint8_t[WAVE_BUFFER_SIZE] }
     ) {
+    this->m_dataMutex = SDL_CreateMutex ();
     this->m_mainloop = pa_mainloop_new ();
     this->m_mainloopApi = pa_mainloop_get_api (this->m_mainloop);
     this->m_context = pa_context_new (this->m_mainloopApi, "wallpaperengine-audioprocessing");
@@ -197,9 +209,27 @@ PulseAudioPlaybackRecorder::PulseAudioPlaybackRecorder () :
     while (pa_context_get_state (this->m_context) != PA_CONTEXT_READY) {
 	pa_mainloop_iterate (this->m_mainloop, 1, nullptr);
     }
+
+    // Capture used to be pumped from the render loop (pa_mainloop_iterate() once per frame via
+    // update()), which meant a slow frame - a GPU/compositor stall, a heavy shader pass, anything
+    // that blocks the render thread - stalled audio capture along with it. PulseAudio/PipeWire then
+    // has to force-drop the backlog once its buffer overflows, so the wallpaper "catches up" all at
+    // once instead of reacting smoothly. Capture now runs on its own thread so it keeps draining
+    // regardless of what rendering is doing.
+    this->m_captureThread
+	= SDL_CreateThread (&PulseAudioPlaybackRecorder::captureThreadEntry, "lwe-audiocapture", this);
 }
 
 PulseAudioPlaybackRecorder::~PulseAudioPlaybackRecorder () {
+    this->m_running.store (false, std::memory_order_relaxed);
+    if (this->m_mainloop) {
+	// unblocks a pa_mainloop_iterate() the capture thread may be blocked in
+	pa_mainloop_wakeup (this->m_mainloop);
+    }
+    if (this->m_captureThread) {
+	SDL_WaitThread (this->m_captureThread, nullptr);
+    }
+
     if (m_captureData.captureStream) {
 	pa_stream_unref (m_captureData.captureStream);
     }
@@ -211,30 +241,41 @@ PulseAudioPlaybackRecorder::~PulseAudioPlaybackRecorder () {
     pa_context_disconnect (this->m_context);
     pa_context_unref (this->m_context);
     pa_mainloop_free (this->m_mainloop);
+
+    if (this->m_dataMutex) {
+	SDL_DestroyMutex (this->m_dataMutex);
+    }
 }
 
 void PulseAudioPlaybackRecorder::update () {
-    pa_mainloop_iterate (this->m_mainloop, 0, nullptr);
+    // capture now runs on its own thread (see the constructor and captureLoop()) - nothing to do
+    // here anymore, kept as a no-op override since AudioDriver still calls this once per frame.
+}
 
-    // interpolate current values to the destination
-    for (int i = 0; i < 64; i++) {
-	this->audio64[i] = movetowards (this->audio64[i], this->m_FFTdestination64[i], 0.3f);
-	if (i >= 32) {
-	    continue;
-	}
-	this->audio32[i] = movetowards (this->audio32[i], this->m_FFTdestination32[i], 0.3f);
-	if (i >= 16) {
+void PulseAudioPlaybackRecorder::lock () const { SDL_LockMutex (this->m_dataMutex); }
+void PulseAudioPlaybackRecorder::unlock () const { SDL_UnlockMutex (this->m_dataMutex); }
+
+int PulseAudioPlaybackRecorder::captureThreadEntry (void* userdata) {
+    static_cast<PulseAudioPlaybackRecorder*> (userdata)->captureLoop ();
+    return 0;
+}
+
+void PulseAudioPlaybackRecorder::captureLoop () {
+    while (this->m_running.load (std::memory_order_relaxed)) {
+	// blocks until there's data, a state change, or pa_mainloop_wakeup() from the destructor -
+	// this thread has nothing else to do, so there's no reason to poll instead of blocking
+	pa_mainloop_iterate (this->m_mainloop, 1, nullptr);
+
+	if (!this->m_captureData.fullFrameReady) {
 	    continue;
 	}
-	this->audio16[i] = movetowards (this->audio16[i], this->m_FFTdestination16[i], 0.3f);
-    }
 
-    if (!this->m_captureData.fullFrameReady) {
-	return;
+	this->m_captureData.fullFrameReady = false;
+	this->processFrame ();
     }
+}
 
-    this->m_captureData.fullFrameReady = false;
-
+void PulseAudioPlaybackRecorder::processFrame () {
     // convert audio data to deltas so the fft library can properly handle it
     for (int i = 0; i < WAVE_BUFFER_SIZE; i++) {
 	this->m_audioFFTbuffer[i] = (this->m_captureData.audioBuffer[i] - 128) / 128.0f;
@@ -243,6 +284,12 @@ void PulseAudioPlaybackRecorder::update () {
     // perform full fft pass
     kiss_fftr (this->m_captureData.kisscfg, this->m_audioFFTbuffer, this->m_FFTinfo);
 
+    // computed into locals first so the lock only needs to be held for the final copy, not the
+    // whole FFT pass
+    float bands64[64];
+    float bands32[32];
+    float bands16[16];
+
     // now reduce to the different bands
     // use just one for loop to produce all 3
     for (int band = 0; band < 64; band++) {
@@ -260,25 +307,46 @@ void PulseAudioPlaybackRecorder::update () {
 	    f1 = 0.35f * log10 (f2) + kLoudnessOffset;
 	}
 
-	this->m_FFTdestination64[band] = fmax (
+	// written directly (no smoothing here) - the wallpaper's own script already smooths this
+	// against real elapsed time via its "smoothing" scriptproperty (see engine.frametime usage
+	// in the audio-response script snippet); an extra fixed-step smoothing pass here would just
+	// double up on that.
+	bands64[band] = fmax (
 	    0.0f, fmin (1.0f, f1 * static_cast<float> (2.0f - pow (M_E, (1.0f - band / 63.0f) * 1.0f - 0.5f)))
 	);
-	this->m_FFTdestination32[band >> 1] = fmax (
+	bands32[band >> 1] = fmax (
 	    0.0f, fmin (1.0f, f1 * static_cast<float> (2.0f - pow (M_E, (1.0f - band / 31.0f) * 1.0f - 0.5f)))
 	);
-	this->m_FFTdestination16[band >> 2] = fmax (
+	bands16[band >> 2] = fmax (
 	    0.0f, fmin (1.0f, f1 * static_cast<float> (2.0f - pow (M_E, (1.0f - band / 15.0f) * 1.0f - 0.5f)))
 	);
     }
 
+    this->lock ();
+    memcpy (this->audio64, bands64, sizeof (bands64));
+    memcpy (this->audio32, bands32, sizeof (bands32));
+    memcpy (this->audio16, bands16, sizeof (bands16));
+    this->unlock ();
+
     static int diagnosticCounter = 0;
     if (++diagnosticCounter >= 100) {
 	diagnosticCounter = 0;
-	sLog.debug (
-	    "Audio processing: audio16[0..3] = ", this->audio16[0], ", ", this->audio16[1], ", ", this->audio16[2],
-	    ", ", this->audio16[3]
-	);
+	sLog.debug ("Audio processing: audio16[0..3] = ", bands16[0], ", ", bands16[1], ", ", bands16[2], ", ", bands16[3]);
+    }
+
+    // Edge-triggered marker for a loud transient (e.g. a clap) reaching the capture layer,
+    // timestamped so it can be correlated against when the transient actually happened and when
+    // the visual pulse reacts to it - isolates whether a future delay regression is in capture or
+    // downstream of it.
+    static bool wasLoud = false;
+    float peak = 0.0f;
+    for (float band : bands16) {
+	peak = fmax (peak, band);
+    }
+    if (peak > 0.5f && !wasLoud) {
+	sLog.debug ("[", wallClockTimestamp (), "] Audio processing: TRANSIENT detected, peak=", peak);
     }
+    wasLoud = peak > 0.5f;
 }
 
 } // namespace WallpaperEngine::Audio::Drivers::Recorders

src/WallpaperEngine/Audio/Drivers/Recorders/PulseAudioPlaybackRecorder.h

@@ -2,6 +2,8 @@
 
 #include "PlaybackRecorder.h"
 #include "kiss_fftr.h"
+#include <SDL.h>
+#include <atomic>
 #include <pulse/pulseaudio.h>
 
 #define WAVE_BUFFER_SIZE 1024
@@ -27,8 +29,14 @@ public:
     ~PulseAudioPlaybackRecorder () override;
 
     void update () override;
+    void lock () const override;
+    void unlock () const override;
 
 private:
+    static int captureThreadEntry (void* userdata);
+    void captureLoop ();
+    void processFrame ();
+
     pa_mainloop* m_mainloop;
     pa_mainloop_api* m_mainloopApi;
     pa_context* m_context;
@@ -36,8 +44,11 @@ private:
 
     float m_audioFFTbuffer[WAVE_BUFFER_SIZE] = { 0.0f };
     kiss_fft_cpx m_FFTinfo[WAVE_BUFFER_SIZE / 2 + 1] = { { .r = 0.0f, .i = 0.0f } };
-    float m_FFTdestination64[64] = { 0 };
-    float m_FFTdestination32[32] = { 0 };
-    float m_FFTdestination16[16] = { 0 };
+
+    // Capture runs on its own thread (see the constructor) so it keeps draining PulseAudio
+    // regardless of how long a render frame takes - see processFrame()'s comment for why.
+    SDL_Thread* m_captureThread = nullptr;
+    mutable SDL_mutex* m_dataMutex = nullptr;
+    std::atomic<bool> m_running { true };
 };
 } // namespace WallpaperEngine::Audio::Drivers::Recorders

src/WallpaperEngine/Scripting/Adapters/VectorAdapter.cpp

@@ -936,7 +936,9 @@ VectorAdapter<components>::VectorAdapter (ScriptEngine& engine) :
 template <int components> VectorAdapter<components>::~VectorAdapter () {
     vectorAdapterInstances<components>.erase (this->m_instanceId);
 
-    JS_FreeValue (this->m_engine.getContext (), m_prototype);
+    // Runs after ScriptEngine has already freed the JS runtime/context (see ScriptEngine's
+    // destructor) - m_prototype and everything else tied to that context is already gone, so
+    // there's nothing left to explicitly release here.
 }
 
 template <int components> JSValue VectorAdapter<components>::instantiate (ScriptableObject& object) {

src/WallpaperEngine/Scripting/EngineObject.cpp

@@ -140,6 +140,10 @@ JSValue audio_buffer_get_values (
 	data = recorder.audio64;
     }
 
+    // audio16/32/64 are written from the recorder's own capture thread (see
+    // PulseAudioPlaybackRecorder), so reading them here (the script thread) needs the same lock.
+    recorder.lock ();
+
     static int diagnosticCounter = 0;
     if (++diagnosticCounter >= 500) {
 	diagnosticCounter = 0;
@@ -150,6 +154,8 @@ JSValue audio_buffer_get_values (
 	JS_SetPropertyUint32 (ctx, result, i, JS_NewFloat64 (ctx, data[i]));
     }
 
+    recorder.unlock ();
+
     return result;
 }
 

src/WallpaperEngine/Scripting/ScriptEngine.cpp

@@ -263,11 +263,6 @@ ScriptEngine::~ScriptEngine () {
 
     JS_FreeValue (this->m_context, this->m_globalThis);
 
-    this->m_adapters.vec4.reset ();
-    this->m_adapters.vec3.reset ();
-    this->m_adapters.vec2.reset ();
-    this->m_adapters.object.reset ();
-
     this->m_consoleObject.reset ();
     this->m_engineObject.reset ();
     this->m_inputObject.reset ();
@@ -282,6 +277,16 @@ ScriptEngine::~ScriptEngine () {
     if (this->m_runtime) {
 	JS_FreeRuntime (this->m_runtime);
     }
+
+    // Freeing the runtime above runs a GC pass that finalizes every still-live vector object (a
+    // script's initialScale, anything a script kept a reference to, etc.) - each one calls back
+    // into its owning VectorAdapter::free() to release the DynamicValue backing it. That means the
+    // adapters have to outlive JS_FreeRuntime(), not get torn down before it: resetting them
+    // earlier leaves those finalizers calling into freed memory.
+    this->m_adapters.vec4.reset ();
+    this->m_adapters.vec3.reset ();
+    this->m_adapters.vec2.reset ();
+    this->m_adapters.object.reset ();
 }
 
 /// Helper to check for and log JS exceptions
@@ -720,6 +725,26 @@ void ScriptEngine::tick () {
 		    module.value.getVec3 ().z, ")"
 		);
 	    }
+
+	    // Edge-triggered marker for when a pulse actually lands on the visual side, timestamped so
+	    // it can be correlated against the capture-layer TRANSIENT marker in
+	    // PulseAudioPlaybackRecorder - useful for tracking down audio-to-visual delay regressions.
+	    static std::map<std::string, bool> wasPulsing;
+	    const bool pulsingNow = std::abs (module.value.getVec3 ().x - 1.0f) > 0.03f;
+	    if (pulsingNow && !wasPulsing[key]) {
+		const auto now = std::chrono::system_clock::now ();
+		const auto ms = std::chrono::duration_cast<std::chrono::milliseconds> (now.time_since_epoch ()) % 1000;
+		const std::time_t t = std::chrono::system_clock::to_time_t (now);
+		std::tm tmBuf {};
+		localtime_r (&t, &tmBuf);
+		char buf[16];
+		std::strftime (buf, sizeof (buf), "%H:%M:%S", &tmBuf);
+		sLog.debug (
+		    "[", buf, ".", (ms.count () < 100 ? "0" : ""), (ms.count () < 10 ? "0" : ""), ms.count (),
+		    "] scale script '", key, "': PULSE, vec3.x=", module.value.getVec3 ().x
+		);
+	    }
+	    wasPulsing[key] = pulsingNow;
 	}
 
 	jsToDynamicValue (this->m_context, result, module.value);

src/WallpaperEngine/Testing/Cases/ModuleNamespaceTiming.cpp

@@ -3,6 +3,7 @@
 #include "quickjs.h"
 
 #include <cstring>
+#include <memory>
 #include <string>
 
 // Isolated repro for the queueScript() timing question: if we set some external C++ state
@@ -248,3 +249,56 @@ TEST_CASE ("An exotic get_property handler that only special-cases some names mu
     JS_FreeContext (ctx);
     JS_FreeRuntime (rt);
 }
+
+// Mirrors the VectorAdapter/ScriptEngine teardown bug: a class finalizer calls back into an
+// external C++ object (VectorAdapter::free()) to release state it owns. If that object is
+// destroyed before JS_FreeRuntime() runs its GC pass, the finalizer for any still-live instance
+// (e.g. a script's initialScale) calls into freed memory. The owning object must outlive
+// JS_FreeRuntime(); only that order is exercised here.
+namespace {
+JSClassID g_ownedClassId = 0;
+
+struct Adapter {
+    int freedCount = 0;
+    void free () { freedCount++; }
+};
+
+struct OwnedOpaque {
+    Adapter& adapter;
+};
+
+void ownedFinalizer (JSRuntime* /*rt*/, JSValueConst val) {
+    auto* opaque = static_cast<OwnedOpaque*> (JS_GetOpaque (val, g_ownedClassId));
+    if (opaque != nullptr) {
+	opaque->adapter.free ();
+	delete opaque;
+    }
+}
+} // namespace
+
+TEST_CASE ("An owning adapter freed after JS_FreeRuntime sees its still-live instances finalized "
+	   "safely") {
+    JSRuntime* rt = JS_NewRuntime ();
+    JSContext* ctx = JS_NewContext (rt);
+
+    JS_NewClassID (rt, &g_ownedClassId);
+    JSClassDef classDef = { .class_name = "Owned", .finalizer = ownedFinalizer };
+    JS_NewClass (rt, g_ownedClassId, &classDef);
+
+    auto adapter = std::make_unique<Adapter> ();
+
+    JSValue instance = JS_NewObjectClass (ctx, g_ownedClassId);
+    JS_SetOpaque (instance, new OwnedOpaque { .adapter = *adapter });
+
+    // keep the instance reachable (mirrors a script holding onto a live Vec3) so it's still
+    // around, not yet GC'd, when the runtime gets torn down below
+    JSValue globalObj = JS_GetGlobalObject (ctx);
+    JS_SetPropertyStr (ctx, globalObj, "kept", instance);
+    JS_FreeValue (ctx, globalObj);
+
+    // correct order: adapter is still alive here, so the finalizer's call back into it is safe
+    JS_FreeContext (ctx);
+    JS_FreeRuntime (rt);
+
+    REQUIRE (adapter->freedCount == 1);
+}