Reduce Android video copies and fix OBS converter double-free

[YashasVM]

Android video normalization currently copies encoded frames into another buffer before converting length prefixes. Reuse the JNI buffer, validate complete length-prefixed input before modifying it, and reserve codec-header space once for keyframes. Packet content and malformed-input fallback remain unchanged.

Fix OBS converter ownership when FFmpeg replaces a cached scaling context. The new resolution/pixel-format regression test reproduces a double-free on the previous implementation under AddressSanitizer and passes with this fix.

Validation on a clean branch based on current main:

• 113 repository tests, including compiled native tests for buffer reuse, expected MPEG-TS output, 200 generated inputs, malformed/ambiguous prefixes, and a 1 MB frame.
• 23 Android unit tests, lint, debug APK build, and 16 KB native-library alignment.
• Linux plugin release build and four C++ test suites.
• Native normalization/muxing and OBS tests under AddressSanitizer and UndefinedBehaviorSanitizer. OBS leak checking suppresses only the system NVIDIA driver library, which retains 183 bytes at process exit; no project allocations are suppressed.

Live phone-to-OBS latency and battery improvements have not been measured. Hardware decoding is outside this change.

Summary by CodeRabbit

• Bug Fixes
  • Improved video-stream handling so malformed or ambiguous video data is preserved instead of being partially converted.
  • Fixed frame conversion when video dimensions change, helping maintain correct output dimensions and colors.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: a463c7d0-7534-4295-88ee-62560e3a7956

📥 Commits

Reviewing files that changed from the base of the PR and between e5beef3 and 0337687.

📒 Files selected for processing (4)

• android/app/src/main/cpp/openstream_srt.cpp
• obs-plugin/src/openstream-source.cpp
• obs-plugin/tests/test_source_settings.cpp
• tests/test_native_video_normalization.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The Android video path now validates and normalizes length-prefixed NAL units in place. The OBS frame converter updates cached scaler ownership. New tests cover normalization, mux output, and frame conversion across pixel formats and dimensions.

Changes

Android video normalization

Layer / File(s)	Summary
Normalize and mux video buffers android/app/src/main/cpp/openstream_srt.cpp, tests/test_native_video_normalization.py	normalizeAnnexB validates each NAL length before modifying the buffer and replaces valid prefixes in place. The send path moves the buffer into the normalizer and reserves capacity when prepending codec configuration. Tests cover valid and malformed inputs, storage reuse, and mux output.

OBS frame conversion

Layer / File(s)	Summary
Scaler ownership and format changes obs-plugin/src/openstream-source.cpp, obs-plugin/tests/test_source_settings.cpp	output_decoded_frame transfers the cached scaler out of its smart pointer before calling sws_getCachedContext, then stores the returned pointer. Tests cover BGRA output across RGB24 and BGR24 frames and changing dimensions.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 03376

No actionable issue is established that would prevent merging after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 03376

The changes preserve existing media interfaces and session checks, and no new security weakness was established. Remaining uncertainty concerns third-party resource ownership when conversion-context allocation fails.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• observed — The OBS converter processes frames decoded from the existing SRT/MPEG-TS input inside the OBS host process. Media dimensions and pixel format select the cached-scaler path. Each decode invocation owns its scaler locally; the PR changes resource ownership on this existing input path rather than adding another listener or caller.

Trust Boundaries and Controls

• observed — In-place normalization operates on the native copy, not directly on Java-owned memory. Before publishing codec configuration or sending media, native code checks the caller's session generation and connection state under mediaMutex. Begin-session and disconnect transitions invalidate media ownership and clear cached configuration; these controls are unchanged by the PR.

Resilience and Maintainability Implications

• inferred — After a NULL scaler result, conversion returns without output and a later frame can retry with an empty owner. Incomplete conversion also suppresses output. The decode-local owner is destroyed on loop exit, and stopping joins the worker. These internal cleanup paths are coherent, provided the external API consumes the supplied context according to the assumed contract; that dependency remains a coverage gap rather than a verified leak or vulnerability.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes both main changes: reducing Android video copies and fixing the OBS converter double-free.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[YashasVM]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

[YashasVM]

Merge assessment for 0337687:

• CodeRabbit completed its full review with no actionable findings and minimal merge risk; there are no inline review threads.
• Android, Linux plugin, Windows plugin, and website CI succeeded. The release-signed workflow is intentionally skipped for pull requests.
• Local checks additionally covered generated/malformed native video inputs, buffer reuse, 16 KB APK alignment, and address/undefined-behavior sanitizers. The new OBS regression test reproduces a double-free on the previous implementation and passes with this change.
• The generic 80% docstring-coverage warning is nonblocking. Validation-before-mutation and FFmpeg ownership semantics are documented directly at the changed operations; the native test module also documents its purpose. This does not justify adding boilerplate documentation to embedded/existing C++ functions.

Live stream latency and battery gains remain unmeasured; this PR makes no quantitative performance claim.