Skip to content

Commit 1a6fa04

Browse files
fix(vp9): improve encoder/decoder allocations and add shutdown comments
- Change next_pts duration default from 0 to 1 so libvpx rate-control always sees a non-zero duration (Fix #5). - Add comment about data loss on explicit encoder shutdown (Fix #7). - Use Bytes::copy_from_slice in drain_packets instead of .to_vec() + Bytes::from(), avoiding an intermediate Vec allocation per encoded packet (Fix #9). - Use Vec::with_capacity(1) in decode_packet since most VP9 packets produce exactly one frame, avoiding a heap alloc in the common case (Fix #10). Co-Authored-By: Claudio Costa <cstcld91@gmail.com>
1 parent 4d78b76 commit 1a6fa04

File tree

1 file changed

+23
-8
lines changed

1 file changed

+23
-8
lines changed

crates/nodes/src/video/vp9.rs

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,7 @@ impl ProcessorNode for Vp9EncoderNode {
467467
stats_tracker.received();
468468

469469
let output_packet = Packet::Binary {
470-
data: Bytes::from(encoded.data),
470+
data: encoded.data,
471471
content_type: Some(Cow::Borrowed(VP9_CONTENT_TYPE)),
472472
metadata: encoded.metadata,
473473
};
@@ -491,6 +491,12 @@ impl ProcessorNode for Vp9EncoderNode {
491491
Some(control_msg) = context.control_rx.recv() => {
492492
if matches!(control_msg, streamkit_core::control::NodeControlMessage::Shutdown) {
493493
tracing::info!("Vp9EncoderNode received shutdown signal");
494+
// NOTE: Aborting the input task and dropping encode_tx causes
495+
// the encode thread to flush remaining frames, but because we
496+
// break out of the forwarding loop here those flushed packets
497+
// are never sent downstream. This differs from the graceful
498+
// input-closed path (below) which does drain result_rx. Data
499+
// loss on explicit shutdown is acceptable.
494500
input_task.abort();
495501
drop(encode_tx);
496502
break;
@@ -505,7 +511,7 @@ impl ProcessorNode for Vp9EncoderNode {
505511
stats_tracker.received();
506512

507513
let output_packet = Packet::Binary {
508-
data: Bytes::from(encoded.data),
514+
data: encoded.data,
509515
content_type: Some(Cow::Borrowed(VP9_CONTENT_TYPE)),
510516
metadata: encoded.metadata,
511517
};
@@ -539,7 +545,7 @@ impl ProcessorNode for Vp9EncoderNode {
539545
}
540546

541547
struct EncodedPacket {
542-
data: Vec<u8>,
548+
data: Bytes,
543549
metadata: Option<PacketMetadata>,
544550
}
545551

@@ -598,7 +604,9 @@ impl Vp9Decoder {
598604
};
599605
check_vpx(res, &raw mut self.ctx, "VP9 decode")?;
600606

601-
let mut frames = Vec::new();
607+
// Most VP9 packets produce exactly one frame; pre-allocate for that
608+
// common case to avoid a heap allocation + realloc in the hot path.
609+
let mut frames = Vec::with_capacity(1);
602610
let mut iter: vpx::vpx_codec_iter_t = std::ptr::null_mut();
603611
let mut remaining_metadata = metadata;
604612

@@ -842,11 +850,15 @@ impl Vp9Encoder {
842850
packet.data.frame
843851
};
844852

845-
let data = unsafe {
853+
let data: Bytes = unsafe {
846854
// SAFETY: frame_pkt.buf is valid for frame_pkt.sz bytes.
855+
// Copy into Bytes directly so the downstream Packet::Binary
856+
// doesn't need a second Vec → Bytes conversion.
847857
#[allow(clippy::cast_possible_truncation)]
848-
std::slice::from_raw_parts(frame_pkt.buf as *const u8, frame_pkt.sz as usize)
849-
.to_vec()
858+
Bytes::copy_from_slice(std::slice::from_raw_parts(
859+
frame_pkt.buf as *const u8,
860+
frame_pkt.sz as usize,
861+
))
850862
};
851863

852864
let is_keyframe = (frame_pkt.flags as u32 & VPX_FRAME_IS_KEY) != 0;
@@ -876,7 +888,10 @@ impl Vp9Encoder {
876888
}
877889

878890
fn next_pts(&mut self, metadata: Option<&PacketMetadata>) -> (i64, u64) {
879-
let duration = metadata.and_then(|meta| meta.duration_us).unwrap_or(0);
891+
// Default to 1µs rather than 0 so libvpx rate-control heuristics
892+
// always see a non-zero duration. The PTS advance fallback already
893+
// uses `pts + 1`, so this keeps the two paths consistent.
894+
let duration = metadata.and_then(|meta| meta.duration_us).unwrap_or(1);
880895

881896
let pts =
882897
metadata.and_then(|meta| meta.timestamp_us).map_or(self.next_pts, u64::cast_signed);

0 commit comments

Comments
 (0)