fix(cr-2026-04-22): read_wav surfaces sample decode errors
MAJOR from the 2026-04-22 review (wav.rs:135-145): read_wav used filter_map(|s| s.ok()) on both integer and float sample iterators, so any per-sample decode error (truncated payload, corrupted format descriptor after a partial write) was silently discarded. Callers received Ok with a short samples vec, losing audio without any signal to investigate. Switches to collect::<Result<Vec<f32>>>() with a map that converts hound's per-sample errors into KonError::AudioDecodeFailed. First error aborts the read rather than returning a partial vector. Test fabricates the regression by writing a valid WAV and chopping the last 10 bytes; the previous filter_map would have returned Ok with a shortened vec, the new code returns Err.
This commit is contained in:
@@ -123,7 +123,13 @@ pub fn write_wav(path: &Path, audio: &AudioSamples) -> Result<()> {
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Read a WAV file to f32 PCM AudioSamples.
|
/// Read a WAV file to f32 PCM `AudioSamples`.
|
||||||
|
///
|
||||||
|
/// Any per-sample decode error is surfaced as `KonError::AudioDecodeFailed`
|
||||||
|
/// rather than silently dropped. A previous implementation used
|
||||||
|
/// `filter_map(|s| s.ok())`, so a truncated or corrupt payload returned
|
||||||
|
/// a short, silently-partial `AudioSamples` — callers got `Ok` while
|
||||||
|
/// losing audio (flagged by the 2026-04-22 review).
|
||||||
pub fn read_wav(path: &Path) -> Result<AudioSamples> {
|
pub fn read_wav(path: &Path) -> Result<AudioSamples> {
|
||||||
let reader = hound::WavReader::open(path)
|
let reader = hound::WavReader::open(path)
|
||||||
.map_err(|e| KonError::AudioDecodeFailed(format!("WAV open failed: {e}")))?;
|
.map_err(|e| KonError::AudioDecodeFailed(format!("WAV open failed: {e}")))?;
|
||||||
@@ -131,17 +137,27 @@ pub fn read_wav(path: &Path) -> Result<AudioSamples> {
|
|||||||
let spec = reader.spec();
|
let spec = reader.spec();
|
||||||
let sample_rate = spec.sample_rate;
|
let sample_rate = spec.sample_rate;
|
||||||
let channels = spec.channels;
|
let channels = spec.channels;
|
||||||
|
let bits_per_sample = spec.bits_per_sample;
|
||||||
|
|
||||||
let samples: Vec<f32> = match spec.sample_format {
|
let samples: Vec<f32> = match spec.sample_format {
|
||||||
hound::SampleFormat::Int => reader
|
hound::SampleFormat::Int => reader
|
||||||
.into_samples::<i32>()
|
.into_samples::<i32>()
|
||||||
.filter_map(|s| s.ok())
|
.map(|sample| {
|
||||||
.map(|s| s as f32 / (1 << (spec.bits_per_sample - 1)) as f32)
|
sample
|
||||||
.collect(),
|
.map(|s| s as f32 / (1 << (bits_per_sample - 1)) as f32)
|
||||||
|
.map_err(|e| {
|
||||||
|
KonError::AudioDecodeFailed(format!("WAV sample decode failed: {e}"))
|
||||||
|
})
|
||||||
|
})
|
||||||
|
.collect::<Result<Vec<f32>>>()?,
|
||||||
hound::SampleFormat::Float => reader
|
hound::SampleFormat::Float => reader
|
||||||
.into_samples::<f32>()
|
.into_samples::<f32>()
|
||||||
.filter_map(|s| s.ok())
|
.map(|sample| {
|
||||||
.collect(),
|
sample.map_err(|e| {
|
||||||
|
KonError::AudioDecodeFailed(format!("WAV sample decode failed: {e}"))
|
||||||
|
})
|
||||||
|
})
|
||||||
|
.collect::<Result<Vec<f32>>>()?,
|
||||||
};
|
};
|
||||||
|
|
||||||
Ok(AudioSamples::new(samples, sample_rate, channels))
|
Ok(AudioSamples::new(samples, sample_rate, channels))
|
||||||
@@ -217,6 +233,37 @@ mod tests {
|
|||||||
let _ = std::fs::remove_file(&path);
|
let _ = std::fs::remove_file(&path);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn read_wav_surfaces_truncated_sample_stream_errors() {
|
||||||
|
// Regression for the 2026-04-22 review: filter_map(|s| s.ok())
|
||||||
|
// previously swallowed decode errors on corrupt input, so a
|
||||||
|
// truncated WAV returned Ok with a short samples vec. The
|
||||||
|
// new code must propagate the error.
|
||||||
|
let temp_dir = std::env::temp_dir();
|
||||||
|
let path = temp_dir.join("kon_test_truncated_wav.wav");
|
||||||
|
let _ = std::fs::remove_file(&path);
|
||||||
|
|
||||||
|
// Write 100 samples (200 bytes at 16-bit).
|
||||||
|
let original =
|
||||||
|
AudioSamples::mono_16khz((0..100).map(|i| (i as f32) / 100.0).collect());
|
||||||
|
write_wav(&path, &original).unwrap();
|
||||||
|
|
||||||
|
// Drop the last 10 bytes — 5 samples' worth. hound's iterator
|
||||||
|
// should surface an UnexpectedEof on the final read once its
|
||||||
|
// internal data-chunk accounting runs out of bytes.
|
||||||
|
let content = std::fs::read(&path).unwrap();
|
||||||
|
let truncated = &content[..content.len() - 10];
|
||||||
|
std::fs::write(&path, truncated).unwrap();
|
||||||
|
|
||||||
|
let result = read_wav(&path);
|
||||||
|
assert!(
|
||||||
|
result.is_err(),
|
||||||
|
"truncated WAV must surface an AudioDecodeFailed error, got: {result:?}"
|
||||||
|
);
|
||||||
|
|
||||||
|
let _ = std::fs::remove_file(&path);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn wav_roundtrip() {
|
fn wav_roundtrip() {
|
||||||
let temp_dir = std::env::temp_dir();
|
let temp_dir = std::env::temp_dir();
|
||||||
|
|||||||
Reference in New Issue
Block a user