craft-audio: review round 4 for the coded formats

- craft-media: Mp4Writer counts written bytes with a checked sum, so a
  writer that takes bytes past the last position gets an error, not an
  overflow (correction to round 3, which checked only the header).
- Docs: the AAC bit-rate floor keeps a margin above where the encoder
  codes silence, about half of it.

Refs #1230

crates/audio/README.md

@@ -257,7 +257,8 @@ writes nothing. After a write fails for another reason, every later
 `with_bitrate(bits_per_second)` sets a constant bit rate for all channels together. A rate the
 stream cannot carry is moved to the nearest one it can: MP3 and AC-3 use the highest rate of their
 tables not above it, AAC and E-AC-3 keep it inside their range (AAC from 6 kbit/s or a quarter of
-the sample rate per channel, whichever is more: below that the encoder has bits for silence only). `with_quality(0.0..=1.0)` is AAC's
+the sample rate per channel, whichever is more: from about half of that down the encoder codes
+silence). `with_quality(0.0..=1.0)` is AAC's
 constant quality (VBR); the other lossy formats return `Unsupported` for it. Without either, AAC
 is written at 64 kbit/s per channel, MP3 at 96 kbit/s per channel, and AC-3 and E-AC-3 at
 96 kbit/s per full-bandwidth channel (the LFE not counted).

crates/audio/src/lib.rs

@@ -19,7 +19,7 @@
 //!   gives [`wav_float_header`]), FLAC (feature `flac-encode`), AAC-LC and ALAC in M4A
 //!   (`aac-encode`, `alac-encode`) and MP3, AC-3 and E-AC-3 streams (`mp3-encode`, `ac3-encode`,
 //!   `eac3-encode`), whole-buffer or incremental into any `Write + Seek`. Feature
-//!   `lossless-encode` turns on WAV, AIFF and FLAC; feature `encode` turns on every format.
+//!   `lossless-encode` turns on WAV, AIFF and FLAC; feature `encode` turns on every format and encoder.
 //! - [`wav_pcm`], [`aiff_pcm`] (feature `decode-native`) and [`pcm_wav_file`], [`pcm_wav_header`],
 //!   [`pcm_aiff_file`], [`pcm_aiff_header`] (feature `pcm-encode`): the integer PCM of a WAV or AIFF
 //!   file bit for bit, and minimal files around such PCM, for essence embedded in other containers.
@@ -367,7 +367,8 @@ pub enum RateControl {
     /// A constant bit rate in bits per second, for all channels together. A format with a fixed
     /// set of rates (MP3, AC-3) uses the highest one not above it; a rate outside what the
     /// stream can carry is moved to the nearest one inside. AAC is written at 6 kbit/s or a
-    /// quarter of the sample rate per channel at least, where its encoder starts to code audio.
+    /// quarter of the sample rate per channel at least: from about half of that down its encoder
+    /// codes silence.
     Bitrate(u32),
     /// A constant quality from 0.0 (smallest) to 1.0 (best), at whatever bit rate each frame
     /// needs. AAC only; values outside the range are clamped, and NaN is an error.

crates/audio/src/write/coded.rs

@@ -105,8 +105,8 @@ fn aac_order(channels: usize) -> Option<&'static [usize]> {
 }
 
 /// The lowest AAC bit rate the writer uses, per channel: 6 kbit/s, and a quarter of the sample
-/// rate (256 bits a frame). The encoder accepts less, but then has too few bits for anything but
-/// silence, and a file of silence is no answer to a low bit rate (or to kbit/s given as bit/s).
+/// rate (256 bits a frame). The encoder accepts less, and from about half of this down codes
+/// silence; a file of silence is no answer to a low bit rate (or to kbit/s given as bit/s).
 #[cfg(feature = "aac-encode")]
 fn aac_floor_per_channel(sample_rate: u32) -> u32 {
     (sample_rate / 4).max(6000)

crates/media/src/isobmff/mux.rs

@@ -615,6 +615,12 @@ fn validate_track(entry: &SampleEntry, timescale: u32) -> Result<Option<u32>> {
     Ok(None)
 }
 
+/// The position after `bytes` more. A writer that takes bytes past the last position a file can
+/// have is an error here, not an overflow.
+fn advance(pos: u64, bytes: usize) -> Result<u64> {
+    pos.checked_add(bytes as u64).ok_or_else(|| Error::Mux("the file would end past the last position".into()))
+}
+
 /// Progressive MP4/MOV writer: `ftyp`, then one `mdat` streamed to `W`, then `moov` at [`finish`](Self::finish)
 /// (or moved in front of `mdat` by [`finish_faststart`](Self::finish_faststart)).
 pub struct Mp4Writer<W: Write + Seek> {
@@ -739,7 +745,7 @@ impl<W: Write + Seek> Mp4Writer<W> {
         }
         self.w.write_all(s.data)?;
         let offset = self.pos - self.start;
-        self.pos += s.data.len() as u64;
+        self.pos = advance(self.pos, s.data.len())?;
         let contiguous = self.last_track == Some(track);
         self.last_track = Some(track);
         let tr = &mut self.tracks[track];
@@ -780,7 +786,7 @@ impl<W: Write + Seek> Mp4Writer<W> {
             let d = rescale(vd, vts, tts).max(tc_dur as u64).min(u32::MAX as u64) as u32;
             self.w.write_all(&start.to_be_bytes())?;
             let off = self.pos - self.start;
-            self.pos += 4;
+            self.pos = advance(self.pos, 4)?;
             self.last_track = None;
             let t = &mut self.tracks[i].t;
             t.stts.push((1, d));
@@ -843,7 +849,7 @@ impl<W: Write + Seek> Mp4Writer<W> {
             let d = u32::try_from(end.saturating_sub(c.start)).map_err(|_| Error::Mux("chapter longer than 2^32 movie ticks".into()))?;
             let data = text_sample(&c.title);
             self.w.write_all(&data)?;
-            self.pos += data.len() as u64;
+            self.pos = advance(self.pos, data.len())?;
             Tables::push_run(&mut t.stts, 1, d);
             Tables::push_run(&mut t.ctts, 1, 0);
             t.sizes.push(data.len() as u32);
@@ -1415,6 +1421,31 @@ mod tests {
         }
     }
 
+    /// A writer that keeps taking bytes where positions end makes writing fail, not overflow.
+    #[test]
+    fn samples_past_the_last_position_are_refused() {
+        struct Endless(u64);
+        impl Write for Endless {
+            fn write(&mut self, data: &[u8]) -> std::io::Result<usize> {
+                self.0 = self.0.wrapping_add(data.len() as u64);
+                Ok(data.len())
+            }
+            fn flush(&mut self) -> std::io::Result<()> {
+                Ok(())
+            }
+        }
+        impl Seek for Endless {
+            fn seek(&mut self, _: SeekFrom) -> std::io::Result<u64> {
+                Ok(self.0)
+            }
+        }
+        let mut w = Mp4Writer::new(Endless(u64::MAX - 100), WriterOptions::new(Brand::Mp4)).unwrap();
+        let v = w.add_track(TrackConfig::new(avc_entry(), 1000)).unwrap();
+        let sample = WriteSample { data: &[0; 40], duration: 40, composition_offset: 0, is_sync: true };
+        w.write_sample(v, sample).unwrap();
+        assert!(matches!(w.write_sample(v, sample), Err(Error::Mux(_))));
+    }
+
     /// A file begun in the middle of a writer is a file from there: its chunk offsets count from
     /// its own first byte, with `moov` at the end or in front, a timecode track and chapters.
     #[test]