pyronear / pyronear/pyro-engine
`/cameras/capture?patrol_id=` moves the camera outside the per-camera lock
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:
- the patrol loop,
pyro_camera_api/camera/patrol.py:85, already documented as a known limitation in #390; - the capture route,
pyro_camera_api/api/routes_cameras.py:122, where_capture_implcallscam.capture(patrol_id=patrol_id). For Reolink that runsmove_camera("ToPos", idx=patrol_id)followed bysleep(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_posewrites the preset azimuth under the running handler, which then overwrites it with its own dead reckoning, so/control/azimuthreports a confident wrong value instead of null;movingstays 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_idon the capture route entirely and require callers to use/control/goto_presetfirst, 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/azimuthexpose whether a patrol is currently driving the camera, somovingstops 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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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