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

@@ -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

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"

examples/client/src/mp4.rs

@@ -382,7 +382,7 @@ impl<W: AsyncWrite + AsyncSeek + Send + Unpin> Mp4Writer<W> {
                             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<W: AsyncWrite + AsyncSeek + Send + Unpin> Mp4Writer<W> {
                             buf.put_u32(0); // version
                             buf.put_u32(1); // entry_count
                             buf.extend_from_slice(
-                                parameters
-                                    .sample_entry()
+                                &parameters
+                                    .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())))
                 }

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::<u8>(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::<u8>(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::<u8>(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<Vec<u8>, String> {
+) -> Result<Vec<u8>, 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.

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,
             },
         })
     }

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,

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<u8>) {
     write_mp4_box!(buf, *b"esds", {
         buf.extend_from_slice(&0u32.to_be_bytes()[..]); // version

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<NonZeroU32>,
     clock_rate: u32,
     extra_data: Vec<u8>,
-
-    #[cfg_attr(not(feature = "unstable-sample-entry"), allow(unused))]
-    sample_entry: Option<Vec<u8>>,
+    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<Vec<u8>, 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()
+                ));
+            }
+        }
     }
 }
 

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,

src/error.rs

@@ -139,4 +139,7 @@ pub(crate) enum ErrorInt {
 
     #[error("Timeout")]
     Timeout,
+
+    #[error("Unsupported: {0}")]
+    Unsupported(String),
 }