Skip to content

feat(plugin-sdk): add frame pool allocation for native plugins#249

Merged
streamer45 merged 5 commits intomainfrom
devin/1775334640-native-plugin-frame-pool
Apr 5, 2026
Merged

feat(plugin-sdk): add frame pool allocation for native plugins#249
streamer45 merged 5 commits intomainfrom
devin/1775334640-native-plugin-frame-pool

Conversation

@staging-devin-ai-integration
Copy link
Copy Markdown
Contributor

@staging-devin-ai-integration staging-devin-ai-integration bot commented Apr 4, 2026

Summary

Zero-copy buffer semantics for native plugin video/audio frames via host-side frame pool allocation. Eliminates per-frame memcpy on the hot path by letting plugins write directly into pool-allocated buffers and pass them back by handle.

DHAT profiling results (300-frame Slint watermark pipeline):

  • Total allocations: 1,042,800,741 → 252,676,535 bytes (−75.8%)
  • Target to_vec copy site (9,504,000 bytes / 300 blocks): eliminated entirely

Changes

SDK (sdks/plugin-sdk/native/src/)

  • New PooledVideoBuffer / PooledAudioBuffer linear-type wrappers with Drop-based pool return
  • OutputSender::alloc_video(), alloc_audio(), send_video(), send_audio() for zero-copy path
  • CNodeCallbacks struct consolidating output + telemetry + alloc callbacks (replaces positional params)
  • CNodeCallbacks.struct_size field for forward-compatible extensibility
  • result_from_c() helper extracting duplicated CResult→Result conversion
  • CAllocVideoResult / CAllocAudioResult allocation result types
  • Buffer types re-exported in prelude
  • API version bump: 5 → 6

Host (crates/plugin-native/src/wrapper.rs)

  • CallbackContext extended with VideoFramePool / AudioFramePool
  • alloc_video_shim / alloc_audio_shim / free_*_buffer C callbacks
  • free_packet_buffer_handle() safety net: frees pooled handles on callback error paths
  • build_node_callbacks() constructs the consolidated struct with struct_size

Conversions (sdks/plugin-sdk/native/src/conversions.rs)

  • packet_from_c: zero-copy branch reclaims PooledVideoData/PooledSamples via Box::from_raw
  • packet_from_c: null-data/null-samples error paths now free buffer_handle before returning (fixes buffer leak)
  • packet_to_c: extended CPacketOwned::Video/Audio with metadata storage (prevents dangling pointer)
  • Regression tests for buffer-handle cleanup on null-pointer error paths

Slint plugin

  • Uses pool allocation with automatic fallback to legacy copy path

Review feedback addressed

From reviewer:

  • Critical fix(tools): install script #1: Fixed buffer leak in packet_from_c when data/samples is null but buffer_handle is non-null
  • Critical docs: improve UI documentation #2: Fixed buffer leak in output_callback_shim when pin name parsing fails before packet_from_c runs
  • Suggestion fix(plugins): resolve flaky native plugin reload test #3: Removed redundant data_len/sample_count params from send_video/send_audio — now uses buf.len()/buf.sample_count()
  • Suggestion docs: remove redundant note #4: Removed const fn from as_mut_slice/consume — these dereference heap pointers and will never be called in const context. Added #[allow(clippy::missing_const_for_fn)] with rationale.
  • Suggestion Built-in authentication #5: Added struct_size to CNodeCallbacks for v7-on-v6 forward compat
  • Suggestion fix(docs): http API inaccuracy #6: Extracted result_from_c() helper for CResult→Result conversion (4 call sites)
  • Nit: Renamed CAllocResultCAllocVideoResult
  • Nit: Added PooledVideoBuffer/PooledAudioBuffer to prelude
  • !Send: Acknowledged — both buffer types are !Send due to raw pointers, consistent with OutputSender. If cross-thread buffer passing is needed later, explicit unsafe impl Send would be required.

From Devin Review:

  • Fixed resource leak in alloc_video/alloc_audio when free_fn is None but allocation succeeded — now checks free_fn.is_none() alongside data.is_null()

Additional:

  • Downgraded info!trace! for debug logging in __plugin_flush macro (4 lines that would spam production logs)

Review & Testing Checklist for Human

  • Buffer ownership across FFI: Verify Box::into_rawBox::from_raw round-trip in alloc_video_shimpacket_from_c is sound (the Vec heap pointer survives the Box indirection)
  • Error-path cleanup: Confirm free_packet_buffer_handle in output_callback_shim correctly handles all error paths where buf.consume() has already suppressed Drop
  • struct_size field: Verify CNodeCallbacks struct layout with the new leading struct_size: usize field doesn't break ABI alignment for any callback pointer fields
  • Backward compat: Build a plugin against SDK v6 with alloc_video returning null (no pool) — should fall through to legacy .to_vec() copy path
  • End-to-end: Run Slint watermark pipeline and verify video output is correct with pool allocation active

