allows seamless colortrans switching

[henkwiedig]

Fix colortrans switching during runtime.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Enable runtime color correction toggling without reinitializing GL context

🐞 Bug fix ✨ Enhancement 🕐 10-20 Minutes

AI Description

• Make color correction gain/offset updates safe and visible across threads.
• Re-read correction parameters every frame to reflect runtime colortrans changes.
• Avoid stale parameters baked into the first GL initialization.

Diagram

graph TD
  A["UI/Caller thread"] --> B["FrameProcessor::set_color_correction()"] --> C["Atomic params + enable flag"] --> D["Processor thread: process_loop()"] --> E["FrameColorCorrect (GL)"] --> F["Corrected frame output"]
  D --> G["OSD snapshot"] --> E

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Command queue to processor thread

• ➕ Avoids atomics and memory-order reasoning by serializing updates on the processor thread
• ➕ Naturally batches multiple parameter changes
• ➖ Requires additional plumbing (queue, wakeups) and careful lifecycle handling
• ➖ More latency/complexity than necessary for two scalar params

2. Mutex-protected shared params

• ➕ Straightforward correctness model
• ➕ Easier to extend to more parameters
• ➖ Adds lock contention on the per-frame hot path unless carefully structured
• ➖ Still needs ordering discipline for enable/disable semantics

3. Reinitialize GL context on each toggle

• ➕ Simplifies state management inside the GL stage
• ➖ Expensive and disruptive; can cause stalls and visible glitches
• ➖ Does not scale if users tweak parameters frequently

Recommendation: Current approach (atomics + per-frame parameter refresh) is the best trade-off here: it keeps the per-frame cost minimal, avoids expensive GL reinitialization, and ensures the processor thread never observes an enable without the matching parameters (release-store on enable + acquire-load on read). The main review focus should be verifying memory-ordering and ensuring set_params() is only called from the processor thread as documented.

Files changed (3) +27 / -9

Enhancement (1) +5 / -0

frame_colorcorrect.hAdd runtime parameter update API for color correction +5/-0

Add runtime parameter update API for color correction

• Introduces FrameColorCorrect::set_params(gain, offset) to update uniforms for an already-initialized GL context. Documents that the change takes effect on the next frame and must be called from the processor thread.

src/frame_colorcorrect.h

Bug fix (2) +22 / -9

frame_processor.cppPublish color correction params atomically and apply them every frame +20/-8

Publish color correction params atomically and apply them every frame

• Stores gain/offset in atomics and enables correction using release semantics to prevent reordering. In the processor loop, reads enable with acquire semantics, derives default params when disabled, and calls FrameColorCorrect::set_params() each frame so runtime toggles are reflected without reinit.

src/frame_processor.cpp

frame_processor.hSwitch color correction parameters to atomics +2/-1

Switch color correction parameters to atomics

• Replaces plain float members with std::atomic<float> for cross-thread safe publication of gain and offset.

src/frame_processor.h

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Non-atomic drm_fd_ access ✓ Resolved 🐞 Bug ☼ Reliability

Description

FrameProcessor::set_color_correction() writes drm_fd_ (plain int) from the UI/control thread
while process_loop() reads drm_fd_ on the processor thread, which is a C++ data race (undefined
behavior). This can manifest as sporadic wrong need_gl decisions or EGL/GBM init using a
stale/invalid fd during runtime colortrans switching.

Code

src/frame_processor.cpp[R86-89]

+    // Release-store so the processor thread can't observe the enable before
+    // the gain/offset it belongs to.
+    color_correct_.store(true, std::memory_order_release);
+    // Actual EGL/GL init happens lazily on the processor thread (first frame);

Evidence

The code explicitly documents that color correction can be toggled/updated from the UI thread, and
main.cpp calls the setter from a live-control callback. In that setter, drm_fd_ is written without
atomic/mutex protection, while in process_loop() the processor thread reads drm_fd_ to
decide/init the GL path, creating concurrent read/write on a non-atomic variable (UB).

src/frame_processor.h[67-75]
src/frame_processor.h[126-136]
src/frame_processor.cpp[82-91]
src/frame_processor.cpp[180-199]
src/main.cpp[668-689]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`FrameProcessor::set_color_correction()` updates `drm_fd_` without atomic/mutex protection while the processor thread reads it every frame. This is a C++ data race (UB) when `set_color_correction()` is called at runtime (as the header/docs and main.cpp show).
### Issue Context
This PR made gain/offset atomics and added release/acquire on `color_correct_`, but `drm_fd_` remains non-atomic and is still written from the UI thread.
### Fix Focus Areas
- Make `drm_fd_` thread-safe (prefer `std::atomic<int>`), and use `store()`/`load()` consistently where it is written/read.
- Optionally, if `drm_fd_` is intended to be immutable after construction, remove the runtime write path and enforce/document that invariant.
- src/frame_processor.h[67-75]
- src/frame_processor.h[126-136]
- src/frame_processor.cpp[82-91]
- src/frame_processor.cpp[180-199]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}