Conversation
…, harden release paths Android reservation now fully delegates to ReservationState: pending selection is advertised but not busy, only /reserve confirms. busy/advertised providers use isBusy/advertisedId, 45s lease split into pending rollback vs confirmed release. Fixes MainActivity val-vs-assignment compile break. Discovery prefers UDP peer with explicit manual override and blank-host rejection. Encoder log tags unified to shinEncoder with AVC hardware-only selection contract. Dock restores peer-bound lens/torch/identify via bounded slots (8 buttons). Worker exit documents release-before-clear destroy ordering. Tests: 101 passed.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…tall destinations - Android unit tests run on JVM against android.jar stubs where org.json.JSONObject throws 'not mocked'. Add test-only org.json:json so ObsDiscoveryProtocolTest exercises real parsing. On-device code still uses platform org.json. - Windows CMake MODULE target needs LIBRARY (and ARCHIVE) destination; configure failed with 'install TARGETS given no LIBRARY DESTINATION'. RUNTIME-only was insufficient. Tests: pytest -q 101 passed locally. Android unit-test + Windows build verification via CI.
YashasVM
left a comment
YashasVM
left a comment
There was a problem hiding this comment.
Yashas's code review bot: Reviewed the current reliability patch and its reservation-state migration. The pending-vs-confirmed ownership model is materially safer than the prior half-migrated state, the release paths preserve ownership checks, and all four current CI workflows (Android, Windows OBS, Linux OBS, Website) pass on this exact head. I found no remaining correctness, security, or merge-blocking issue in this PR. This head is safe to merge.
- Camera2Controller: appContext + nullable CameraManager, non-crashing select/open/switch with single-Back fallback and warnings. - CameraControlServer: clientLock sync, bind errors surfaced via onError. - SrtStreamClient: guarded native load (loadError/isAvailable), explicit unavailable error instead of UnsatisfiedLinkError crash. - docs/architecture: canonical 51615/239.255.43.99/9100/9101. - gitignore local .claude/ agent scaffolding. Tests: pytest -q 113 passed.
Keep pending-advertised/confirmed-busy ReservationState model while porting crash/ANR fixes: appContext, native-unavailable status, control onError surface, onStop off-UI (shinActivityStop daemon), identify leak fix, permission-denied UI, bitrate lock, lens restart off-UI with single-flight lock, stopPhoneServer blockingWork off-UI with Reserved escape hatch. Tests: pytest -q 113 passed.
…rties - Discovery registry bounded 64 evict-eldest with warning (AGENTS 3/16). - Reserved names SRT target + control-OK log (TCP 9101 OK vs SRT 9100 blocked diagnosable). - Properties minimal: Test/Disconnect/Zoom/Identify only; torch/lens live in dock via bounded send (no duplication). Drop dead command table. Tests: pytest -q 113 passed.
- New test_prod_hardening_contract: 12 guards covering crash-free Android, ReservationState preserved, bounded discovery, diagnosable Reserved, dock-full/properties-minimal split, canonical docs. - test_srt_contract: accept guarded native wrappers (nativeSendVideo/ nativeSendAudio) + require loadError/isAvailable. Tests: pytest -q 113 passed.
YashasVM
left a comment
YashasVM
left a comment
There was a problem hiding this comment.
Yashas's code review bot: IMPORTANT — the ported off-UI teardown has the same replacement-session race here. stopPhoneServer() returns immediately on the main thread while a background worker later disconnects the shared streamClient and stops camera/encoder/audio. The Stop button immediately calls startPreviewIfAllowed() / startPhoneServerIfAllowed() after that return, and a fast onStop() → onStart() can also restart discovery/control/listening before shinActivityStop finishes. The stale teardown can then stop the newly created session. Please make teardown/restart ordering explicit (or generation-guard stale teardown against newer starts) and cover stop→immediate restart plus lifecycle stop→start with a regression/contract test before merging.
1 participant
Resolves code-review + grill findings on HEAD working-tree diff.
Standards:
Spec (reliability-audit + AGENT_PROGRESS provisional):
Follow-up (prod-ready pass, 4 commits):
Queues (Rule 16): discovery 64 evict-eldest + warning; control/lifecycle bounded coalescing slots; release queue urgent cap 4 drop-newest.
Perf (Rule 14): UI teardown 0ms block (was 2s join); discovery O(1) bounded; lens restart + MediaCodecList off UI; OBS slot render diffed (no per-beacon inflation).
HW (Rule 18): single HW AVC encoder, explicit throw, no silent SW; decode HW probe with explicit logged fallback.
Merge note: unified design = dock FULL (8 peer-bound buttons) + properties MINIMAL (no duplication). Pairs with #40 (crash/connect scope); if #40 merges first, this branch rebases keeping this dock. Physical 2-phone Wi-Fi acceptance still outstanding per AGENT_PROGRESS.