pyronear / pyronear/pyro-engine

`/cameras/capture?patrol_id=` moves the camera outside the per-camera lock

Open
#395 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
19
Forks
8
PR merge metrics
No merged PRs in 30d

Description

Context

Every PTZ endpoint in routes_control.py goes through _acquire_or_409 (pyro_camera_api/api/routes_control.py:536): it takes MOVE_LOCKS[camera_ip] or returns 409, and holds it for the whole duration of a blocking move, including the server-side sleep.

That is what guarantees a single mouvement at a time per camera, and since PR #390 it is also what protects the dead-reckoned azimuth from being written by two places at once.

Two code paths issue PTZ commands without ever touching that lock:

  1. the patrol loop, pyro_camera_api/camera/patrol.py:85, already documented as a known limitation in #390;
  2. the capture route, pyro_camera_api/api/routes_cameras.py:122, where _capture_impl calls cam.capture(patrol_id=patrol_id). For Reolink that runs move_camera("ToPos", idx=patrol_id) followed by sleep(1) and a snapshot (camera/adapters/reolink.py:98-100).

The second one is worth tracking separately because it is not covered by "stop the patrol before manual control". It is an endpoint that reads like a read-only capture, is reachable by any client at any time, and takes no lock and performs no active-stream check. Consequences while a manual PTZ move is in flight:

  • the camera physically leaves the operator's target mid-move;
  • _sync_azimuth_from_pose writes the preset azimuth under the running handler, which then overwrites it with its own dead reckoning, so /control/azimuth reports a confident wrong value instead of null;
  • moving stays false throughout, so no client can detect any of it.
Why this is not a one-liner

The obvious fix, wrapping the capture in _acquire_or_409, is worse than the problem. cam.capture(patrol_id=...) takes one to three seconds end to end, so holding the lock for its duration would make every manual PTZ command fail with a 409 during each engine capture, which happens continuously during patrol-driven inference.

Options worth weighing:

  • acquire the lock only around the ToPos, release it before the snapshot. Narrows the window without starving manual control, but the camera is still settling unlocked.
  • non-blocking acquire: if the camera is busy, skip the move and serve last_images[patrol_id] instead of repositioning. Changes the freshness contract of the endpoint.
  • refuse patrol_id on the capture route entirely and require callers to use /control/goto_preset first, which is locked. Cleanest semantics, but it is a breaking change for existing callers.

The same reasoning applies to the patrol loop itself, which would need to hold the lock per pose and release it between poses.

Related
  • #390 (azimuth tracking), known-limitation note in the description.
  • Would also let /control/azimuth expose whether a patrol is currently driving the camera, so moving stops being misleading.

Priority: low. No known caller captures with a patrol_id during a live session, and the engine skips inference while a stream is active.

Co authored with claude.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Read _acquire_or_409 in pyro_camera_api/api/routes_control.py:536, the capture path in pyro_camera_api/api/routes_cameras.py:122, and Reolink handling in camera/adapters/reolink.py:98-100. Compare the capture and patrol paths, then establish which locking or endpoint behavior is intended; done means capture cannot disrupt an in-flight PTZ move or corrupt azimuth state.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
api, backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.