Skip to content

Commit

Permalink
codec::Parameters -> codec::ParametersRef
Browse files Browse the repository at this point in the history 
This reduces copying. Part of #47.
  • Loading branch information
@scottlamb
scottlamb committed May 11, 2022
1 parent a36b01b commit b1e1fa9
Show file tree
Hide file tree
Showing 11 changed files with 80 additions and 67 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
* BREAKING: `retina::client::PacketItem::SenderReport` has been replaced with
`retina::client::PacketItem::Rtcp`, to expose full RTCP compound packets.
Likewise `retina::codec::CodecItem`.
* BREAKING: `retina::codec::Parameters` is now `retina::codec::ParametersRef`,
which references parameters stored within the `Stream` to reduce copying.
* minimum Rust version is now 1.59.

## `v0.3.10` (2022-05-09)
Expand Down
12 changes: 6 additions & 6 deletions examples/client/mp4.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use futures::{Future, StreamExt};
use log::{debug, info, warn};
use retina::{
client::{SetupOptions, Transport},
codec::{AudioParameters, CodecItem, Parameters, VideoParameters},
codec::{AudioParameters, CodecItem, ParametersRef, VideoParameters},
};

use std::num::NonZeroU32;
Expand Down Expand Up @@ -98,7 +98,7 @@ macro_rules! write_box {
pub struct Mp4Writer<W: AsyncWrite + AsyncSeek + Send + Unpin> {
mdat_start: u32,
mdat_pos: u32,
video_params: Vec<Box<VideoParameters>>,
video_params: Vec<VideoParameters>,

/// The most recently used 1-based index within `video_params`.
cur_video_params_sample_description_index: Option<u32>,
Expand Down Expand Up @@ -548,13 +548,13 @@ impl<W: AsyncWrite + AsyncSeek + Send + Unpin> Mp4Writer<W> {
i
} else {
match stream.parameters() {
Some(Parameters::Video(params)) => {
Some(ParametersRef::Video(params)) => {
log::info!("new video params: {:?}", params);
let pos = self.video_params.iter().position(|p| **p == params);
let pos = self.video_params.iter().position(|p| p == params);
if let Some(pos) = pos {
u32::try_from(pos + 1)?
} else {
self.video_params.push(Box::new(params));
self.video_params.push(params.clone());
u32::try_from(self.video_params.len())?
}
}
Expand Down Expand Up @@ -767,7 +767,7 @@ pub async fn run(opts: Opts) -> Result<(), Error> {
.find_map(|(i, s)| match s.parameters() {
// Only consider audio streams that can produce a .mp4 sample
// entry.
Some(retina::codec::Parameters::Audio(a)) if a.sample_entry().is_some() => {
Some(retina::codec::ParametersRef::Audio(a)) if a.sample_entry().is_some() => {
log::info!("Using {} audio stream (rfc 6381 codec {})", s.encoding_name(), a.rfc6381_codec().unwrap());
Some((i, Box::new(a.clone())))
}
Expand Down
7 changes: 6 additions & 1 deletion examples/client/onvif.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,12 @@ async fn run_inner(opts: Opts, session_group: Arc<SessionGroup>) -> Result<(), E
let onvif_stream_i = session
.streams()
.iter()
.position(|s| matches!(s.parameters(), Some(retina::codec::Parameters::Message(..))))
.position(|s| {
matches!(
s.parameters(),
Some(retina::codec::ParametersRef::Message(..))
)
})
.ok_or_else(|| anyhow!("couldn't find onvif stream"))?;
session
.setup(onvif_stream_i, SetupOptions::default())
Expand Down
2 changes: 1 addition & 1 deletion src/client/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -814,7 +814,7 @@ impl Stream {
/// When directly using [`Session`]'s packet-by-packet `futures::Stream` impl, codec
/// depacketization logic is bypassed. The parameters returned by this function may be out of
/// date.
pub fn parameters(&self) -> Option<crate::codec::Parameters> {
pub fn parameters(&self) -> Option<crate::codec::ParametersRef> {
self.depacketizer.as_ref().ok().and_then(|d| d.parameters())
}

Expand Down
36 changes: 18 additions & 18 deletions src/client/parse.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,7 @@ mod tests {
use bytes::Bytes;
use url::Url;

use crate::{client::StreamStateInit, codec::Parameters};
use crate::{client::StreamStateInit, codec::ParametersRef};
use crate::{StreamContext, StreamContextInner, TcpStreamContext};

use super::super::StreamState;
Expand Down Expand Up @@ -744,7 +744,7 @@ mod tests {
assert_eq!(p.streams[0].rtp_payload_type, 96);
assert_eq!(p.streams[0].clock_rate_hz, 90_000);
match p.streams[0].parameters().unwrap() {
Parameters::Video(v) => {
ParametersRef::Video(v) => {
assert_eq!(v.rfc6381_codec(), "avc1.64001E");
assert_eq!(v.pixel_dimensions(), (704, 480));
assert_eq!(v.pixel_aspect_ratio(), None);
Expand All @@ -763,7 +763,7 @@ mod tests {
assert_eq!(p.streams[1].rtp_payload_type, 97);
assert_eq!(p.streams[1].clock_rate_hz, 48_000);
match p.streams[1].parameters() {
Some(Parameters::Audio(_)) => {}
Some(ParametersRef::Audio(_)) => {}
_ => panic!(),
}

Expand All @@ -778,7 +778,7 @@ mod tests {
assert_eq!(p.streams[2].clock_rate_hz, 90_000);
assert!(matches!(
p.streams[2].parameters(),
Some(Parameters::Message(_))
Some(ParametersRef::Message(_))
));

// SETUP.
Expand Down Expand Up @@ -825,7 +825,7 @@ mod tests {
assert_eq!(p.streams[1].encoding_name(), "pcma");
assert_eq!(p.streams[1].rtp_payload_type, 8);
match p.streams[1].parameters().unwrap() {
Parameters::Audio(_) => {}
ParametersRef::Audio(_) => {}
_ => panic!(),
};
}
Expand Down Expand Up @@ -856,7 +856,7 @@ mod tests {
assert_eq!(p.streams[0].rtp_payload_type, 96);
assert_eq!(p.streams[0].clock_rate_hz, 90_000);
match p.streams[0].parameters().unwrap() {
Parameters::Video(v) => {
ParametersRef::Video(v) => {
assert_eq!(v.rfc6381_codec(), "avc1.4D0029");
assert_eq!(v.pixel_dimensions(), (1920, 1080));
assert_eq!(v.pixel_aspect_ratio(), None);
Expand All @@ -876,7 +876,7 @@ mod tests {
assert_eq!(p.streams[1].clock_rate_hz, 90_000);
assert!(matches!(
p.streams[1].parameters(),
Some(Parameters::Message(_))
Some(ParametersRef::Message(_))
));

// SETUP.
Expand Down Expand Up @@ -936,7 +936,7 @@ mod tests {
assert_eq!(p.streams[0].rtp_payload_type, 96);
assert_eq!(p.streams[0].clock_rate_hz, 90_000);
match p.streams[0].parameters().unwrap() {
Parameters::Video(v) => {
ParametersRef::Video(v) => {
assert_eq!(v.rfc6381_codec(), "avc1.640033");
assert_eq!(v.pixel_dimensions(), (2560, 1440));
assert_eq!(v.pixel_aspect_ratio(), None);
Expand All @@ -955,7 +955,7 @@ mod tests {
assert_eq!(p.streams[1].rtp_payload_type, 97);
assert_eq!(p.streams[1].clock_rate_hz, 16_000);
match p.streams[1].parameters() {
Some(Parameters::Audio(_)) => {}
Some(ParametersRef::Audio(_)) => {}
_ => panic!(),
}

Expand Down Expand Up @@ -1019,7 +1019,7 @@ mod tests {
assert_eq!(p.streams[0].clock_rate_hz, 12_000);
assert_eq!(p.streams[0].channels, NonZeroU16::new(2));
match p.streams[0].parameters() {
Some(Parameters::Audio(_)) => {}
Some(ParametersRef::Audio(_)) => {}
_ => panic!(),
}

Expand All @@ -1033,7 +1033,7 @@ mod tests {
assert_eq!(p.streams[1].rtp_payload_type, 97);
assert_eq!(p.streams[1].clock_rate_hz, 90_000);
match p.streams[1].parameters().unwrap() {
Parameters::Video(v) => {
ParametersRef::Video(v) => {
assert_eq!(v.rfc6381_codec(), "avc1.42C01E");
assert_eq!(v.pixel_dimensions(), (240, 160));
assert_eq!(v.pixel_aspect_ratio(), None);
Expand Down Expand Up @@ -1118,7 +1118,7 @@ mod tests {
assert_eq!(p.streams[0].rtp_payload_type, 96);
assert_eq!(p.streams[0].clock_rate_hz, 90_000);
match p.streams[0].parameters().unwrap() {
Parameters::Video(v) => {
ParametersRef::Video(v) => {
assert_eq!(v.rfc6381_codec(), "avc1.4D001F");
assert_eq!(v.pixel_dimensions(), (1280, 720));
assert_eq!(v.pixel_aspect_ratio(), None);
Expand All @@ -1138,7 +1138,7 @@ mod tests {
assert_eq!(p.streams[1].clock_rate_hz, 8_000);
assert_eq!(p.streams[1].channels, NonZeroU16::new(1));
match p.streams[1].parameters().unwrap() {
Parameters::Audio(_) => {}
ParametersRef::Audio(_) => {}
_ => panic!(),
};
}
Expand All @@ -1165,7 +1165,7 @@ mod tests {
assert_eq!(p.streams[0].clock_rate_hz, 90_000);

match p.streams[0].parameters().unwrap() {
Parameters::Video(v) => {
ParametersRef::Video(v) => {
assert_eq!(v.rfc6381_codec(), "avc1.4D002A");
assert_eq!(v.pixel_dimensions(), (1920, 1080));
assert_eq!(v.pixel_aspect_ratio(), None);
Expand All @@ -1185,7 +1185,7 @@ mod tests {
assert_eq!(p.streams[1].clock_rate_hz, 8_000);
assert_eq!(p.streams[1].channels, NonZeroU16::new(1));
match p.streams[1].parameters().unwrap() {
Parameters::Audio(_) => {}
ParametersRef::Audio(_) => {}
_ => panic!(),
};
}
Expand All @@ -1211,7 +1211,7 @@ mod tests {
assert_eq!(p.streams[0].rtp_payload_type, 96);
assert_eq!(p.streams[0].clock_rate_hz, 90_000);
match p.streams[0].parameters().unwrap() {
Parameters::Video(v) => {
ParametersRef::Video(v) => {
assert_eq!(v.rfc6381_codec(), "avc1.4D002A");
assert_eq!(v.pixel_dimensions(), (1920, 1080));
assert_eq!(v.pixel_aspect_ratio(), None);
Expand All @@ -1231,7 +1231,7 @@ mod tests {
assert_eq!(p.streams[1].clock_rate_hz, 8_000);
assert_eq!(p.streams[1].channels, NonZeroU16::new(1));
match p.streams[1].parameters().unwrap() {
Parameters::Audio(_) => {}
ParametersRef::Audio(_) => {}
_ => panic!(),
};

Expand Down Expand Up @@ -1308,7 +1308,7 @@ mod tests {
assert_eq!(p.streams[0].rtp_payload_type, 96);
assert_eq!(p.streams[0].clock_rate_hz, 90_000);
match p.streams[0].parameters().unwrap() {
Parameters::Video(v) => {
ParametersRef::Video(v) => {
assert_eq!(v.rfc6381_codec(), "avc1.4D001E");
assert_eq!(v.pixel_dimensions(), (720, 480));
assert_eq!(v.pixel_aspect_ratio(), None);
Expand Down
9 changes: 6 additions & 3 deletions src/codec/aac.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use std::{

use crate::{error::ErrorInt, rtp::ReceivedPacket, ConnectionContext, Error, StreamContext};

use super::CodecItem;
use super::{AudioParameters, CodecItem};

/// An AudioSpecificConfig as in ISO/IEC 14496-3 section 1.6.2.1.
///
Expand Down Expand Up @@ -432,6 +432,7 @@ fn parse_format_specific_params(
#[derive(Debug)]
pub(crate) struct Depacketizer {
config: AudioSpecificConfig,
parameters: AudioParameters,
state: DepacketizerState,
}

Expand Down Expand Up @@ -527,14 +528,16 @@ impl Depacketizer {
channels, config.channels
));
}
let parameters = config.to_parameters();
Ok(Self {
config,
parameters,
state: DepacketizerState::default(),
})
}

pub(super) fn parameters(&self) -> Option<super::Parameters> {
Some(super::Parameters::Audio(self.config.to_parameters()))
pub(super) fn parameters(&self) -> Option<super::ParametersRef> {
Some(super::ParametersRef::Audio(&self.parameters))
}

pub(super) fn push(&mut self, pkt: ReceivedPacket) -> Result<(), String> {
Expand Down
24 changes: 15 additions & 9 deletions src/codec/g723.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,14 @@ use std::num::NonZeroU32;

use bytes::Bytes;

use super::AudioParameters;

const FIXED_CLOCK_RATE: u32 = 8_000;

#[derive(Debug)]
pub(crate) struct Depacketizer {
pending: Option<super::AudioFrame>,
parameters: AudioParameters,
}

impl Depacketizer {
Expand All @@ -23,17 +26,20 @@ impl Depacketizer {
FIXED_CLOCK_RATE, clock_rate
));
}
Ok(Self { pending: None })
Ok(Self {
pending: None,
parameters: AudioParameters {
rfc6381_codec: None,
frame_length: NonZeroU32::new(240),
clock_rate: FIXED_CLOCK_RATE,
extra_data: Bytes::new(),
sample_entry: None,
},
})
}

pub(super) fn parameters(&self) -> Option<super::Parameters> {
Some(super::Parameters::Audio(super::AudioParameters {
rfc6381_codec: None,
frame_length: NonZeroU32::new(240),
clock_rate: FIXED_CLOCK_RATE,
extra_data: Bytes::new(),
sample_entry: None,
}))
pub(super) fn parameters(&self) -> Option<super::ParametersRef> {
Some(super::ParametersRef::Audio(&self.parameters))
}

fn validate(pkt: &crate::rtp::ReceivedPacket) -> bool {
Expand Down
8 changes: 4 additions & 4 deletions src/codec/h264.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,10 @@ impl Depacketizer {
})
}

pub(super) fn parameters(&self) -> Option<super::Parameters> {
pub(super) fn parameters(&self) -> Option<super::ParametersRef> {
self.parameters
.as_ref()
.map(|p| super::Parameters::Video(p.generic_parameters.clone()))
.map(|p| super::ParametersRef::Video(&p.generic_parameters))
}

pub(super) fn push(&mut self, pkt: ReceivedPacket) -> Result<(), String> {
Expand Down Expand Up @@ -1360,7 +1360,7 @@ mod tests {
fn depacketize_parameter_change() {
let mut d = super::Depacketizer::new(90_000, Some("a=fmtp:96 profile-level-id=420029; packetization-mode=1; sprop-parameter-sets=Z01AHppkBYHv/lBgYGQAAA+gAAE4gBA=,aO48gA==")).unwrap();
match d.parameters() {
Some(crate::codec::Parameters::Video(v)) => {
Some(crate::codec::ParametersRef::Video(v)) => {
assert_eq!(v.pixel_dimensions(), (704, 480));
}
_ => unreachable!(),
Expand Down Expand Up @@ -1426,7 +1426,7 @@ mod tests {
// After pull, new_parameters and parameters() both reflect the change.
assert!(frame.has_new_parameters);
match d.parameters() {
Some(crate::codec::Parameters::Video(v)) => {
Some(crate::codec::ParametersRef::Video(v)) => {
assert_eq!(v.pixel_dimensions(), (640, 480));
}
_ => unreachable!(),
Expand Down
13 changes: 6 additions & 7 deletions src/codec/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub enum CodecItem {
Rtcp(crate::rtcp::ReceivedCompoundPacket),
}

/// Parameters which describe a stream.
/// Reference to parameters which describe a stream.
///
/// Parameters are often, but not always, available immediately
/// after `DESCRIBE` via [`crate::client::Stream::parameters`]. They should
Expand All @@ -47,10 +47,10 @@ pub enum CodecItem {
///
/// Currently audio and message streams' parameters never change mid-stream.
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
pub enum Parameters {
Video(VideoParameters),
Audio(AudioParameters),
Message(MessageParameters),
pub enum ParametersRef<'a> {
Video(&'a VideoParameters),
Audio(&'a AudioParameters),
Message(&'a MessageParameters),
}

/// Parameters which describe a video stream.
Expand Down Expand Up @@ -514,7 +514,7 @@ impl Depacketizer {
/// If the caller has called `push` more recently than `pull`, it's currently undefined
/// whether the depacketizer returns parameters as of the most recently pulled or the upcoming
/// frame.
pub fn parameters(&self) -> Option<Parameters> {
pub fn parameters(&self) -> Option<ParametersRef> {
match &self.0 {
DepacketizerInner::Aac(d) => d.parameters(),
DepacketizerInner::G723(d) => d.parameters(),
Expand Down Expand Up @@ -595,7 +595,6 @@ mod tests {
"SenderReport",
std::mem::size_of::<crate::client::rtp::SenderReport>(),
),
("Parameters", std::mem::size_of::<Parameters>()),
("VideoParameters", std::mem::size_of::<VideoParameters>()),
("AudioParameters", std::mem::size_of::<AudioParameters>()),
(
Expand Down
Loading

0 comments on commit b1e1fa9

Please sign in to comment.