craft-audio: review fixes, round 5 (#442)

- libFLAC and ffmpeg read a FLAC with 5 or 6 channels and no mask comment
  as having side surrounds, so the comment is now written for anything
  else, including the crate's usual back-surround layouts. The round 3
  check treated back surrounds as FLAC's assignment, which lost them.
- Tests: the mask comment rules, opposite-polarity stereo, STREAMINFO
  block and frame sizes.
- A stale comment said only WAV records the layout.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

crates/audio/README.md

@@ -271,7 +271,7 @@ match craft_audio::decode(&bytes, None) {
 | Codec | Spec | Scope | Verified against |
 |---|---|---|---|
 | WAV, AIFF | Microsoft RIFF, Apple AIFF/AIFF-C | read: integer 8–32-bit, float 32/64-bit, `WAVE_FORMAT_EXTENSIBLE` masks, RF64/BW64, BWF `bext`, truncated files; write: 16/24/32-bit integer and 32-bit float, RF64 above 4 GiB, any layout, TPDF dither | round trips; hostile-input tests |
-| FLAC encoder | RFC 9639 | 16, 24, 32 bits; 1–8 channels, layouts outside FLAC's channel assignment as a `WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment; up to 655,350 Hz; LPC, stereo decorrelation, STREAMINFO MD5 | bit-exact through ffmpeg, STREAMINFO MD5 checked against ffmpeg |
+| FLAC encoder | RFC 9639 | 16, 24, 32 bits; 1–8 channels, layouts decoders would not assume for the count as a `WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment; up to 655,350 Hz; LPC, stereo decorrelation, STREAMINFO MD5 | bit-exact through ffmpeg, STREAMINFO MD5 checked against ffmpeg |
 
 ## symphonia
 

crates/audio/src/lib.rs

@@ -269,9 +269,10 @@ pub struct EncodeOptions {
     /// The speaker layout to record, for layouts a channel count alone cannot identify (5.1 with
     /// side vs back surrounds, 7.0 vs 6.1, Ambisonics as [`ChannelLayout::unordered`]). `None` is
     /// the usual layout for the channel count. WAV stores it as `dwChannelMask`, FLAC as a
-    /// `WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment when it is not FLAC's fixed assignment for the
-    /// count (which for 7 channels is 6.1 with side surrounds, not the usual layout); AIFF has a
-    /// fixed channel order and ignores it. It must have as many channels as the audio.
+    /// `WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment when decoders would assume another layout for
+    /// the count (they read 5 and 6 channels with side surrounds and 7 as 6.1 with sides, so the
+    /// usual layouts for those counts get one); AIFF has a fixed channel order and ignores it. It
+    /// must have as many channels as the audio.
     pub layout: Option<ChannelLayout>,
     /// Write WAV in the RF64 layout even below 4 GiB (WAV only; ignored for other formats).
     pub force_rf64: bool,

crates/audio/src/write/flac/mod.rs

@@ -39,18 +39,23 @@ fn check(channels: usize, sample_rate: u32, depth: BitDepth) -> Result<u8> {
 /// Frames quantized and handed to the encoder at a time: its block size.
 const CHUNK: usize = 4096;
 
-/// FLAC's channel assignment for 1–8 channels (RFC 9639 §9.1.3) as a WAV mask: the usual layout
-/// for the count, except that 7 channels are 6.1 with side surrounds (FL FR FC LFE BC SL SR).
-fn flac_mask(channels: usize) -> u32 {
-    let layout = if channels == 7 { ChannelLayout::SURROUND_6_1 } else { ChannelLayout::default_for(channels as u16) };
-    layout.mask().unwrap_or(0)
+/// The layout decoders assume for 1–8 channels with no channel mask comment, as a WAV mask. RFC
+/// 9639 §9.1.3 leaves "back/surround" open for 5 and 6 channels; libFLAC and ffmpeg read side
+/// surrounds. 7 channels are 6.1 with sides (FL FR FC LFE BC SL SR).
+fn assumed_mask(channels: usize) -> u32 {
+    match channels {
+        5 => 0x607,
+        6 => 0x60F,
+        7 => 0x70F,
+        n => ChannelLayout::default_for(n as u16).mask().unwrap_or(0),
+    }
 }
 
-/// The `WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment (RFC 9639 §8.6.2) for a layout FLAC's channel
-/// assignment does not already say: `mask` (the usual layout for the count when `None`).
+/// The `WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment (RFC 9639 §8.6.2) for a layout decoders would
+/// not assume: `mask` (the usual layout for the count when `None`).
 fn mask_comment(channels: usize, mask: Option<u32>) -> Option<String> {
     let mask = mask.unwrap_or_else(|| ChannelLayout::default_for(channels as u16).mask().unwrap_or(0));
-    (mask != flac_mask(channels)).then(|| format!("WAVEFORMATEXTENSIBLE_CHANNEL_MASK=0x{mask:04X}"))
+    (mask != assumed_mask(channels)).then(|| format!("WAVEFORMATEXTENSIBLE_CHANNEL_MASK=0x{mask:04X}"))
 }
 
 fn encoder_error(e: encoder::Error) -> AudioError {
@@ -73,7 +78,7 @@ pub(crate) struct Writer<W> {
 
 impl<W: Write + Seek> Writer<W> {
     /// `mask` is the speaker layout as a WAV channel mask (the usual layout for the count when
-    /// `None`), recorded as a comment when FLAC's channel assignment says otherwise.
+    /// `None`), recorded as a comment when decoders would assume another one.
     pub(crate) fn new(mut out: W, channels: usize, sample_rate: u32, depth: BitDepth, dither: bool, mask: Option<u32>) -> Result<Self> {
         let bps = check(channels, sample_rate, depth)?;
         // `check` bounds channels to 1..=8.

crates/audio/src/write/flac/oracle_tests.rs

@@ -209,10 +209,10 @@ fn full_scale_32_bit_stereo_round_trips() {
     assert!(enc.encode(&[&[i32::MIN, i32::MAX], &[0, -1]]).is_ok());
 }
 
-/// A layout FLAC's channel assignment does not name is recorded as a channel mask comment, which
-/// ffmpeg reads: the usual 7-channel layout (backs and a back centre) differs from FLAC's 6.1.
+/// A layout decoders would not assume for the channel count is recorded as a channel mask comment,
+/// which ffmpeg reads: they read 6 channels with side surrounds, and 7 as 6.1 with sides.
 #[test]
-fn layouts_other_than_flacs_are_recorded() {
+fn layouts_decoders_would_not_assume_are_recorded() {
     use crate::{AudioBuffer, ChannelLayout, EncodeFormat, EncodeOptions};
     let Some(ffprobe) = common::ffprobe() else { return };
     let cases = [
@@ -220,6 +220,8 @@ fn layouts_other_than_flacs_are_recorded() {
         ("layout-7-default", 7, None, "6.1(back)"),
         ("layout-6.1-side", 7, Some(ChannelLayout::SURROUND_6_1), "6.1"),
         ("layout-5.1-side", 6, Some(ChannelLayout::SURROUND_5_1_SIDE), "5.1(side)"),
+        ("layout-5.1-back", 6, Some(ChannelLayout::SURROUND_5_1), "5.1"),
+        ("layout-6-default", 6, None, "5.1"),
     ];
     for (name, channels, layout, expected) in cases {
         let buf = AudioBuffer::new(48_000, channels, 100);
@@ -233,3 +235,47 @@ fn layouts_other_than_flacs_are_recorded() {
         assert_eq!(String::from_utf8_lossy(&out.stdout).trim(), expected, "{name}");
     }
 }
+
+#[test]
+fn the_mask_comment_is_written_only_when_decoders_would_assume_otherwise() {
+    use super::mask_comment;
+    let comment = |m: &str| Some(format!("WAVEFORMATEXTENSIBLE_CHANNEL_MASK={m}"));
+    for channels in [1, 2, 3, 4, 8] {
+        assert_eq!(mask_comment(channels, None), None, "{channels}");
+    }
+    assert_eq!(mask_comment(5, None), comment("0x0037"));
+    assert_eq!(mask_comment(5, Some(0x607)), None);
+    assert_eq!(mask_comment(6, None), comment("0x003F"));
+    assert_eq!(mask_comment(6, Some(0x60F)), None);
+    assert_eq!(mask_comment(7, None), comment("0x013F"));
+    assert_eq!(mask_comment(7, Some(0x70F)), None);
+    // no speaker assignment
+    assert_eq!(mask_comment(2, Some(0)), comment("0x0000"));
+}
+
+/// Left and right at opposite full scale: the side channel needs its extra bit at every depth.
+#[test]
+fn opposite_polarity_stereo_round_trips() {
+    for bps in [16u8, 24] {
+        let left = signal("square", 9_000, bps, 1);
+        let right: Vec<i32> = left.iter().map(|&v| !v).collect();
+        let planar = vec![left, right];
+        if round_trip(&format!("opposite-{bps}"), &planar, EncoderConfig::new(48_000, 2, bps), 4096).is_none() {
+            return;
+        }
+    }
+}
+
+#[test]
+fn streaminfo_states_the_block_and_frame_sizes() {
+    let planar = [signal("music", 10_000, 16, 21), signal("noise", 10_000, 16, 22)];
+    let mut enc = Encoder::new(EncoderConfig::new(48_000, 2, 16)).unwrap();
+    let parts: Vec<&[i32]> = planar.iter().map(Vec::as_slice).collect();
+    let mut sizes: Vec<usize> = enc.encode_frames(&parts).unwrap().iter().map(Vec::len).collect();
+    sizes.push(enc.finish().len());
+    let si = enc.streaminfo();
+    let u24 = |b: &[u8]| usize::from(b[0]) << 16 | usize::from(b[1]) << 8 | usize::from(b[2]);
+    assert_eq!(&si[..4], &[0x10, 0x00, 0x10, 0x00]);
+    assert_eq!(u24(&si[4..7]), sizes.iter().copied().min().unwrap());
+    assert_eq!(u24(&si[7..10]), sizes.iter().copied().max().unwrap());
+}

crates/audio/src/write/mod.rs

@@ -26,7 +26,7 @@ use std::io::{Seek, Write};
 /// # }
 /// ```
 pub fn encode(buf: &AudioBuffer, opts: &EncodeOptions) -> Result<Vec<u8>> {
-    // Checked for every format; only a WAV records it.
+    // Checked for every format; WAV and FLAC record it.
     let mask = opts.layout.map(|l| wav_mask(&l, buf.channels.len())).transpose()?;
     match opts.format {
         #[cfg(feature = "pcm-encode")]