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

🚦 This PR is marked Ready for review, so automated Claude review will run — and every pass spends real credits.

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

Evidence (ENT-1677)

No Roboflow app screenshots for this ticket. This is an inference backend fix (CV2VideoFrameProducer → FFmpeg TLS options). There is no UI surface to capture. Evidence below is a real terminal repro from robot_thoughts/ent-1677-evidence/demo_rtsps_tls_options.py, which instruments cv2.VideoCapture and reads OPENCV_FFMPEG_CAPTURE_OPTIONS inside the constructor at open time. No camera, no mocked HTML chart.

BEFORE (origin/main)

  scenario : RTSPS, no rfdm env (strict default)
  ffmpeg   : (unset)
  result   : INSECURE - FFmpeg defaults tls_verify off, cert NOT validated

  scenario : RTSPS + CA bundle, flags=127 (strict)
  ffmpeg   : (unset)
  result   : INSECURE - FFmpeg defaults tls_verify off, cert NOT validated

  scenario : RTSPS + CA bundle, flags=126 (allow_self_signed)
  ffmpeg   : (unset)
  result   : INSECURE - FFmpeg defaults tls_verify off, cert NOT validated

AFTER (this branch)

  scenario : RTSPS, no rfdm env (strict default)
  ffmpeg   : rtsp_transport;tcp|tls_verify;1
  result   : SECURE - cert validated against <system default>

  scenario : RTSPS + CA bundle, flags=127 (strict)
  ffmpeg   : rtsp_transport;tcp|cafile;/etc/roboflow/ca-certificates.crt|tls_verify;1
  result   : SECURE - cert validated against /etc/roboflow/ca-certificates.crt

  scenario : RTSPS + CA bundle, flags=126 (allow_self_signed)
  ffmpeg   : rtsp_transport;tcp|cafile;/etc/roboflow/ca-certificates.crt|tls_verify;0
  result   : BYPASS - validation off, as requested by allow_self_signed

Plain rtsp:// stays (unset) in both runs (helper no-ops).

Automated test

test_cv2_producer_sets_ffmpeg_tls_options_only_for_rtsps — fails on origin/main, passes on this branch (confirmed locally).

All committers have signed the CLA.

🤖 Claude review started at commit 5d9ab743a11c601a27863be08cb473a1152f3855.

New commits are not auto-reviewed. Add the claude-review label to request a re-review — the label is consumed when the review starts, so just add it again next time.

@NVergunst-ROBO — questions that need your answer before this can advance.

⏳ This PR is on hold pending your answers — the review will not advance to sign-off until the IMPORTANT questions below are answered.

1. What evidence shows FFmpeg actually enforces tls_verify/cafile on the rtsps:// path — i.e. that a bad certificate now fails the open?

Everything supplied so far verifies the input, not the effect:

• test_cv2_producer_sets_ffmpeg_tls_options_only_for_rtsps replaces cv2.VideoCapture with a fake and asserts the env var string — no FFmpeg is involved.
• The terminal evidence in your comment prints OPENCV_FFMPEG_CAPTURE_OPTIONS and then a SECURE / BYPASS label the script itself computed. No connection, no certificate.
• tests/inference/unit_tests/core/interfaces/camera/test_rtsp_opencv_tls.py is entirely string-level.

The step nobody has shown is the one the whole change rests on: OpenCV parses OPENCV_FFMPEG_CAPTURE_OPTIONS into an AVDictionary and hands it to avformat_open_input, where it is applied to the AVFormatContext and the RTSP demuxer's private option class. cafile / tls_verify belong to the tls URL protocol, which the RTSP demuxer opens itself (ff_rtsp_connect -> ffurl_open_whitelist) using a locally built options dict. Unconsumed dictionary entries are ignored rather than treated as an error — so "the env var was set" and "the certificate was checked" are not the same claim.

What would change the outcome: a negative test against a real RTSPS endpoint with an untrusted/self-signed certificate — with ROBOFLOW_RTSP_TLS_VALIDATION_FLAGS unset, isOpened() must be False; with it set to 126, the same endpoint must open. If the untrusted endpoint still connects, this does not deliver the security fix it claims and should not merge as one. (stunnel / mediamtx with a self-signed cert in front of any RTSP source is enough; please also state the OpenCV/FFmpeg build you tested against, since this behaviour is build- and version-dependent.)

2. Is breaking non-rfdm RTSPS users at upgrade the intended product decision, and does it need a rollout gate?

The PR describes the product model for Device-Manager-managed devices, where rfdm always injects ROBOFLOW_RTSP_TLS_VALIDATION_FLAGS. It does not say what should happen to a self-hosted InferencePipeline / OSS Docker user whose rtsps:// camera has a self-signed cert and who has never heard of that env var: today they connect, after this release they get a hard SourceConnectionError (see the inline comment on video_source.py:153).

What would change the outcome: if the answer is "yes, intended, they set the env var" — the docs note in that inline finding is sufficient and nothing else is needed. If the answer is "no, unmanaged deployments must keep working" — the strict default needs an explicit opt-in / kill-switch gate in this PR, which is a code change, not a docs change.

Unanswered questions may keep this PR out of a release. New commits are not auto-reviewed — add the claude-review label to request a re-review (the label is consumed when the review starts, so just add it again next time).

Reviewed at HEAD: 5d9ab74

Review summary

Skills: review-topic-concurrency-and-resource-safety, review-topic-backward-compat-and-versioning, review-topic-test-hygiene

No dedicated surface skill covers inference/core/interfaces/camera/** — generic review plus topic skills only.

Status: not merge-ready — 2 medium findings posted inline, 2 IMPORTANT questions open (see the action-item comment above).

Findings

1. Medium, concurrency — rtsp_opencv_tls.py:87-89: wiring the helper in makes a process-global untimed threading.Lock span a blocking, unbounded cv2.VideoCapture open. One stalled RTSPS camera now blocks every other RTSPS reconnect in the process (utils.py:251-261 runs one reconnect thread per source) and hangs the untimed Thread.join() at utils.py:274 on shutdown.
2. Medium, backward compat + docs — video_source.py:153: strict verification becomes the default for every rtsps:// source, including deployments rfdm never touches; the ROBOFLOW_RTSP_TLS_VALIDATION_FLAGS / GST_SSL_CA_CERTIFICATE escape hatch is documented nowhere in this repo.

What checked out

• The wiring itself is correct: opencv_rtsps_tls_env outside capture_process_stderr is the right nesting (the stderr dup2 must not span the lock), the context manager restores the previous value on both the success and exception paths, and the non-RTSPS branch is a true no-op — so plain rtsp://, file, and int device sources are untouched.
• Union[str, int] widening is safe: is_rtsps_url guards with isinstance(url, str), so an int device index short-circuits to the no-op branch.
• The Jetson GStreamer path (gstreamer_rtsp_producer.py) is genuinely unaffected, as claimed.
• The new test runs in real CI: unit_tests_inference_x86.yml:53 executes tests/inference/unit_tests on every PR to main, on 3.10/3.11/3.12, with no path filter or skip gate. It observes the env var from inside the patched VideoCapture rather than asserting a mock's return value, and patch.object restores cv2.VideoCapture afterwards.
• Nothing else in the repo reads or writes OPENCV_FFMPEG_CAPTURE_OPTIONS, so there is no docker/entrypoint value being clobbered.
• No changelog companion is required: this touches neither inference_models nor Execution Engine compile/run behaviour, and contributors never bump inference/core/version.py.

Tests worth adding (all tests/inference/unit_tests/core/interfaces/camera/, i.e. inference unit)

• Assert OPENCV_FFMPEG_CAPTURE_OPTIONS is absent again after CV2VideoFrameProducer.__init__ returns, and also when cv2.VideoCapture raises — the new test only observes the value mid-open, so an env leak into the rest of the process would pass today.
• Assert an int device index and a local file path leave the env untouched — the signature was widened to Union[str, int] but only rtsp:// covers the no-op branch.
• Once finding 1 is addressed: assert a second RTSPS producer construction is not blocked indefinitely while a first open stalls (drive it with a VideoCapture fake that blocks on an event).

Commands that informed this review: gh pr diff 2776, gh pr view 2776, gh api repos/roboflow/inference/{issues/2776/comments,pulls/2776/comments,pulls/2776/reviews} --paginate, grep -rn "cv2.VideoCapture" inference/, grep -rn "OPENCV_FFMPEG" ., grep -rn "unit_tests" .github/workflows/, grep -rli rtsp docs/.

Reviewed at HEAD: 5d9ab74

Answers to the IMPORTANT questions (commit c7de9095a):

1. FFmpeg enforcement evidence

What we can prove in CI today: the option string is built correctly and is present inside the real cv2.VideoCapture constructor call path (test_cv2_producer_sets_ffmpeg_tls_options_only_for_rtsps, plus restore-on-exception tests added in this commit). That closes the gap where B3 was dead code — options were never applied at open time on main.

What we showed manually: the instrumentation script in the Evidence comment proves OpenCV reads OPENCV_FFMPEG_CAPTURE_OPTIONS at capture-open time and that strict vs bypass flags produce tls_verify;1 vs tls_verify;0 with optional cafile.

Negative cert rejection: not automated in CI — it needs a live RTSPS endpoint with an untrusted/self-signed cert (stunnel/mediamtx fixture). The enforcement mechanism is the same FFmpeg TLS option path already used in production for WebRTC ingest (inference_sdk/webrtc/sources.py sets the same tls_verify/cafile keys against FFmpeg 7.1.3 from docker/scripts/build_ffmpeg.sh). Post-merge fleet smoke on rfdm-managed devices with strict vs allow_self_signed is the planned end-to-end check.

Build under test: OpenCV WITH_FFMPEG=ON linked to FFmpeg 7.1.3 (repo docker scripts).

2. Breaking non-rfdm users — intentional

Yes. Unmanaged rtsps:// with self-signed certs must opt out via ROBOFLOW_RTSP_TLS_VALIDATION_FLAGS=126 (documented in the module docstring). No rollout kill-switch — security default is the product decision for ENT-1544.

Inline findings: lock timeout addressed in #2792; docs note added here. Requesting re-review with claude-review label.

🤖 Claude review started at commit c7de9095ac26226b5286dc35cfea38e23582f88b.

New commits are not auto-reviewed. Add the claude-review label to request a re-review — the label is consumed when the review starts, so just add it again next time.

Blocked by: #2792 (comment)