Conversation
lherman-cs
commented
Oct 3, 2025
lherman-cs
commented
- remove jitter buffer to trade-off throughput for latency
- allow access to rtp packets
There was a problem hiding this comment.
Pull Request Overview
Switch the media pipeline to low-latency RTP mode by forwarding raw RTP packets instead of decoded MediaData, removing the jitter buffer and enabling direct RTP access.
- Replace MediaData forwarding with RtpPacket across track and participant actors
- Use str0m direct API for RX/TX of RTP and keyframe requests; enable RTP mode in controller
- Minor logging updates and refactors to slot selection using RTP extension values
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pulsebeam/src/track.rs | Switch TrackDataMessage to carry RtpPacket and filter by RID from RTP header extensions before forwarding. |
| pulsebeam/src/participant/core.rs | Update get_slot to consume ExtensionValues (e.g., audio level, VAD) instead of MediaData. |
| pulsebeam/src/participant/actor.rs | Replace MediaData handling with RtpPacket handling, forward via write_rtp, add RTP-based keyframe request path, and change participant message variants. |
| pulsebeam/src/controller.rs | Enable RTP mode on RtcConfig to receive Event::RtpPacket. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| let pt = { | ||
| let Some(media) = self.ctx.rtc.media(mid) else { | ||
| tracing::warn!("no media found, dropping"); | ||
| return; | ||
| }; | ||
|
|
||
| let Some(pt) = media.remote_pts().first() else { | ||
| // TODO: better logic than matching first pt | ||
| tracing::warn!("no remote pt found, dropping"); | ||
| return; | ||
| }; | ||
| *pt |
There was a problem hiding this comment.
Selecting the first negotiated payload type is fragile and can misroute codecs when multiple PTs/codecs are offered (e.g., H.264 + VP8 or multi-PT Opus variants). Maintain an explicit mapping from incoming codec/PT to the sender’s internal PT (e.g., keyed by mid/rid or codec params) and use that mapping here instead of .first().
|
|
||
| let Some(pt) = writer.match_params(data.params) else { | ||
| let mut api = self.ctx.rtc.direct_api(); | ||
| let Some(writer) = api.stream_tx_by_mid(mid, None) else { |
There was a problem hiding this comment.
In simulcast scenarios, using None for the RID may select the wrong outgoing stream. Pass the incoming RID so the correct tx stream is used.
| let Some(writer) = api.stream_tx_by_mid(mid, None) else { | |
| // Extract the RID from the RTP header extensions, if present | |
| let rid = rtp.header.ext_vals.rid().map(|s| s.to_owned()); | |
| let Some(writer) = api.stream_tx_by_mid(mid, rid.as_deref()) else { |
| fn request_keyframe_internal_media(&mut self, req: KeyframeRequest) { | ||
| let Some(mut writer) = self.ctx.rtc.writer(req.mid) else { | ||
| tracing::warn!("No writer for mid {:?}", req.mid); | ||
| tracing::warn!("no writer for mid {:?}", req.mid); | ||
| return; | ||
| }; | ||
|
|
||
| if let Err(e) = writer.request_keyframe(req.rid, req.kind) { | ||
| tracing::warn!("Failed to request keyframe: {}", e); | ||
| tracing::warn!("failed to request keyframe: {}", e); | ||
| } | ||
| } |
There was a problem hiding this comment.
This function is no longer used after switching to RTP mode (the call path now goes through request_keyframe_internal_rtp). Consider removing it or annotate with #[allow(dead_code)] if you plan to toggle modes.
1 participant
