Skip to content

Serialise stderr capture around video source opens - #3012

Closed
shntu wants to merge 1 commit into
mainfrom
fix/serialise-stderr-capture
Closed

shntu wants to merge 1 commit into
mainfrom
fix/serialise-stderr-capture

Conversation

@shntu

@shntu shntu commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

What changed

capture_process_stderr in inference/core/interfaces/camera/stream_error_classifier.py now takes a process-wide lock for the duration of the capture and drains the pipe on a thread instead of one os.read after the fact.

Why

The helper redirects the process-wide fd 2 into a pipe around every cv2.VideoCapture open so FFmpeg's error text can be classified. Each VideoSource opens its capture on its own thread, so two sources starting together (for example two RTSP streams in one process after a restart) interleave their redirects:

  1. Source A dups fd 2 to its pipe.
  2. Source B saves fd 2, which is now A's pipe write end, and dups fd 2 to its own pipe.
  3. A finishes first, restores fd 2, closes its write end and does a blocking os.read on its pipe. B still holds a dup of A's write end, so if A's open wrote nothing to stderr the read never returns.
  4. B finishes and restores fd 2 to what it saved, which is A's pipe. A stays blocked and the process's stderr now points at a pipe nobody reads.

The source that opened first sits in INITIALISING forever with no error. Only a process restart recovers it, and that repeats the race.

The lock is the same approach opencv_rtsps_tls_env already uses to serialise the capture-options env var around the same open, and it is held across the open for the same reason. The drain thread fixes a second problem in the old single read: anything over the pipe buffer (64 KiB on Linux) blocked the writer inside the open, and only the first 64 KiB was ever kept. The thread is joined before the context exits, so nothing outlives the capture.

An open timeout for the capture itself is a separate change and not part of this PR.

Tests

Two regression tests in tests/inference/unit_tests/core/interfaces/camera/test_stream_error_classifier.py, collected by unit_tests_inference_x86.yml:

  • test_capture_process_stderr_serialises_interleaved_opens forces the interleave above with events. Before the fix it fails with fd 2 left pointing at a pipe after both captures exit. After the fix both captures complete with their own output and fd 2 is restored.
  • test_capture_process_stderr_drains_output_larger_than_pipe_buffer writes 256 KiB to fd 2 inside a capture. Before the fix the writer blocks forever. After the fix everything is captured.

Commands run:

python -m pytest tests/inference/unit_tests/core/interfaces/camera/
# 276 passed

python -m pytest tests/inference/unit_tests tests/unit/core/cache/test_air_gapped.py
# 3552 passed, 10 skipped, 1 failed
# The failure is tests/inference/unit_tests/core/utils/test_url_utils.py::test_gateway_scheme_policy_is_shared_with_model_provider,
# which fails identically on unmodified main in my environment (published inference-models instead of ./inference_models).

make check_code_quality   # black, isort, flake8 on the changed files

capture_process_stderr redirects process-wide fd 2 into a pipe for the
duration of each cv2.VideoCapture open. Sources open on their own threads,
so two opens starting together interleave their redirects: the first to
finish blocks forever reading a pipe whose write end the second still
holds, and fd 2 is finally restored to that pipe instead of real stderr.

Take a process-wide lock around the capture and drain the pipe on a thread
so output larger than the pipe buffer cannot block the writer either.
@github-actions

Copy link
Copy Markdown
Contributor

👋 Thanks for the pull request! Here is how automated Claude review works here, so you spend credits (and reviewer time) wisely.

🚧 Right now this is a draft, so automated Claude review is paused — nothing is being spent yet. Mark it Ready for review to trigger it.

Warning

💸 The Claude reviewer bills in credits, not vibes

Automated review spins up a real agent that reads real code and spends real credits on every pass. It is glad to help — but it is not a rubber duck, a linter you poke in a loop, or a substitute for reading the contributing guide. Treat it like an expensive senior reviewer whose time you booked, and show up prepared.

Draft when unsure, Ready when you mean it:

  • 🌱 Not sure the PR is in good shape yet? Keep it (or set it back) as a draft — drafts pause review, so you can push and iterate without burning credits on a moving target.
  • 💪 Feel strong about the contents? Mark it Ready for review and the reviewer will take a look.

However you get there, arrive prepared:

  • 🧱 Bring a SOLID, thorough PR. Point your local agent at our skills/ to tune it to our guidelines first — or, if you are one of those fabled carbon-based contributors, read them yourself. A half-baked diff costs exactly the same to review as a finished one.
  • ✅ Resolve every comment before you re-request review. Re-requesting with threads still open means paying twice for the same conversation.
  • 🔁 Do not use CI review as an inner loop for a local agent. The reviewer is not a step-by-step debugger — do the unfolding locally and arrive with the answer, not the search.
  • 🙋 If something looks off, ask a human. One question to a maintainer is cheaper and faster than three rounds of agent re-review chasing a misread.

Reviews are not free. A draft costs nothing to review; a Ready PR is a promise that it is worth reviewing.

  • Prefer to skip automated review entirely? Add the skip-claude-review label.

@PawelPeczek-Roboflow

Copy link
Copy Markdown
Collaborator

Duplicate re #3014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants