wled-screen-controller-mixed/CODEBASE_REVIEW.md

Codebase Review — 2026-02-26

Findings from full codebase review. Items ordered by priority within each category.

Stability (Critical)

• Fatal loop exception leaks resources — Added outer try/except/finally with self._running = False to all 10 processing loop methods across live_stream.py, color_strip_stream.py, effect_stream.py, audio_stream.py, composite_stream.py, mapped_stream.py. Also added per-iteration try/except where missing.
• _is_running flag cleanup — Fixed via finally: self._running = False in all loop methods. (Race condition via threading.Event deferred — current pattern sufficient with the finally block.)
• ColorStripStreamManager thread safety — FALSE POSITIVE: All access is from the async event loop; methods are synchronous with no await points, so no preemption is possible.
• Audio stream.stop() called under lock — Moved stream.stop() outside lock scope in both release() and release_all() in audio_capture.py.
• WS accept-before-validate — FALSE POSITIVE: All WS endpoints validate auth and resolve configs BEFORE calling websocket.accept().
• Capture error no-backoff — Added consecutive error counter with exponential backoff (min(1.0, 0.1 * (errors - 5))) in ScreenCaptureLiveStream._capture_loop().
• WGC session close not detected — Deferred (Windows-specific edge case, low priority).
• LiveStreamManager.acquire() not thread-safe — FALSE POSITIVE: Same as ColorStripStreamManager — all access from async event loop, no await in methods.

Performance (High Impact)

• Per-pixel Python loop in send_pixels() — Replaced per-pixel Python loop with np.array().tobytes() in ddp_client.py. Hot path already uses send_pixels_numpy().
• WGC 6MB frame allocation per callback — Deferred (Windows-specific, requires WGC API changes).
• Gradient rendering O(LEDs×Stops) Python loop — Vectorized with NumPy: np.searchsorted for stop lookup + vectorized interpolation in _compute_gradient_colors().
• PixelateFilter nested Python loop — Replaced with cv2.resize down (INTER_AREA) + up (INTER_NEAREST) — pure C++ backend.
• DownscalerFilter double allocation — FALSE POSITIVE: Already uses single cv2.resize() call (vectorized C++).
• SaturationFilter ~25MB temp arrays — FALSE POSITIVE: Already uses pre-allocated scratch buffer and vectorized in-place numpy.
• FrameInterpolationFilter copies full image — FALSE POSITIVE: Already uses vectorized numpy integer blending with image pool.
• datetime.utcnow() per frame — LOW IMPACT: ~1-2μs per call, negligible at 60fps. Deprecation tracked under Backend Quality.
• Unbounded diagnostic lists — FALSE POSITIVE: Lists are cleared every 5 seconds (~300 entries max at 60fps). Trivial memory.

Frontend Quality

• lockBody()/unlockBody() not re-entrant — Added _lockCount reference counter and _savedScrollY in ui.js. First lock saves scroll, last unlock restores.
• XSS via unescaped engine config keys — FALSE POSITIVE: Both capture template and audio template card renderers already use escapeHtml() on keys and values.
• LED preview WS onclose not nulled — Added ws.onclose = null before ws.close() in disconnectLedPreviewWS() in targets.js.
• fetchWithAuth retry adds duplicate listeners — Added { once: true } to abort signal listener in api.js.
• Audio requestAnimationFrame loop continues after WS close — FALSE POSITIVE: Loop already checks testAudioModal.isOpen before scheduling next frame, and _cleanupTest() cancels the animation frame.

Backend Quality

• No thread-safety in JsonStore — Deferred (low risk — all stores are accessed from async event loop).
• Auth token prefix logged — Removed token prefix from log message in auth.py. Now logs only "Invalid API key attempt".
• Duplicate capture/test code — Deferred (code duplication, not a bug — refactoring would reduce LOC but doesn't fix a defect).
• Update methods allow duplicate names — Added name uniqueness checks to update_template in template_store.py, postprocessing_template_store.py, audio_template_store.py, pattern_template_store.py, and update_profile in profile_store.py. Also added missing check to create_profile.
• Routes access manager._private attrs — Deferred (stylistic, not a bug — would require adding public accessor methods).
• Non-atomic file writes — Created utils/file_ops.py with atomic_write_json() helper (tempfile + os.replace). Updated all 10 store files.
• 444 f-string logger calls — Deferred (performance impact negligible — Python evaluates f-strings very fast; lazy %s formatting only matters at very high call rates).
• get_source() silent bug — Fixed: color_strip_sources.py:_resolve_display_index() called picture_source_store.get_source() which doesn't exist (should be get_stream()). Was silently returning 0 for display index.
• get_config() race — Deferred (low risk — config changes are infrequent user-initiated operations).
• datetime.utcnow() deprecated — Deferred (functional, deprecation warning only appears in Python 3.12+).
• Inconsistent DELETE status codes — Changed audio_sources.py and value_sources.py DELETE endpoints from 200 to 204 (matching all other DELETE endpoints).

Architecture (Observations, no action needed)

Strengths: Clean layered design, plugin registries, reference-counted stream sharing, consistent API patterns.

Weaknesses: No backpressure (slow consumers buffer frames), thread count grows linearly, config global singleton, reference counting races.