Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/reachy_mini/daemon/app/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
from reachy_mini.apps.manager import AppManager
from reachy_mini.daemon.app.routers import (
apps,
camera,
daemon,
kinematics,
motors,
Expand Down Expand Up @@ -104,6 +105,7 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]:

router = APIRouter(prefix="/api")
router.include_router(apps.router)
router.include_router(camera.router)
router.include_router(daemon.router)
router.include_router(kinematics.router)
router.include_router(motors.router)
Expand Down
57 changes: 57 additions & 0 deletions src/reachy_mini/daemon/app/routers/camera.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
"""Camera streaming API routes."""

import asyncio

import cv2
from fastapi import APIRouter, Depends
from fastapi.responses import StreamingResponse

from ...backend.mujoco.backend import MujocoBackend
from ...daemon import Daemon
from reachy_mini.media.camera_constants import CameraResolution
from reachy_mini.media.camera_opencv import OpenCVCamera
from ..dependencies import get_backend, get_daemon

Check failure on line 13 in src/reachy_mini/daemon/app/routers/camera.py

View workflow job for this annotation

GitHub Actions / ruff

Ruff (I001)

src/reachy_mini/daemon/app/routers/camera.py:3:1: I001 Import block is un-sorted or un-formatted

Check failure on line 13 in src/reachy_mini/daemon/app/routers/camera.py

View workflow job for this annotation

GitHub Actions / ruff

Ruff (I001)

src/reachy_mini/daemon/app/routers/camera.py:3:1: I001 Import block is un-sorted or un-formatted

Comment on lines +16 to +17
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Global state management without thread safety. The _shared_camera and _camera_refs variables are accessed and modified by multiple concurrent requests without synchronization (e.g., locks). This can lead to race conditions where:

  1. Multiple requests could simultaneously check _shared_camera is None and create multiple camera instances
  2. The reference count could be corrupted due to concurrent increments/decrements
  3. The camera could be closed while another request is still using it

Consider using asyncio.Lock or threading.Lock to protect these shared variables.

Copilot uses AI. Check for mistakes.
router = APIRouter(prefix="/camera")

_shared_camera = None
_camera_refs = 0

async def _get_shared_camera(is_sim: bool):
global _shared_camera, _camera_refs
if _shared_camera is None:
_shared_camera = OpenCVCamera(log_level="WARNING", resolution=CameraResolution.R1280x720)
_shared_camera.open(udp_camera="udp://@127.0.0.1:5005" if is_sim else None)
_camera_refs += 1
return _shared_camera

def _release_camera():
global _shared_camera, _camera_refs
_camera_refs -= 1
Comment on lines 29 to 37
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resource leak with reference counting. The reference counting mechanism is flawed because _camera_refs is incremented in _get_shared_camera() but only decremented in the finally block of _stream(). If a client disconnects unexpectedly or the stream generator is never consumed, the finally block may not execute, leading to a leaked reference that prevents the camera from being closed.

Additionally, the current design means the camera is only released when ALL clients disconnect. If you want to share the camera among concurrent clients, the cleanup should happen when the individual stream ends, not when all streams end. Consider using a context manager or a more robust reference counting mechanism with proper cleanup guarantees.

Copilot uses AI. Check for mistakes.
if _camera_refs <= 0 and _shared_camera is not None:
_shared_camera.close()
_shared_camera = None
_camera_refs = 0

Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space after equals sign in type annotation. Python style guide (PEP 8) requires spaces around the equals sign in type-annotated default values.

Should be: daemon: Daemon = Depends(get_daemon)

Copilot uses AI. Check for mistakes.
@router.get("/stream")
async def stream_camera(
backend=Depends(get_backend),
daemon: Daemon=Depends(get_daemon),
) -> StreamingResponse:
"""Stream camera feed as MJPEG."""
async def _stream():
is_sim = daemon.status().simulation_enabled and isinstance(backend, MujocoBackend)
cam = await _get_shared_camera(is_sim)
Comment on lines 22 to 55
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolution mismatch between camera initialization and usage. The camera is initialized with CameraResolution.R1280x720 (1280x720) on line 19, but the frames are immediately resized to 640x480 on line 44. This wastes bandwidth and processing:

  1. For hardware cameras, you're requesting 1280x720 from the camera and then downscaling
  2. For UDP cameras, you're receiving 1280x720 frames over UDP and then downscaling

Consider initializing the camera with a resolution closer to your target output size, or use CameraResolution.R640x480 if it exists. The PR description mentions "640x480 resolution" as an optimization, but this isn't achieved at the camera level.

Copilot uses AI. Check for mistakes.
try:
while True:
f = cam.read()
if f is not None:
Comment on lines 49 to 59
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing error handling for camera read failures. If cam.read() returns None (line 42-43), the frame is simply skipped. While this prevents crashes, it means clients could receive no frames for extended periods without any indication of an error. Consider:

  1. Adding a counter for consecutive None reads and raising an error after a threshold
  2. Logging warnings when frames are dropped
  3. Breaking the loop and returning an error response if the camera becomes permanently unavailable

This would provide better diagnostics when camera issues occur.

Copilot uses AI. Check for mistakes.
f = cv2.resize(f, (640, 480))
_, j = cv2.imencode(".jpg", f, [cv2.IMWRITE_JPEG_QUALITY, 80])
if _:
yield b"--frame\r\nContent-Type: image/jpeg\r\n\r\n" + j.tobytes() + b"\r\n"
await asyncio.sleep(0.04)
finally:
_release_camera()
return StreamingResponse(_stream(), media_type="multipart/x-mixed-replace; boundary=frame")

4 changes: 2 additions & 2 deletions src/reachy_mini/daemon/backend/mujoco/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,8 @@ def run(self) -> None:
if not self.headless:
viewer.sync()

rendering_thread = Thread(target=self.rendering_loop, daemon=True)
rendering_thread.start()
rendering_thread = Thread(target=self.rendering_loop, daemon=True)
rendering_thread.start()

# 3) now enter your normal loop
while not self.should_stop.is_set():
Expand Down
Loading