perf(fpv): drop the redundant client-side color conversion

[anjrew]

What

The FPV frame path converts colors twice on every delivered frame:

Stage	Conversion
Simulator capture (_tick_impl, every streamed frame)	glReadPixels RGBA → cv2.cvtColor(arr, COLOR_RGBA2BGR)
Client get_frame_read (every read)	cv2.imdecode → cv2.cvtColor(image, COLOR_BGR2RGB)

The frame only ever travels between these two points through a cv2.imencode('.png') → cv2.imdecode round trip, and that round trip is an identity on channel order (lossless PNG, decoded straight back to an array by the same library). So the two conversions are pure round-tripping: BGR on the way out, RGB on the way in.

Capturing straight to RGB in the simulator (COLOR_RGBA2RGB — same cost as COLOR_RGBA2BGR) makes the stored/encoded frame already RGB, so the client can return the decoded frame as-is and skip its own full-frame cvtColor entirely.

• Net effect: one fewer full-frame color conversion per frame read, on the client's per-frame hot path (get_frame_read, called in a loop by e.g. examples/6_record_video.py).
• Contract unchanged: get_frame_read() still returns RGB, matching the real Tello / djitellopy. All examples already treat the frame as RGB (COLOR_RGB2BGR before imwrite) and are unaffected.

Why it's safe

The output is byte-identical, proven both algebraically and empirically over a full 360×640 frame:

# current  vs  proposed
cv2.cvtColor(imdecode(imencode(cv2.cvtColor(x, RGBA2BGR))), BGR2RGB)
        ==   imdecode(imencode(cv2.cvtColor(x, RGBA2RGB)))
# -> np.array_equal(...) == True

Both reduce to (R, G, B) from the source (R, G, B, A) pixel.

Dead FPV state removed along the way

• UrsinaAdapter.get_latest_frame() — unused (the get_latest_frame command handler reads adapter.latest_frame directly and encodes it). It assumed BGR, so leaving it would have made it a latent bug after this change. Its now-orphaned from cv2.typing import MatLike import goes with it.
• CommandServer.latest_frame — never read anywhere; the server uses adapter.latest_frame.

Verification

• python -m py_compile on all touched modules — clean.
• ruff check on the three touched files — only the pre-existing documented F403 baseline (from ursina import *); no new findings, and removing the MatLike import left no unused-import/undefined-name error.
• Color-path equivalence proven empirically (numpy/opencv, full-frame random RGBA → np.array_equal True).
• No GUI/visual run possible in this environment, but the change is byte-identical by construction, so no visual regression is possible.

Scope check

No open issues, and neither open PR (#19 scene-editor, #22 agent-kombat) touches the FPV color path — no duplication.

🤖 Generated with Claude Code

---

Generated by Claude Code

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

Summary by CodeRabbit

• Bug Fixes
  • Improved simulator video frame handling by storing and returning images in the correct color format.
  • Removed redundant image conversion during frame retrieval, improving efficiency and preventing color inaccuracies.

[coderabbitai]

Warning

Review limit reached

Next included review available in 54 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9c156302-f900-43cb-a7e9-f9c2ffc5d5c3

📥 Commits

Reviewing files that changed from the base of the PR and between f53da2f and 8e69fd7.

📒 Files selected for processing (1)

• tests/test_fpv_frame_color.py

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: be2b6a2e-9321-47c5-a16d-aa0f1acfd846

📥 Commits

Reviewing files that changed from the base of the PR and between 10746b5 and f53da2f.

📒 Files selected for processing (3)

• tello_sim/command_server.py
• tello_sim/ursina_adapter.py
• tello_sim_client.py

💤 Files with no reviewable changes (1)

• tello_sim/command_server.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The simulator now stores streamed frames in RGB order. The client returns decoded frames without a second color conversion. Unused frame access code and initialization were removed.

Changes

Frame pipeline cleanup

Layer / File(s)	Summary
RGB frame flow tello_sim/ursina_adapter.py, tello_sim_client.py	Streamed frames now use RGB channel order. get_frame_read returns decoded images directly.
Unused frame access cleanup tello_sim/ursina_adapter.py, tello_sim/command_server.py	The unused get_latest_frame method, MatLike import, and CommandServer.latest_frame initialization were removed.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: 🔵 Low · up to f53da

Low risk: the change removes one client-side color conversion, but external consumers may be affected because the latest-frame PNG path now handles RGB data differently, and an unused-looking frame accessor is removed. The PR is mergeable with explicit owner confirmation that these external compatibility paths are not relied upon or are handled appropriately.

Poem

I hop through RGB, bright and clear,
No extra color swap appears.
Old frame hooks rest in the hay,
Clean pixels stream along their way.
Squeak, ship, and render today!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the primary change: removing the redundant client-side FPV color conversion.
Docstring Coverage	✅ Passed	Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch perf/fpv-drop-redundant-color-conversion

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Adversarial review — f53da2f

Blocking

(none)

Non-blocking nits

• tello_sim_client.py:48-54 — comment claims "imencode/imdecode round trip preserves channel order" but no verification code is included. The reasoning rests on this claim being true (the old code converts BGR→RGB after decode; the new code assumes decode yields RGB), so it's fragile without test coverage. Suggest adding an assert or commented verification that documents the empirical behavior.

SUSPECT (not confirmed, worth a second look)

• tello_sim_client.py:48-54 — color channel preservation claim needs verification. OpenCV's cv2.imdecode(..., cv2.IMREAD_COLOR) traditionally interprets all color images as BGR by convention, which would swap channels on RGB-encoded PNG data. The commit message claims empirical verification: BGR2RGB(imdecode(imencode(RGBA2BGR(x)))) == imdecode(imencode(RGBA2RGB(x))) over a 360x640 frame, but no test or verification code appears in the diff. Either (a) OpenCV's PNG handling is channel-agnostic in a way that differs from the standard IMREAD_COLOR behavior, or (b) the claim needs to be tested and documented. Before merge, confirm that a real client receiving frames from the simulator produces RGB-correct output (e.g., by comparing against known-good djitellopy reference frames). — Run a side-by-side comparison of frames from old and new pipelines; if byte-identical, the claim is sound; if channels are swapped, it's a latent bug.

[anjrew]

Addressed the review nit / SUSPECT point in 8e69fd7 by adding tests/test_fpv_frame_color.py — a self-contained, framework-free check (run python tests/test_fpv_frame_color.py, also pytest-discoverable) that proves, over a full 360×640 frame, both that the client's decoded frame equals the RGB the simulator captured and that the new pipeline is byte-identical to the old one.

On the underlying suspicion — you're right that cv2.imdecode(..., IMREAD_COLOR) is BGR-by-convention in isolation. The reason it's safe here is that both ends are OpenCV: cv2.imencode treats its input array as BGR and writes a standard RGB PNG (swapping internally), and cv2.imdecode reads that RGB PNG straight back into a BGR array (swapping again). The two swaps cancel, so the imencode → imdecode round trip is an identity on the array bytes regardless of what the channels actually mean — it's the pairing that's channel-agnostic, not imdecode alone. That's exactly what the test pins down, so a future change to either end that breaks the symmetry will fail it rather than silently swapping red/blue.

---

Generated by Claude Code

[github-actions]

Adversarial review — 8e69fd7

Blocking

(none)

Non-blocking nits

(none)

SUSPECT (not confirmed, worth a second look)

(none)

No defects found after adversarial pass.