Notes

  • All 10 non-Slint native plugins compile unchanged against the new API — the CNodeCallbacks consolidation is handled transparently by the macros.

Link to Devin session: https://staging.itsdev.in/sessions/ed6a1517580e43839384cd56691bf505
Requested by: @streamer45


Staging: Open in Devin

streamkit-devin and others added 2 commits April 4, 2026 21:13
Implement zero-copy buffer semantics for native plugin video/audio frames
via host-side frame pool allocation, eliminating per-frame heap allocations
from .to_vec() copies in the conversions layer.

Changes:
- Bump NATIVE_PLUGIN_API_VERSION from 5 to 6
- Add CAllocResult, CAllocAudioResult, CAllocVideoFn, CAllocAudioFn types
- Consolidate callback parameters into CNodeCallbacks struct
- Extend CVideoFrame/CAudioFrame with buffer_handle and metadata fields
- Add PooledVideoBuffer/PooledAudioBuffer SDK wrappers with linear-type
  semantics (Drop returns buffer to pool if not consumed)
- Add OutputSender::alloc_video/alloc_audio/send_video/send_audio methods
- Implement host-side alloc/free shims in wrapper.rs using frame pools
- Update native_plugin_entry! and native_source_plugin_entry! macros
- Update Slint plugin to use pool allocation with legacy fallback
- Extend CPacketOwned with VideoOwned/AudioOwned for metadata ownership

DHAT profiling (300-frame Slint watermark pipeline):
  Before: 1,042,800,741 bytes total, 9,504,000 bytes from to_vec copies
  After:    252,676,535 bytes total, 0 bytes from to_vec copies
  Result: 75.8% total allocation reduction, to_vec copies eliminated

Signed-off-by: Devin AI <devin@cognition.ai>
Signed-off-by: StreamKit Devin <devin@streamkit.dev>
Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
Signed-off-by: Devin AI <devin@cognition.ai>
Signed-off-by: StreamKit Devin <devin@streamkit.dev>
Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
@staging-devin-ai-integration
Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Copy link
Copy Markdown
Contributor Author

@staging-devin-ai-integration staging-devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 6 additional findings.

Staging: Open in Devin
Debug

Playground

Critical fixes:
- Free pooled buffer_handle in packet_from_c when data/samples pointer
  is null (prevents leak on malformed frames)
- Free pooled buffer_handle in output_callback_shim when pin name
  parsing fails before packet_from_c can reclaim it

API improvements:
- Remove redundant data_len from send_video (use buf.len() instead)
- Remove redundant sample_count from send_audio (use buf.sample_count())
- Add struct_size field to CNodeCallbacks for forward compatibility
- Extract duplicated CResult-to-Result pattern into result_from_c helper

Naming/exports:
- Rename CAllocResult to CAllocVideoResult for consistency with
  CAllocAudioResult
- Re-export PooledVideoBuffer and PooledAudioBuffer in the prelude

Tests:
- Add regression tests for buffer_handle cleanup on null data/samples
  error paths in packet_from_c

Signed-off-by: StreamKit Devin <devin@streamkit.dev>
Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
staging-devin-ai-integration[bot]

This comment was marked as resolved.

streamkit-devin and others added 2 commits April 5, 2026 07:21
Check free_fn.is_none() alongside data.is_null() before constructing
PooledVideoBuffer/PooledAudioBuffer. A buggy host returning non-null
data with free_fn: None would previously leak via the ? operator
silently returning None without cleanup.

Signed-off-by: StreamKit Devin <devin@streamkit.dev>
Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
- Remove const qualifier from as_mut_slice/consume on PooledVideoBuffer
  and PooledAudioBuffer — these dereference heap pointers and will never
  be called in const context. Add #[allow(clippy::missing_const_for_fn)]
  with rationale comment.
- Downgrade info! to trace! in __plugin_flush macro — these are
  development debugging lines that would spam production logs.

Signed-off-by: StreamKit Devin <devin@streamkit.dev>
Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
@streamer45 streamer45 merged commit 5020a9c into main Apr 5, 2026
17 checks passed
@streamer45 streamer45 deleted the devin/1775334640-native-plugin-frame-pool branch April 5, 2026 07:52
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