From 1066c4aa654bb7687bd4347f580c64bcc026122a Mon Sep 17 00:00:00 2001 From: Ramyak Mehra Date: Sun, 13 Oct 2024 17:24:57 +0530 Subject: [PATCH 1/3] feat: add marker bit when packetizing opus. incase of silence opus, rtp payload sets the marker bit after a talking spurt. usually the data in silence packets are empty, it just uses 1-2 bytes for header. introduced a marker state inside the OpusPacketizer since we don't have access to previous packet incase of audio. fixes #125 --- src/packet/mod.rs | 2 +- src/packet/opus.rs | 26 ++++- .../{rtp_to_frame.rs => talk_start_spurt.rs} | 104 +++++++++++++++++- 3 files changed, 125 insertions(+), 7 deletions(-) rename tests/{rtp_to_frame.rs => talk_start_spurt.rs} (53%) diff --git a/src/packet/mod.rs b/src/packet/mod.rs index 768fbca6..a650f6ce 100644 --- a/src/packet/mod.rs +++ b/src/packet/mod.rs @@ -209,7 +209,7 @@ pub(crate) enum CodecDepacketizer { impl From for CodecPacketizer { fn from(c: Codec) -> Self { match c { - Codec::Opus => CodecPacketizer::Opus(OpusPacketizer), + Codec::Opus => CodecPacketizer::Opus(OpusPacketizer::default()), Codec::H264 => CodecPacketizer::H264(H264Packetizer::default()), Codec::H265 => unimplemented!("Missing packetizer for H265"), Codec::Vp8 => CodecPacketizer::Vp8(Vp8Packetizer::default()), diff --git a/src/packet/opus.rs b/src/packet/opus.rs index 365da501..d6628e6a 100644 --- a/src/packet/opus.rs +++ b/src/packet/opus.rs @@ -2,7 +2,10 @@ use super::{CodecExtra, Depacketizer, MediaKind, PacketError, Packetizer}; /// Packetizes Opus RTP packets. #[derive(Default, Debug, Copy, Clone)] -pub struct OpusPacketizer; +pub struct OpusPacketizer { + // stores if a marker was previously set + marker: bool, +} impl Packetizer for OpusPacketizer { fn packetize(&mut self, mtu: usize, payload: &[u8]) -> Result>, PacketError> { @@ -23,8 +26,23 @@ impl Packetizer for OpusPacketizer { } fn is_marker(&mut self, data: &[u8], previous: Option<&[u8]>, last: bool) -> bool { - // TODO: dtx - false + // any non silenced packet would generally have more than 2 byts + let mut is_marker = data.len() > 2; + + match self.marker { + true => { + if !is_marker { + self.marker = false; + } + is_marker = false; + } + false => { + if is_marker { + self.marker = true; + } + } + } + is_marker } } @@ -84,7 +102,7 @@ mod test { #[test] fn test_opus_payload() -> Result<(), PacketError> { - let mut pck = OpusPacketizer; + let mut pck = OpusPacketizer::default(); let empty = &[]; let payload = &[0x90, 0x90, 0x90]; diff --git a/tests/rtp_to_frame.rs b/tests/talk_start_spurt.rs similarity index 53% rename from tests/rtp_to_frame.rs rename to tests/talk_start_spurt.rs index 62f3af14..0c52b1e8 100644 --- a/tests/rtp_to_frame.rs +++ b/tests/talk_start_spurt.rs @@ -2,7 +2,7 @@ use std::collections::VecDeque; use std::time::Duration; use str0m::format::Codec; -use str0m::media::MediaKind; +use str0m::media::{Frequency, MediaKind, MediaTime}; use str0m::rtp::{ExtensionValues, Ssrc}; use str0m::{Event, Rtc, RtcError}; @@ -10,7 +10,7 @@ mod common; use common::{connect_l_r_with_rtc, init_log, progress}; #[test] -pub fn audio_start_of_talk_spurt() -> Result<(), RtcError> { +pub fn audio_start_of_talk_spurt_frame() -> Result<(), RtcError> { init_log(); let rtc1 = Rtc::builder().set_rtp_mode(true).build(); @@ -108,3 +108,103 @@ pub fn audio_start_of_talk_spurt() -> Result<(), RtcError> { Ok(()) } + +#[test] +pub fn audio_start_of_talk_spurt_rtp() -> Result<(), RtcError> { + init_log(); + + let rtc1 = Rtc::builder().build(); + let rtc2 = Rtc::builder() + .set_reordering_size_audio(0) + .set_rtp_mode(true) + .build(); + + let (mut l, mut r) = connect_l_r_with_rtc(rtc1, rtc2); + + let mid = "audio".into(); + let ssrc_tx: Ssrc = 1337.into(); + + l.direct_api().declare_media(mid, MediaKind::Audio); + l.direct_api().declare_stream_tx(ssrc_tx, None, mid, None); + r.direct_api().declare_media(mid, MediaKind::Audio); + + let max = l.last.max(r.last); + l.last = max; + r.last = max; + + let params = l.params_opus(); + + assert_eq!(params.spec().codec, Codec::Opus); + let pt = params.pt(); + + let to_write: Vec<&[u8]> = vec![ + // 1 + &[0x1], + // 2 + &[0x1, 0x2, 0x3, 0x4], + // 4 + &[0x9, 0xa, 0xb, 0xc], + // 3 + &[0x5, 0x6, 0x7, 0x8], + // 5 + &[0x1], + // 6 + &[0x9, 0xa, 0xb, 0xc], + ]; + + let mut to_write: VecDeque<_> = to_write.into(); + + let mut write_at = l.last + Duration::from_millis(300); + + let mut counts: Vec = vec![0, 1, 2, 4, 3, 5, 6]; + + loop { + if l.start + l.duration() > write_at { + write_at = l.last + Duration::from_millis(300); + if let Some(packet) = to_write.pop_front() { + let wallclock = l.start + l.duration(); + + let count = counts.remove(0); + let time = count * 1000 + 47_000_000; + + l.writer(mid) + .unwrap() + .write( + pt, + wallclock, + MediaTime::new(time, Frequency::FORTY_EIGHT_KHZ), + packet.to_vec(), + ) + .unwrap(); + } + } + + progress(&mut l, &mut r)?; + + if l.duration() > Duration::from_secs(10) { + break; + } + } + + let rtp_packets: Vec<_> = r + .events + .iter() + .filter_map(|(_, e)| { + if let Event::RtpPacket(p) = e { + Some(p) + } else { + None + } + }) + .collect(); + + assert_eq!(rtp_packets.len(), 6); + let is_marker = [false, true, false, false, false, true]; + + rtp_packets + .iter() + .enumerate() + .for_each(|(i, r)| assert_eq!(r.header.marker, is_marker[i])); + + Ok(()) +} From 6addb0d3e4a4277687854a7a44a0e13c4b39e090 Mon Sep 17 00:00:00 2001 From: Ramyak Mehra Date: Mon, 14 Oct 2024 21:15:39 +0530 Subject: [PATCH 2/3] feat: pass use_dtx flag to opus packetizer. use_dtx is negotiated with the sdp, we can use tha in the opus packetizer while calculating marker bit --- src/packet/mod.rs | 12 ++++++---- src/packet/opus.rs | 54 +++++++++++++++++++++++++++++++++++++++++-- src/packet/payload.rs | 2 +- 3 files changed, 60 insertions(+), 8 deletions(-) diff --git a/src/packet/mod.rs b/src/packet/mod.rs index a650f6ce..b34a88e1 100644 --- a/src/packet/mod.rs +++ b/src/packet/mod.rs @@ -5,7 +5,7 @@ use std::fmt; use std::panic::UnwindSafe; use thiserror::Error; -use crate::format::Codec; +use crate::format::{Codec, CodecSpec}; use crate::sdp::MediaType; mod g7xx; @@ -206,10 +206,12 @@ pub(crate) enum CodecDepacketizer { Boxed(Box), } -impl From for CodecPacketizer { - fn from(c: Codec) -> Self { - match c { - Codec::Opus => CodecPacketizer::Opus(OpusPacketizer::default()), +impl From for CodecPacketizer { + fn from(c: CodecSpec) -> Self { + match c.codec { + Codec::Opus => { + CodecPacketizer::Opus(OpusPacketizer::new(c.format.use_dtx.unwrap_or_default())) + } Codec::H264 => CodecPacketizer::H264(H264Packetizer::default()), Codec::H265 => unimplemented!("Missing packetizer for H265"), Codec::Vp8 => CodecPacketizer::Vp8(Vp8Packetizer::default()), diff --git a/src/packet/opus.rs b/src/packet/opus.rs index d6628e6a..df108db6 100644 --- a/src/packet/opus.rs +++ b/src/packet/opus.rs @@ -1,10 +1,20 @@ use super::{CodecExtra, Depacketizer, MediaKind, PacketError, Packetizer}; /// Packetizes Opus RTP packets. -#[derive(Default, Debug, Copy, Clone)] +#[derive(Debug, Copy, Clone)] pub struct OpusPacketizer { // stores if a marker was previously set marker: bool, + use_dtx: bool, +} + +impl OpusPacketizer { + pub fn new(use_dtx: bool) -> Self { + Self { + marker: false, + use_dtx, + } + } } impl Packetizer for OpusPacketizer { @@ -26,6 +36,9 @@ impl Packetizer for OpusPacketizer { } fn is_marker(&mut self, data: &[u8], previous: Option<&[u8]>, last: bool) -> bool { + if !self.use_dtx { + return false; + } // any non silenced packet would generally have more than 2 byts let mut is_marker = data.len() > 2; @@ -102,7 +115,7 @@ mod test { #[test] fn test_opus_payload() -> Result<(), PacketError> { - let mut pck = OpusPacketizer::default(); + let mut pck = OpusPacketizer::new(true); let empty = &[]; let payload = &[0x90, 0x90, 0x90]; @@ -121,6 +134,43 @@ mod test { Ok(()) } + #[test] + fn test_opus_packetizer_dtx() -> Result<(), PacketError> { + // packetizer with dtx on + let mut pck = OpusPacketizer::new(true); + + let payload = &[0x90, 0x90, 0x90]; + + // Start of talking spurt, marker bit is set. + let is_marker = pck.is_marker(payload, None, false); + assert!(is_marker); + assert!(pck.marker); + + // More talking so is_marker should be false. + let is_marker = pck.is_marker(payload, None, false); + assert!(!is_marker); + assert!(pck.marker); + + // silence packet inserted, internal packetizer state should be reset. + let is_marker = pck.is_marker(&[], None, false); + assert!(!is_marker); + assert!(!pck.marker); + + // talking start again, marker bit is set. + let is_marker = pck.is_marker(payload, None, false); + assert!(is_marker); + assert!(pck.marker); + + let mut pck = OpusPacketizer::new(false); + + // Start of talking spurt, since dtx is false marker should be false + let is_marker = pck.is_marker(payload, None, false); + assert!(!is_marker); + assert!(!pck.marker); + + Ok(()) + } + #[test] fn test_opus_is_partition_head() -> Result<(), PacketError> { let opus = OpusDepacketizer; diff --git a/src/packet/payload.rs b/src/packet/payload.rs index c9244d0b..565139f6 100644 --- a/src/packet/payload.rs +++ b/src/packet/payload.rs @@ -21,7 +21,7 @@ pub struct Payloader { impl Payloader { pub(crate) fn new(spec: CodecSpec) -> Self { Payloader { - pack: spec.codec.into(), + pack: spec.into(), clock_rate: spec.clock_rate, } } From 37c900f307b142509d1e64bb932b4cc4c3ae8703 Mon Sep 17 00:00:00 2001 From: Ramyak Mehra Date: Mon, 14 Oct 2024 23:24:47 +0530 Subject: [PATCH 3/3] fix: pass use_dtx in format params for talk_start_spurt test --- tests/talk_start_spurt.rs | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/tests/talk_start_spurt.rs b/tests/talk_start_spurt.rs index 0c52b1e8..012d7a01 100644 --- a/tests/talk_start_spurt.rs +++ b/tests/talk_start_spurt.rs @@ -1,7 +1,7 @@ use std::collections::VecDeque; use std::time::Duration; -use str0m::format::Codec; +use str0m::format::{Codec, FormatParams}; use str0m::media::{Frequency, MediaKind, MediaTime}; use str0m::rtp::{ExtensionValues, Ssrc}; use str0m::{Event, Rtc, RtcError}; @@ -113,7 +113,26 @@ pub fn audio_start_of_talk_spurt_frame() -> Result<(), RtcError> { pub fn audio_start_of_talk_spurt_rtp() -> Result<(), RtcError> { init_log(); - let rtc1 = Rtc::builder().build(); + let mut rtc1_config = Rtc::builder().clear_codecs(); + let codec_config = rtc1_config.codec_config(); + + // add a custom opus config with dtx flag set to true + codec_config.add_config( + 111.into(), + None, + Codec::Opus, + Frequency::FORTY_EIGHT_KHZ, + Some(2), + FormatParams { + min_p_time: Some(10), + use_inband_fec: Some(true), + use_dtx: Some(true), + ..Default::default() + }, + ); + + let rtc1 = rtc1_config.build(); + let rtc2 = Rtc::builder() .set_reordering_size_audio(0) .set_rtp_mode(true)