From d200667fc104f1e1da05fc810d08d6d379237eb5 Mon Sep 17 00:00:00 2001 From: Scott Lamb Date: Thu, 15 Aug 2024 21:54:29 -0700 Subject: [PATCH] refine mp4 sample entry builder interface This makes the audio one use a builder too, for symmetry and in case there are parameters we want to customize. And it actually shrinks the size of `AudioParameters` from 80 bytes down to 64. --- Cargo.toml | 3 -- examples/client/Cargo.toml | 2 +- examples/client/src/mp4.rs | 9 ++--- src/codec/aac.rs | 40 +++++++++++++---------- src/codec/g723.rs | 2 +- src/codec/h264.rs | 2 +- src/codec/jpeg.rs | 3 +- src/codec/mod.rs | 67 ++++++++++++++++++++++++-------------- src/codec/simple_audio.rs | 2 +- src/error.rs | 3 ++ 10 files changed, 79 insertions(+), 54 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f87422a..c289a50 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,9 +15,6 @@ repository = "https://github.com/scottlamb/retina" include = ["src/**/*", "benches", "Cargo.toml"] rust-version = "1.67" -[features] -unstable-sample-entry = [] - [package.metadata.docs.rs] # https://docs.rs/about/metadata # To generate docs locally, run: RUSTDOCFLAGS="--cfg docsrs" cargo +nightly doc --all-features diff --git a/examples/client/Cargo.toml b/examples/client/Cargo.toml index 12b66df..8c599c0 100644 --- a/examples/client/Cargo.toml +++ b/examples/client/Cargo.toml @@ -9,7 +9,7 @@ rust-version = "1.64" bytes = "1.0.1" futures = "0.3.14" log = "0.4.8" -retina = { path = "../../", features = ["unstable-sample-entry"] } +retina = { path = "../../" } tokio = { version = "1.5.0", features = ["fs", "io-util", "macros", "rt-multi-thread", "signal"] } url = "2.2.1" anyhow = "1.0.41" diff --git a/examples/client/src/mp4.rs b/examples/client/src/mp4.rs index 2922fb6..ecd6994 100644 --- a/examples/client/src/mp4.rs +++ b/examples/client/src/mp4.rs @@ -382,7 +382,7 @@ impl Mp4Writer { buf.put_u32(0); // version buf.put_u32(u32::try_from(self.video_params.len())?); // entry_count for p in &self.video_params { - let e = p.sample_entry().build().map_err(|e| { + let e = p.mp4_sample_entry().build().map_err(|e| { anyhow!( "unable to produce VisualSampleEntry for {} stream: {}", p.rfc6381_codec(), @@ -473,8 +473,9 @@ impl Mp4Writer { buf.put_u32(0); // version buf.put_u32(1); // entry_count buf.extend_from_slice( - parameters - .sample_entry() + ¶meters + .mp4_sample_entry() + .build() .expect("all added streams have sample entries"), ); }); @@ -742,7 +743,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::ParametersRef::Audio(a)) if a.sample_entry().is_some() => { + Some(retina::codec::ParametersRef::Audio(a)) if a.mp4_sample_entry().build().is_ok() => { log::info!("Using {} audio stream (rfc 6381 codec {})", s.encoding_name(), a.rfc6381_codec().unwrap()); Some((i, Box::new(a.clone()))) } diff --git a/src/codec/aac.rs b/src/codec/aac.rs index 65d5aff..fba6294 100644 --- a/src/codec/aac.rs +++ b/src/codec/aac.rs @@ -19,7 +19,7 @@ use bytes::{BufMut, Bytes, BytesMut}; use std::{ convert::TryFrom, fmt::Debug, - num::{NonZeroU16, NonZeroU32}, + num::{NonZeroU16, NonZeroU32, NonZeroU8}, }; use crate::{error::ErrorInt, rtp::ReceivedPacket, ConnectionContext, Error, StreamContext}; @@ -106,16 +106,15 @@ impl AudioSpecificConfig { .map_err(|e| format!("unable to read sampling_frequency ext: {e}"))?, 0x10..=0xff => unreachable!(), }; - let channels = { - let c = r - .read::(4) - .map_err(|e| format!("unable to read channels: {e}"))?; - CHANNEL_CONFIGS - .get(usize::from(c)) - .ok_or_else(|| format!("reserved channelConfiguration 0x{c:x}"))? - .as_ref() - .ok_or_else(|| "program_config_element parsing unimplemented".to_string())? - }; + let channels_config_id = r + .read::(4) + .map_err(|e| format!("unable to read channels: {e}"))?; + let channels = CHANNEL_CONFIGS + .get(usize::from(channels_config_id)) + .ok_or_else(|| format!("reserved channelConfiguration 0x{channels_config_id:x}"))? + .as_ref() + .ok_or_else(|| "program_config_element parsing unimplemented".to_string())?; + let channels_config_id = NonZeroU8::new(channels_config_id).expect("non-zero"); if audio_object_type == 5 || audio_object_type == 29 { // extensionSamplingFrequencyIndex + extensionSamplingFrequency. if r.read::(4) @@ -166,7 +165,7 @@ impl AudioSpecificConfig { rfc6381_codec, frame_length: Some(NonZeroU32::from(frame_length)), extra_data: raw.to_owned(), - sample_entry: Some(make_sample_entry(channels, sampling_frequency, raw)?), + codec: super::AudioParametersCodec::Aac { channels_config_id }, }, frame_length, channels, @@ -176,11 +175,15 @@ impl AudioSpecificConfig { /// Returns an MP4AudioSampleEntry (`mp4a`) box as in ISO/IEC 14496-14 section 5.6.1. /// `config` should be a raw AudioSpecificConfig. -fn make_sample_entry( - channels: &ChannelConfig, +pub(super) fn make_sample_entry( + channel_config_i: NonZeroU8, sampling_frequency: u32, config: &[u8], -) -> Result, String> { +) -> Result, Error> { + let channels = CHANNEL_CONFIGS + .get(usize::from(channel_config_i.get())) + .and_then(Option::as_ref) + .expect("channel_config_i should be valid"); let mut buf = Vec::new(); // Write an MP4AudioSampleEntry (`mp4a`), as in ISO/IEC 14496-14 section 5.6.1. @@ -204,8 +207,11 @@ fn make_sample_entry( // use a SamplingRateBox. The latter also requires changing the // version/structure of the AudioSampleEntryBox and the version of the // stsd box. Just support the former for now. - let sampling_frequency = u16::try_from(sampling_frequency) - .map_err(|_| format!("aac sampling_frequency={sampling_frequency} unsupported"))?; + let Ok(sampling_frequency) = u16::try_from(sampling_frequency) else { + bail!(ErrorInt::Unsupported(format!( + "aac sampling_frequency={sampling_frequency} requires a SamplingRateBox" + ))); + }; buf.put_u32(u32::from(sampling_frequency) << 16); // Write the embedded ESDBox (`esds`), as in ISO/IEC 14496-14 section 5.6.1. diff --git a/src/codec/g723.rs b/src/codec/g723.rs index 32ec41d..864f811 100644 --- a/src/codec/g723.rs +++ b/src/codec/g723.rs @@ -30,7 +30,7 @@ impl Depacketizer { frame_length: NonZeroU32::new(240), clock_rate: FIXED_CLOCK_RATE, extra_data: Vec::new(), - sample_entry: None, + codec: super::AudioParametersCodec::Other, }, }) } diff --git a/src/codec/h264.rs b/src/codec/h264.rs index f43f76f..483cd62 100644 --- a/src/codec/h264.rs +++ b/src/codec/h264.rs @@ -861,7 +861,7 @@ impl InternalParameters { pixel_aspect_ratio, frame_rate, extra_data: avc_decoder_config, - codec: super::VideoCodec::H264, + codec: super::VideoParametersCodec::H264, }, sps_nal, pps_nal, diff --git a/src/codec/jpeg.rs b/src/codec/jpeg.rs index c8f5363..4a81724 100644 --- a/src/codec/jpeg.rs +++ b/src/codec/jpeg.rs @@ -451,7 +451,7 @@ impl Depacketizer { pixel_aspect_ratio: None, frame_rate: None, extra_data: Bytes::new(), - codec: super::VideoCodec::Jpeg, + codec: super::VideoParametersCodec::Jpeg, }), }); } @@ -528,7 +528,6 @@ impl Depacketizer { /// /// This is actually entirely static, but we construct it at runtime with the /// `write_mp4_box!` and `write_mpeg4_descriptor!` macros for readability. -#[cfg(feature = "unstable-sample-entry")] pub(super) fn append_esds(buf: &mut Vec) { write_mp4_box!(buf, *b"esds", { buf.extend_from_slice(&0u32.to_be_bytes()[..]); // version diff --git a/src/codec/mod.rs b/src/codec/mod.rs index 14b9cf9..1f66ada 100644 --- a/src/codec/mod.rs +++ b/src/codec/mod.rs @@ -9,10 +9,12 @@ #![cfg_attr(docsrs, feature(doc_cfg))] +use std::num::NonZeroU8; use std::num::{NonZeroU16, NonZeroU32}; use bytes::Bytes; +use crate::error::ErrorInt; use crate::rtp::ReceivedPacket; use crate::ConnectionContext; use crate::Error; @@ -159,8 +161,7 @@ pub struct VideoParameters { /// The codec, for internal use in sample entry construction. /// /// This is more straightforward than reparsing the RFC 6381 codec string. - #[cfg_attr(not(feature = "unstable-sample-entry"), allow(unused))] - codec: VideoCodec, + codec: VideoParametersCodec, } impl VideoParameters { @@ -173,9 +174,7 @@ impl VideoParameters { /// Returns a builder for an `.mp4` `VideoSampleEntry` box (as defined in /// ISO/IEC 14496-12). - #[cfg(feature = "unstable-sample-entry")] - #[cfg_attr(docsrs, doc(cfg(feature = "unstable-sample-entry")))] - pub fn sample_entry(&self) -> VideoSampleEntryBuilder { + pub fn mp4_sample_entry(&self) -> VideoSampleEntryBuilder { VideoSampleEntryBuilder { params: self, aspect_ratio_override: None, @@ -238,30 +237,25 @@ impl std::fmt::Debug for VideoParameters { } #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] -enum VideoCodec { +enum VideoParametersCodec { H264, Jpeg, } -impl VideoCodec { - #[cfg(feature = "unstable-sample-entry")] +impl VideoParametersCodec { fn visual_sample_entry_box_type(self) -> [u8; 4] { match self { - VideoCodec::H264 => *b"avc1", - VideoCodec::Jpeg => *b"mp4v", + VideoParametersCodec::H264 => *b"avc1", + VideoParametersCodec::Jpeg => *b"mp4v", } } } -#[cfg(feature = "unstable-sample-entry")] -#[cfg_attr(docsrs, doc(cfg(feature = "unstable-sample-entry")))] pub struct VideoSampleEntryBuilder<'p> { params: &'p VideoParameters, aspect_ratio_override: Option<(u16, u16)>, } -#[cfg(feature = "unstable-sample-entry")] -#[cfg_attr(docsrs, doc(cfg(feature = "unstable-sample-entry")))] impl VideoSampleEntryBuilder<'_> { /// Overrides the codec-level pixel aspect ratio via a `pasp` box. #[inline] @@ -305,12 +299,12 @@ impl VideoSampleEntryBuilder<'_> { // Codec-specific portion. match self.params.codec { - VideoCodec::H264 => { + VideoParametersCodec::H264 => { write_mp4_box!(&mut buf, *b"avcC", { buf.extend_from_slice(&self.params.extra_data); }); } - VideoCodec::Jpeg => { + VideoParametersCodec::Jpeg => { jpeg::append_esds(&mut buf); } } @@ -335,9 +329,7 @@ pub struct AudioParameters { frame_length: Option, clock_rate: u32, extra_data: Vec, - - #[cfg_attr(not(feature = "unstable-sample-entry"), allow(unused))] - sample_entry: Option>, + codec: AudioParametersCodec, } impl std::fmt::Debug for AudioParameters { @@ -373,14 +365,41 @@ impl AudioParameters { &self.extra_data } - /// An `.mp4` `AudioSampleEntry` box (as defined in ISO/IEC 14496-12), if possible. + /// Returns a builder for an `.mp4` `AudioSampleEntry` box (as defined in ISO/IEC 14496-12). + pub fn mp4_sample_entry(&self) -> AudioSampleEntryBuilder { + AudioSampleEntryBuilder { params: self } + } +} + +/// Holds codec-specific data needed from the `AudioParameters`. +#[derive(Clone, PartialEq, Eq, Hash)] +enum AudioParametersCodec { + Aac { channels_config_id: NonZeroU8 }, + Other, +} + +pub struct AudioSampleEntryBuilder<'p> { + params: &'p AudioParameters, +} + +impl AudioSampleEntryBuilder<'_> { + /// Builds the `.mp4` `AudioSampleEntry` box, if possible. /// /// Not all codecs can be placed into a `.mp4` file, and even for supported codecs there /// may be unsupported edge cases. - #[cfg(feature = "unstable-sample-entry")] - #[cfg_attr(docsrs, doc(cfg(feature = "unstable-sample-entry")))] - pub fn sample_entry(&self) -> Option<&[u8]> { - self.sample_entry.as_deref() + pub fn build(self) -> Result, Error> { + match self.params.codec { + AudioParametersCodec::Aac { channels_config_id } => aac::make_sample_entry( + channels_config_id, + self.params.clock_rate, + &self.params.extra_data, + ), + AudioParametersCodec::Other => { + bail!(ErrorInt::Unsupported( + "unsupported audio codec for mp4".to_owned() + )); + } + } } } diff --git a/src/codec/simple_audio.rs b/src/codec/simple_audio.rs index 21a8670..fe6a5b9 100644 --- a/src/codec/simple_audio.rs +++ b/src/codec/simple_audio.rs @@ -24,7 +24,7 @@ impl Depacketizer { frame_length: None, // variable clock_rate, extra_data: Vec::new(), - sample_entry: None, + codec: super::AudioParametersCodec::Other, }, bits_per_sample, pending: None, diff --git a/src/error.rs b/src/error.rs index f503d40..c2e263a 100644 --- a/src/error.rs +++ b/src/error.rs @@ -139,4 +139,7 @@ pub(crate) enum ErrorInt { #[error("Timeout")] Timeout, + + #[error("Unsupported: {0}")] + Unsupported(String), }