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

FLAC in Ogg reads its layout the way a FLAC file does: from the mask
comment, or else the one decoders assume for the count. Without this, a
6-channel stream with no comment reported back surrounds in Ogg and side
surrounds in a FLAC file.

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

crates/audio/README.md

@@ -264,7 +264,7 @@ match craft_audio::decode(&bytes, None) {
   `WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment. This includes the usual layouts for 5 and 6 channels:
   without the comment, back surrounds are read as side surrounds. It includes the usual 7-channel
   layout too: without the comment, its back left, back right and back centre are read as back
-  centre, side left and side right. craft-audio reads the comment in FLAC files, not in FLAC in Ogg.
+  centre, side left and side right.
 - FLAC at 32 bits codes stereo channels independently, because a side channel would need 33 bits.
 - Outside RFC 9639's streamable subset (§7), which some decoders require: FLAC at 32 bits, at a
   sample rate the frame header cannot name, or with a layout that needs the mask comment.

crates/audio/src/read/flac.rs

@@ -85,10 +85,25 @@ pub(crate) fn layout(b: &[u8], at: usize) -> Option<ChannelLayout> {
 /// `channels` channels described by the channel mask comment of the stream at `at`, or else in
 /// the layout decoders assume for the count (RFC 9639 §8.6.2, §9.1.3).
 fn layout_of(b: &[u8], at: usize, channels: u16) -> ChannelLayout {
-    let mask = channel_mask(b, at).unwrap_or_else(|| crate::layout::flac_assumed_mask(usize::from(channels)));
+    layout_from(channel_mask(b, at), channels)
+}
+
+/// `channels` channels described by a channel mask comment's value, or else in the layout decoders
+/// assume for the count.
+pub(crate) fn layout_from(mask: Option<u32>, channels: u16) -> ChannelLayout {
+    let mask = mask.unwrap_or_else(|| crate::layout::flac_assumed_mask(usize::from(channels)));
     ChannelLayout::from_mask_and_channels(mask, channels)
 }
 
+/// The mask in a comment, when its key is the channel mask's and its value a `0x` hexadecimal number.
+pub(crate) fn comment_mask(key: &[u8], value: &[u8]) -> Option<u32> {
+    if !key.eq_ignore_ascii_case(b"WAVEFORMATEXTENSIBLE_CHANNEL_MASK") {
+        return None;
+    }
+    let hex = value.strip_prefix(b"0x").or_else(|| value.strip_prefix(b"0X"))?;
+    u32::from_str_radix(std::str::from_utf8(hex).ok()?, 16).ok()
+}
+
 /// The value of the first `WAVEFORMATEXTENSIBLE_CHANNEL_MASK` comment that holds a `0x` hexadecimal
 /// number, in the stream at `at`.
 fn channel_mask(b: &[u8], at: usize) -> Option<u32> {
@@ -118,11 +133,7 @@ fn comments_mask(block: &[u8]) -> Option<u32> {
         let comment = block.get(at + 4..(at + 4).checked_add(len)?)?;
         at += 4 + len;
         let Some(eq) = comment.iter().position(|&c| c == b'=') else { continue };
-        if !comment[..eq].eq_ignore_ascii_case(b"WAVEFORMATEXTENSIBLE_CHANNEL_MASK") {
-            continue;
-        }
-        let hex = comment[eq + 1..].strip_prefix(b"0x").or_else(|| comment[eq + 1..].strip_prefix(b"0X"));
-        if let Some(mask) = hex.and_then(|h| std::str::from_utf8(h).ok()).and_then(|h| u32::from_str_radix(h, 16).ok()) {
+        if let Some(mask) = comment_mask(&comment[..eq], &comment[eq + 1..]) {
             return Some(mask);
         }
     }

crates/audio/src/read/symph.rs

@@ -16,7 +16,7 @@ use symphonia::core::errors::Error as SymError;
 use symphonia::core::formats::probe::{Hint, ProbeOptions, ProbeableFormat, Score, Scoreable};
 use symphonia::core::formats::{FormatOptions, FormatReader, SeekMode, SeekTo, Track};
 use symphonia::core::io::{MediaSourceStream, MediaSourceStreamOptions, ScopedStream};
-use symphonia::core::meta::MetadataOptions;
+use symphonia::core::meta::{MetadataOptions, RawValue};
 use symphonia::core::units::{TimeBase, Timestamp};
 
 use super::id3v2_len;
@@ -446,6 +446,21 @@ fn layout_of(channels: Option<&Channels>) -> ChannelLayout {
     }
 }
 
+/// The channel mask comment among the tags the reader has read.
+fn metadata_channel_mask(reader: &mut dyn FormatReader) -> Option<u32> {
+    let mut metadata = reader.metadata();
+    loop {
+        let mask = metadata.current()?.media.tags.iter().find_map(|t| match &t.raw.value {
+            RawValue::String(value) => super::flac::comment_mask(t.raw.key.as_bytes(), value.as_bytes()),
+            _ => None,
+        });
+        if mask.is_some() {
+            return mask;
+        }
+        metadata.pop()?;
+    }
+}
+
 /// The stored sample format, using the codec and its declared bit depth. Lossy and unusual
 /// codecs decode to `Compressed`.
 fn sample_format_of(p: &AudioCodecParameters) -> SampleFormat {
@@ -547,7 +562,7 @@ impl Symph {
     /// Open audio track `track` (the first when `None`) of the file in `bytes`.
     pub(crate) fn open(bytes: Cow<'_, [u8]>, detected: Option<Container>, track: Option<usize>) -> Result<Symph> {
         guarded(move || {
-            let (reader, flac_layout) = match open(bytes, detected)? {
+            let (mut reader, flac_layout) = match open(bytes, detected)? {
                 Opened::Reader(reader, layout) => (reader, layout),
                 Opened::Empty(info) => {
                     // a stream with no audio still has one track, as WAV and AIFF do
@@ -573,6 +588,8 @@ impl Symph {
             let (track_id, mut info) = info_of(reader.as_ref(), detected, track)?;
             if let Some(layout) = flac_layout.filter(|l| l.channels() == info.channels) {
                 info.layout = layout;
+            } else if info.codec == CodecId::Flac && info.container == Container::Ogg {
+                info.layout = super::flac::layout_from(metadata_channel_mask(reader.as_mut()), info.channels);
             }
             let t = reader.tracks().iter().find(|t| t.id == track_id).ok_or_else(|| AudioError::Malformed("track vanished".into()))?;
             let params = audio_params(t).cloned().ok_or_else(|| AudioError::Malformed("track vanished".into()))?;

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

@@ -236,6 +236,34 @@ fn layouts_decoders_would_not_assume_are_recorded() {
     }
 }
 
+/// FLAC in Ogg reads its layout as a FLAC file does: from the mask comment, or else the one
+/// decoders assume for the count.
+#[cfg(feature = "symphonia")]
+#[test]
+fn ogg_flac_reads_the_mask_comment() {
+    use crate::ChannelLayout;
+    let Some(ff) = ffmpeg() else { return };
+    let cases: [(&str, &str, &[&str], u16, u32); 3] = [
+        ("ogg-mask", "aevalsrc=0|0|0|0:c=4c:d=0.1:s=48000", &["-metadata", "WAVEFORMATEXTENSIBLE_CHANNEL_MASK=0x0107"], 4, 0x107),
+        ("ogg-5.1", "aevalsrc=0|0|0|0|0|0:c=5.1:d=0.1:s=48000", &[], 6, 0x60F),
+        ("ogg-stereo", "aevalsrc=0|0:c=stereo:d=0.1:s=48000", &["-metadata", "TITLE=x"], 2, 0x3),
+    ];
+    for (name, source, extra, channels, mask) in cases {
+        let path = common::fixture_subdir("flac").join(format!("{name}.oga"));
+        let status = Command::new(&ff)
+            .args(["-v", "error", "-y", "-f", "lavfi", "-i", source, "-c:a", "flac", "-f", "ogg"])
+            .args(extra)
+            .arg(&path)
+            .status()
+            .unwrap();
+        assert!(status.success(), "{name}");
+        let bytes = std::fs::read(&path).unwrap();
+        let expected = ChannelLayout::from_mask_and_channels(mask, channels);
+        assert_eq!(crate::probe(&bytes, None).unwrap().layout, expected, "probe {name}");
+        assert_eq!(crate::decode(&bytes, None).unwrap().0.layout, expected, "decode {name}");
+    }
+}
+
 #[test]
 fn the_mask_comment_is_written_only_when_decoders_would_assume_otherwise() {
     use super::mask_comment;