From aad050cb4a98e7fee7d422b971114e48658d3ca1 Mon Sep 17 00:00:00 2001 From: sylph-decoder Date: Sat, 29 Aug 2026 15:42:52 +0000 Subject: [PATCH] formats: XMA1 is not a WAVEFORMATEX -- audio info was reading three wrong fields parse_riff_wave read every fmt chunk as a WAVEFORMATEX. XMA1 (tag 0x0165) is not one, so audio info reported the disc s movie voices as 16 channels, 4310 Hz, 2-bit: 16 is wBitsPerSample read as a channel count and 4310 is wEncodeOptions (0x10d6) read as a sample rate. This misled me earlier in the session and I recorded it as a limitation before finding the cause. XMA1 carries XMAWAVEFORMAT followed by one XMASTREAMFORMAT per stream. The reader now branches on the tag and reads bits at +2, PsuedoBytesPerSec at +12, SampleRate at +16 and Channels at +29. The same three files now report 2 channels, 48000 Hz, 16-bit. The consequence worth having: this crate has no XMA decoder, and data_bytes / PsuedoBytesPerSec is the only route to a duration. Checked against durations decoded independently by the port: ADV presentation 1 137.34 s declared 137.324 s decoded +0.012 percent ADV presentation 2 137.33 s declared 137.324 s decoded +0.004 percent S00A presentation 1 93.71 s declared 93.694 s decoded +0.017 percent So the corpus can now get XMA1 durations off the disc without a decoder, which is a capability I had written down as absent. It is a declared rate rather than a measurement of the samples, and the CLI labels it as such. Regression test pins the real on-disc header bytes and asserts the duration against the independently decoded 137.324 s. 115 lib tests and 3 media disc tests pass. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QsEPXWVaEpyfudtR6re1Pd --- crates/sylpheed-cli/src/main.rs | 10 +- crates/sylpheed-formats/src/audio.rs | 109 ++++++++++++++++-- .../structures/voice-region-leading-chunk.md | 41 ++++++- 3 files changed, 147 insertions(+), 13 deletions(-) diff --git a/crates/sylpheed-cli/src/main.rs b/crates/sylpheed-cli/src/main.rs index e847cb20..ecec9452 100644 --- a/crates/sylpheed-cli/src/main.rs +++ b/crates/sylpheed-cli/src/main.rs @@ -744,8 +744,16 @@ fn cmd_audio_info(file: &Path) -> Result<()> { println!(" Channels : {}", opt(info.channels.map(|c| c.to_string()))); println!(" Sample rate: {}", opt(info.sample_rate.map(|r| format!("{r} Hz")))); println!(" Bit depth : {}", opt(info.bits_per_sample.map(|b| format!("{b}-bit")))); + if let Some(b) = info.avg_bytes_per_sec { + println!(" Byte rate : {} B/s (declared)", b.to_string().yellow()); + } if let Some(d) = info.duration_secs { - println!(" Duration : {d:.2} s"); + let how = if info.codec == sylpheed_formats::AudioCodec::Xma { + " (from the declared byte rate, not decoded)" + } else { + "" + }; + println!(" Duration : {d:.2} s{how}"); } if let Some(p) = info.xma_packets { println!(" XMA packets: {} (2048 B each)", p.to_string().yellow()); diff --git a/crates/sylpheed-formats/src/audio.rs b/crates/sylpheed-formats/src/audio.rs index 90bd0ee0..07282924 100644 --- a/crates/sylpheed-formats/src/audio.rs +++ b/crates/sylpheed-formats/src/audio.rs @@ -97,6 +97,14 @@ pub struct AudioInfo { pub size_bytes: usize, /// 2048-byte XMA packet count, for XMA/raw-XMA streams. pub xma_packets: Option, + /// The stream's **declared** average bytes per second. + /// + /// For XMA1 this is `XMASTREAMFORMAT::PsuedoBytesPerSec`. It is what makes a + /// duration available for a codec we cannot decode: `data_bytes / this` + /// agreed with an independently decoded duration to **0.01 %** on the two + /// movie voices it was checked against + /// (`docs/re/structures/voice-region-leading-chunk.md`). + pub avg_bytes_per_sec: Option, } impl AudioInfo { @@ -110,6 +118,7 @@ impl AudioInfo { duration_secs: None, size_bytes: size, xma_packets: None, + avg_bytes_per_sec: None, } } @@ -181,6 +190,7 @@ fn parse_riff_wave(bytes: &[u8]) -> Option { let mut pos = 12; let (mut tag, mut channels, mut rate, mut bits) = (0u16, 0u16, 0u32, 0u16); + let mut avg_bps = 0u32; let mut data_bytes: Option = None; let mut have_fmt = false; @@ -191,13 +201,37 @@ fn parse_riff_wave(bytes: &[u8]) -> Option { match id { b"fmt " if body + 16 <= bytes.len() => { tag = le16(body); - channels = le16(body + 2); - rate = le32(body + 4); - bits = le16(body + 14); - // WAVE_FORMAT_EXTENSIBLE stores the real tag in the GUID's first - // two bytes, right after cbSize (+2) → +24 from the fmt body. - if tag == WAVE_FORMAT_EXTENSIBLE && body + 26 <= bytes.len() { - tag = le16(body + 24); + // 🔴 XMA1 is NOT a WAVEFORMATEX. Reading it as one is where + // `audio info` got "16 channels, 4310 Hz, 2-bit" from the + // movie voices: 16 is `wBitsPerSample` read as channels, and + // 4310 is `wEncodeOptions` (0x10d6) read as a sample rate. + // + // XMA1 carries `XMAWAVEFORMAT`, then one `XMASTREAMFORMAT` per + // stream (xenia-canary `src/xenia/apu/xma_context.h`, + // cross-checked against the disc's own movie-voice headers): + // + // +0 wFormatTag +2 wBitsPerSample +4 wEncodeOptions + // +6 wLargestSkip +8 wNumStreams +10 bLoopCount (u8) + // +11 bStreamCount (u8) + // +12 PsuedoBytesPerSec +16 SampleRate +20 LoopStart + // +24 LoopEnd +28 SubframeData (u8) +29 Channels (u8) + // +30 ChannelMask + if tag == WAVE_FORMAT_XMA && body + 32 <= bytes.len() { + bits = le16(body + 2); + avg_bps = le32(body + 12); + rate = le32(body + 16); + channels = bytes[body + 29] as u16; + } else { + channels = le16(body + 2); + rate = le32(body + 4); + bits = le16(body + 14); + avg_bps = le32(body + 8); + // WAVE_FORMAT_EXTENSIBLE stores the real tag in the GUID's + // first two bytes, right after cbSize (+2) → +24 from the + // fmt body. + if tag == WAVE_FORMAT_EXTENSIBLE && body + 26 <= bytes.len() { + tag = le16(body + 24); + } } have_fmt = true; } @@ -222,6 +256,15 @@ fn parse_riff_wave(bytes: &[u8]) -> Option { info.channels = Some(channels).filter(|&c| c > 0); info.sample_rate = Some(rate).filter(|&r| r > 0); info.bits_per_sample = Some(bits).filter(|&b| b > 0); + info.avg_bytes_per_sec = Some(avg_bps).filter(|&b| b > 0); + + // A declared byte rate gives a duration for a codec we cannot decode. Only + // for XMA1, where the field is `PsuedoBytesPerSec` and means exactly this. + if codec == AudioCodec::Xma && avg_bps > 0 { + if let Some(d) = data_bytes { + info.duration_secs = Some(d as f32 / avg_bps as f32); + } + } match codec { AudioCodec::Pcm | AudioCodec::PcmFloat => { @@ -356,6 +399,58 @@ mod tests { assert!((audio.samples[2] - 0.99997).abs() < 1e-3); // 32767/32768 } + /// XMA1 is not a `WAVEFORMATEX`, and reading it as one produced nonsense. + /// + /// The bytes here are the real `fmt ` chunk of `ADV`'s first movie-voice + /// presentation, copied off the disc. Read as a `WAVEFORMATEX` it reports + /// **16 channels, 4310 Hz, 2-bit** — 16 is `wBitsPerSample`, 4310 is + /// `wEncodeOptions` (`0x10d6`). Read as an `XMAWAVEFORMAT` it reports 2 + /// channels, 48 kHz, 16-bit, 8142 B/s. + /// + /// The duration is the part worth guarding: this crate has no XMA decoder, + /// and `data_bytes / PsuedoBytesPerSec` is the only route to one. It agrees + /// with an independently decoded 137.324 s to **0.02 %**. + #[test] + fn xma1_fmt_is_not_a_waveformatex() { + let mut v = Vec::new(); + v.extend_from_slice(b"RIFF"); + v.extend_from_slice(&0u32.to_le_bytes()); + v.extend_from_slice(b"WAVE"); + v.extend_from_slice(b"fmt "); + v.extend_from_slice(&32u32.to_le_bytes()); + // XMAWAVEFORMAT, exactly as it appears on the disc. + v.extend_from_slice(&[ + 0x65, 0x01, // wFormatTag = 0x0165 (XMA1) + 0x10, 0x00, // wBitsPerSample = 16 + 0xd6, 0x10, // wEncodeOptions = 0x10d6 <- was misread as the rate + 0x00, 0x00, // wLargestSkip + 0x01, 0x00, // wNumStreams + 0x00, // bLoopCount + 0x02, // bStreamCount + 0xce, 0x1f, 0x00, 0x00, // PsuedoBytesPerSec = 8142 + 0x80, 0xbb, 0x00, 0x00, // SampleRate = 48000 + 0x00, 0x00, 0x00, 0x00, // LoopStart + 0x00, 0x00, 0x00, 0x00, // LoopEnd + 0x00, // SubframeData + 0x02, // Channels = 2 <- was read from +2 as 16 + 0x02, 0x00, // ChannelMask + ]); + v.extend_from_slice(b"data"); + v.extend_from_slice(&1_118_208u32.to_le_bytes()); + + let info = AudioInfo::probe(&v); + assert_eq!(info.codec, AudioCodec::Xma); + assert_eq!(info.channels, Some(2), "channels came from wBitsPerSample"); + assert_eq!(info.sample_rate, Some(48_000), "rate came from wEncodeOptions"); + assert_eq!(info.bits_per_sample, Some(16)); + assert_eq!(info.avg_bytes_per_sec, Some(8142)); + let d = info.duration_secs.expect("duration from the declared byte rate"); + assert!( + (d - 137.324).abs() < 0.05, + "declared-rate duration {d} should match the decoded 137.324 s" + ); + } + #[test] fn probe_xma2_riff_reports_metadata_not_decode() { // Minimal RIFF/WAVE with an XMA2 fmt tag. diff --git a/docs/re/structures/voice-region-leading-chunk.md b/docs/re/structures/voice-region-leading-chunk.md index c0fa5403..a64bc1f4 100644 --- a/docs/re/structures/voice-region-leading-chunk.md +++ b/docs/re/structures/voice-region-leading-chunk.md @@ -299,11 +299,42 @@ undermine it. What it touches is the *explanation*: "more bytes means a duplicated channel, not better fidelity" is true of `ADV` and is **not** a fact about the format. It should not harden into one. -⚠️ Note for anyone reading our own tooling: `sylpheed-cli audio info` reports -these chunks as *16 channels, 4310 Hz, 2-bit*. Those are the `wBitsPerSample` -(16), `wEncodeOptions` (`0x10d6` = 4310) and channel fields read at the wrong -offsets. The header above is the correct layout; the CLI's reader is misaligned -for XMA1 and should not be used on these. +### ✅ FIXED 2026-08-29 — and the disc will now tell you a duration without a decoder + +`sylpheed-cli audio info` used to report these chunks as *16 channels, 4310 Hz, +2-bit*. The cause: `parse_riff_wave` read every `fmt ` chunk as a +`WAVEFORMATEX`, and **XMA1 is not one**. 16 is `wBitsPerSample` read as a channel +count; 4310 is `wEncodeOptions` (`0x10d6`) read as a sample rate. + +XMA1 carries `XMAWAVEFORMAT` followed by one `XMASTREAMFORMAT` per stream, and +the reader now branches on the tag. Same three files: + +``` +Channels : 2 Sample rate: 48000 Hz Bit depth : 16-bit +Byte rate : 8142 B/s (declared) +Duration : 137.34 s (from the declared byte rate, not decoded) +``` + +✅ **The duration is the part that matters, because this crate has no XMA +decoder.** `data_bytes / PsuedoBytesPerSec` is the only route to one, and it was +checked against durations the port decoded independently: + +| stream | declared-rate duration | independently decoded | error | +|---|---|---|---| +| `ADV` presentation 1 | 137.34 s | 137.324 s | **+0.012 %** | +| `ADV` presentation 2 | 137.33 s | 137.324 s | **+0.004 %** | +| `S00A` presentation 1 | 93.71 s | 93.694 s | **+0.017 %** | + +⚠️ It is a *declared* rate, so this is the file's own claim about itself rather +than a measurement of the samples — but on the three streams where an independent +decode exists, the claim is accurate to 0.02 %. Regression test +`xma1_fmt_is_not_a_waveformatex` pins the real on-disc header bytes. + +⚠️ **Retroactive note:** several statements earlier in this session said this +container could not obtain a duration for these streams. That was true of the +decoder and *not* of the file, which had been declaring it at `fmt +0x20` the +whole time. The tool was misreading it, and a broken tool reported as a missing +capability is worth more than the fix. ⚠️ **Do not "fix" it by concatenating.** The port measured a concatenated region at 359 s against a 137 s movie.