Fixed encoder error when encoding WAV 8/24-bit input.

flacenc-bin/Cargo.toml

@@ -38,7 +38,9 @@ pprof = { version = "0.13", features = [
   "flamegraph",
   "protobuf-codec",
 ], optional = true }
+md-5 = "0.10.6"
 rmp-serde = "1.1.2"
+tempfile = "3"
 termcolor = "1.4"
 toml = "0.8.10"
 
@@ -49,5 +51,4 @@ simd-nightly = ["flacenc/simd-nightly"]
 
 [dev-dependencies]
 flacenc = { version = "0.5.0", path = "..", features = ["__export_sigen"] }
-tempfile = "3"
 rstest = "0.22"

flacenc-bin/src/main.rs

@@ -61,6 +61,7 @@ use clap::Args;
 use clap::Parser;
 use clap::Subcommand;
 use log::info;
+use md5::Digest;
 #[cfg(feature = "pprof")]
 use pprof::protos::Message;
 
@@ -207,10 +208,6 @@ fn main_enc_body(args: EncodeArgs) -> Result<(), NonZeroU8> {
         display::show_error_msg("Invalid config parameter is detected.", Some(e));
         EX_DATAERR
     })?;
-    if let Err(e) = encoder_config.verify() {
-        eprintln!("Error: {}", e.within("encoder_config"));
-        return Err(EX_DATAERR);
-    }
 
     let _ = display::show_progress(&args, &Progress::Started);
 
@@ -311,8 +308,13 @@ fn main_dec_body(args: DecodeArgs) -> Result<(), NonZeroU8> {
         })?;
     }
 
-    let mut writer = hound::WavWriter::create(
-        &args.output,
+    let temp_wav_file = tempfile::NamedTempFile::new().map_err(|e| {
+        display::show_error_msg("Unable to create temporary output file.", Some(e));
+        EX_IOERR
+    })?;
+
+    let mut writer = hound::WavWriter::new(
+        &temp_wav_file,
         hound::WavSpec {
             channels: stream_info.channels() as u16,
             sample_rate: stream_info.sample_rate() as u32,
@@ -321,20 +323,36 @@ fn main_dec_body(args: DecodeArgs) -> Result<(), NonZeroU8> {
         },
     )
     .map_err(|e| {
-        display::show_error_msg("Failed to create the output file.", Some(e));
+        display::show_error_msg("Failed to create temporary output file.", Some(e));
         EX_CANTCREAT
     })?;
 
+    let mut md5 = md5::Md5::new();
+    let bytes_per_sample = (stream_info.bits_per_sample() + 7) / 8;
     for n in 0..frame_count {
-        let frame_signal = stream.frame(n).unwrap().decode();
+        let frame = stream.frame(n).unwrap();
+        let frame_signal = frame.decode();
         for v in &frame_signal {
             let v = *v;
             writer.write_sample(v).map_err(|e| {
                 display::show_error_msg("Failed to write decoded samples.", Some(e));
                 EX_CANTCREAT
             })?;
+            md5.update(&v.to_le_bytes()[0..bytes_per_sample]);
         }
     }
+    let computed_digest: [u8; 16] = md5.finalize().into();
+    let stored_digest = stream_info.md5_digest();
+    if stored_digest != &[0x00; 16] && stored_digest != &computed_digest {
+        display::show_error_msg::<std::convert::Infallible>("MD5 digests did not match.", None);
+        return Err(EX_CANTCREAT);
+    }
+
+    std::fs::rename(temp_wav_file.path(), &args.output).map_err(|e| {
+        display::show_error_msg("Failed to write to the output path.", Some(e));
+        EX_CANTCREAT
+    })?;
+
     let decode_time = decoder_start.elapsed();
     let _ = display::show_progress(
         &args,
@@ -516,6 +534,10 @@ mod tests {
             hound::WavReader::open(expected).expect("expected file should be openable.");
         let mut reader_actual =
             hound::WavReader::open(actual).expect("actual file should be openable.");
+
+        assert_eq!(reader_expected.spec(), reader_actual.spec());
+        assert_eq!(reader_expected.len(), reader_actual.len());
+
         for (n, (e, a)) in reader_expected
             .samples()
             .zip(reader_actual.samples())
@@ -525,25 +547,55 @@ mod tests {
             let a: i32 = a.expect("sample from actual file  should be readable");
             assert_eq!(
                 e, a,
-                "expected and actual samples should be identical (sample-offset={n}."
+                "expected and actual samples should be identical (sample-offset={n})."
             );
         }
     }
 
     #[rstest]
-    #[case(SourceConfig { duration_secs: 3, sample_rate: 44100, channels: 2, bits_per_sample: 16})]
-    #[case(SourceConfig { duration_secs: 3, sample_rate: 44097, channels: 2, bits_per_sample: 16})]
-    #[case(SourceConfig { duration_secs: 3, sample_rate: 44100, channels: 1, bits_per_sample: 16})]
-    #[case(SourceConfig { duration_secs: 3, sample_rate: 44100, channels: 3, bits_per_sample: 16})]
-    fn integration_encoder_decoder(#[case] source_config: SourceConfig) {
+    #[case(
+        "canonical", 
+        SourceConfig { duration_secs: 3, sample_rate: 44100, channels: 2, bits_per_sample: 16 }
+    )]
+    #[case(
+        "sr44097",
+        SourceConfig { duration_secs: 3, sample_rate: 44097, channels: 2, bits_per_sample: 16 }
+    )]
+    #[case(
+        "ch1",
+        SourceConfig { duration_secs: 3, sample_rate: 44100, channels: 1, bits_per_sample: 16 }
+    )]
+    #[case(
+        "ch3",
+        SourceConfig { duration_secs: 3, sample_rate: 44100, channels: 3, bits_per_sample: 16 }
+    )]
+    #[case(
+        "bps24",
+        SourceConfig { duration_secs: 3, sample_rate: 44100, channels: 2, bits_per_sample: 24 }
+    )]
+    #[case(
+        "bps8",
+        SourceConfig { duration_secs: 3, sample_rate: 44100, channels: 1, bits_per_sample: 8 }
+    )]
+    fn integration_encoder_decoder(#[case] case_name: &str, #[case] source_config: SourceConfig) {
         let tmpdir = tempfile::tempdir().unwrap();
-        let mut config_dump_path = tmpdir.as_ref().to_path_buf();
+        let tmpdir_path = std::env::var_os("FLACENC_TEST_WORKDIR").map_or_else(
+            || tmpdir.as_ref().to_path_buf(),
+            |dir| {
+                eprintln!("FLACENC_TEST_WORKDIR is set to {}", dir.to_string_lossy());
+                let mut workdir = std::path::PathBuf::from(dir);
+                workdir.push(case_name);
+                std::fs::create_dir_all(&workdir).expect("Work dir creation should not fail.");
+                workdir
+            },
+        );
+        let mut config_dump_path = tmpdir_path.clone();
         config_dump_path.push("dumped.toml");
-        let mut flac_path = tmpdir.as_ref().to_path_buf();
+        let mut flac_path = tmpdir_path.clone();
         flac_path.push("compressed.flac");
-        let mut source_path = tmpdir.as_ref().to_path_buf();
+        let mut source_path = tmpdir_path.clone();
         source_path.push("source.wav");
-        let mut decoded_path = tmpdir.as_ref().to_path_buf();
+        let mut decoded_path = tmpdir_path;
         decoded_path.push("decoded.wav");
 
         generate_test_wav(&source_path, &source_config);

flacenc-bin/src/source.rs

@@ -115,6 +115,12 @@ impl Source for HoundSource {
             .map_err(SourceError::from_io_error)?;
 
         self.current_offset += to_read;
+        if self.bytes_per_sample == 1 {
+            // 8-bit wav is not in two's complement, so convert it first
+            self.bytebuf.iter_mut().for_each(|p| {
+                *p = (i32::from(*p) - 128).to_le_bytes()[0];
+            });
+        }
         dest.fill_le_bytes(&self.bytebuf, self.bytes_per_sample)?;
 
         Ok(read_bytes / self.channels() / self.bytes_per_sample)

src/arrayutils.rs

@@ -368,7 +368,7 @@ pub fn le_bytes_to_i32s(bytes: &[u8], dest: &mut [i32], bytes_per_sample: usize)
 /// Converts i32s to little-endian bytes.
 ///
 /// NOTE: Currenty, this function is not used in "flacenc-bin", and therefore
-/// this might be slow.
+/// the performance is not checked recently, might be too slow.
 #[allow(dead_code)] // only used in unittests with `cfg(features = "serde")`.
 pub fn i32s_to_le_bytes(ints: &[i32], dest: &mut [u8], bytes_per_sample: usize) {
     let mut n = 0;
@@ -741,6 +741,14 @@ mod tests {
         assert_eq!(dest, [0x12_3456, 0x13_579B, -1, 0x24_68AC]);
     }
 
+    #[test]
+    fn convert_bytes_to_i8s() {
+        let bytes = [0x56, 0x34, 0x12, 0x9B, 0x80, 0x13, 0xFF, 0x68];
+        let mut dest = [0i32; 8];
+        le_bytes_to_i32s(&bytes, &mut dest, 1);
+        assert_eq!(dest, [0x56, 0x34, 0x12, -0x65, -0x80, 0x13, -0x01, 0x68]);
+    }
+
     #[cfg(feature = "simd-nightly")]
     #[test]
     fn convert_le16_bytes_to_ints() {

src/component/parser.rs

@@ -421,7 +421,7 @@ pub fn subframe<'a, E>(
 where
     E: ParseError<BitInput<'a>>,
 {
-    debug_assert!(bits_per_sample <= MAX_BITS_PER_SAMPLE);
+    debug_assert!(bits_per_sample <= MAX_BITS_PER_SAMPLE + 1);
     alt((
         into(constant::<E>(block_size, bits_per_sample)),
         into(fixed_lpc::<E>(block_size, bits_per_sample)),
@@ -456,7 +456,7 @@ pub fn constant<'a, E>(
 where
     E: ParseError<BitInput<'a>>,
 {
-    debug_assert!(bits_per_sample <= MAX_BITS_PER_SAMPLE);
+    debug_assert!(bits_per_sample <= MAX_BITS_PER_SAMPLE + 1);
     move |input| {
         let remaining_input = input;
         let (remaining_input, (typetag, _wasted_flag)) = subframe_header(remaining_input)?;
@@ -493,7 +493,7 @@ pub fn fixed_lpc<'a, E>(
 where
     E: ParseError<BitInput<'a>>,
 {
-    debug_assert!(bits_per_sample <= MAX_BITS_PER_SAMPLE);
+    debug_assert!(bits_per_sample <= MAX_BITS_PER_SAMPLE + 1);
     move |input| {
         let remaining_input = input;
         let (remaining_input, (typetag, _wasted_flag)) = subframe_header(remaining_input)?;
@@ -532,7 +532,7 @@ pub fn lpc<'a, E>(
 where
     E: ParseError<BitInput<'a>>,
 {
-    debug_assert!(bits_per_sample <= MAX_BITS_PER_SAMPLE);
+    debug_assert!(bits_per_sample <= MAX_BITS_PER_SAMPLE + 1);
     move |input| {
         let remaining_input = input;
         let (remaining_input, (typetag, _wasted_flag)) = subframe_header(remaining_input)?;
@@ -598,7 +598,7 @@ pub fn verbatim<'a, E>(
 where
     E: ParseError<BitInput<'a>>,
 {
-    debug_assert!(bits_per_sample <= MAX_BITS_PER_SAMPLE);
+    debug_assert!(bits_per_sample <= MAX_BITS_PER_SAMPLE + 1);
     move |input| {
         let remaining_input = input;
         let (remaining_input, (typetag, _wasted_flag)) = subframe_header(remaining_input)?;
@@ -703,7 +703,7 @@ fn raw_samples<'a, E>(
 where
     E: ParseError<BitInput<'a>>,
 {
-    debug_assert!(bits_per_sample <= MAX_BITS_PER_SAMPLE);
+    debug_assert!(bits_per_sample <= MAX_BITS_PER_SAMPLE + 1);
     move |input| {
         let mut remaining_input = input;
         let mut data = Vec::with_capacity(size);