pdeljanov / Symphonia

Pure Rust multimedia format demuxing, tag reading, and audio decoding library
Mozilla Public License 2.0
2.28k stars 131 forks source link

Issue parsing WAVE file: "error: malformed steam: wav: malformed fmt_ieee chunk" #247

Closed mxw-meta closed 7 months ago

mxw-meta commented 8 months ago

I'm experiencing an error when trying to load a WAV file (OBJ_LaserCutter_Collision_Loop_On_01.wav.zip) due to an error: malformed steam: wav: malformed fmt_ieee chunk error.

🔴 This is the header of the file that is failing (metadata suggests it's exported from ProTools):

{
  chunkId: 'fmt ',
  chunkSize: 40,
  audioFormat: 3,
  numChannels: 2,
  sampleRate: 48000,
  byteRate: 384000,
  blockAlign: 8,
  bitsPerSample: 32,
  cbSize: 0,
  validBitsPerSample: 0,
  dwChannelMask: 0,
  subformat: [ 0, 0, 0, 0 ]
}

✅ Upon re-exporting the file, the issue is gone. Here's what the header looks like after re-exporting:

{
  chunkId: 'fmt ',
  chunkSize: 16,
  audioFormat: 3,
  numChannels: 2,
  sampleRate: 48000,
  byteRate: 384000,
  blockAlign: 8,
  bitsPerSample: 32,
  cbSize: 0,
  validBitsPerSample: 0,
  dwChannelMask: 0,
  subformat: []
}

They only differ in the chunkSize and subformat fields. The chunkSize appears to be valid for a WAVE file, from what I checked.

So I'm not sure what could be causing this right now. Other DAWs (Audacity, Ableton) have no issues processing the file - so I'm quite certain this is not expected behavior. What do others think?

Please let me know if any further information is required!

dedobbin commented 8 months ago

I'll take a look!

dedobbin commented 8 months ago

The file is rejected because of the FMT chunk size is off yeah. The size reported (40) seems the correct value. I'm curious what the additional data contains? According to the spec it should not be there. Perhaps a Pro-Tools thing.

By just removing the check (symphonia-format-wav/src/chunks.rs::381) it plays correctly. Since the chunk size 40 is properly used it doesn't give any trouble. It also passed symphonia-check. Other test files run without regression.

I say we just turn it into a warning instead of rejecting the file, are you ok with that @pdeljanov

pdeljanov commented 8 months ago

The file is rejected because of the FMT chunk size is off yeah. The size reported (40) seems the correct value. I'm curious what the additional data contains? According to the spec it should not be there. Perhaps a Pro-Tools thing.

By just removing the check (symphonia-format-wav/src/chunks.rs::381) it plays correctly. Since the chunk size 40 is properly used it doesn't give any trouble. It also passed symphonia-check. Other test files run without regression.

I say we just turn it into a warning instead of rejecting the file, are you ok with that @pdeljanov

It looks like the encoder wrote a WAVEFORMATEXTENSIBLE structure in the format chunk, but treated it as a WAVEFORMATEX.

The former is 40 bytes long, while the latter may be 16 or 18 bytes long.

This is in violation of the spec in multiple ways, but we can workaround it in the same way as read_fmt_pcm by skipping over the extra bytes (see here). If the chunk is not 16, 18, or 40 bytes long, then it should still be an error.

It is not sufficient to simply delete the warning because there are still extra bytes present in the file. By ignoring them the reader becomes de-synchronized (test it with RUST_LOG=trace and notice the warnings). It just so happens that the smallest size for a chunk is 8 bytes, so after 3 failed chunk iterations it re-synchronizes properly (40 byte format chunk - 16 bytes already read = 24 extra bytes).

dedobbin commented 7 months ago

The file is rejected because of the FMT chunk size is off yeah. The size reported (40) seems the correct value. I'm curious what the additional data contains? According to the spec it should not be there. Perhaps a Pro-Tools thing. By just removing the check (symphonia-format-wav/src/chunks.rs::381) it plays correctly. Since the chunk size 40 is properly used it doesn't give any trouble. It also passed symphonia-check. Other test files run without regression. I say we just turn it into a warning instead of rejecting the file, are you ok with that @pdeljanov

It looks like the encoder wrote a WAVEFORMATEXTENSIBLE structure in the format chunk, but treated it as a WAVEFORMATEX.

The former is 40 bytes long, while the latter may be 16 or 18 bytes long.

This is in violation of the spec in multiple ways, but we can workaround it in the same way as read_fmt_pcm by skipping over the extra bytes (see here). If the chunk is not 16, 18, or 40 bytes long, then it should still be an error.

It is not sufficient to simply delete the warning because there are still extra bytes present in the file. By ignoring them the reader becomes de-synchronized (test it with RUST_LOG=trace and notice the warnings). It just so happens that the smallest size for a chunk is 8 bytes, so after 3 failed chunk iterations it re-synchronizes properly (40 byte format chunk - 16 bytes already read = 24 extra bytes).

Ah! Sorry for my wrong assumption, i could have checked if it actually stayed in sync. Since it passed everything i assumed it did. I'll work on a proper fix!

pdeljanov commented 7 months ago

The file is rejected because of the FMT chunk size is off yeah. The size reported (40) seems the correct value. I'm curious what the additional data contains? According to the spec it should not be there. Perhaps a Pro-Tools thing. By just removing the check (symphonia-format-wav/src/chunks.rs::381) it plays correctly. Since the chunk size 40 is properly used it doesn't give any trouble. It also passed symphonia-check. Other test files run without regression. I say we just turn it into a warning instead of rejecting the file, are you ok with that @pdeljanov

It looks like the encoder wrote a WAVEFORMATEXTENSIBLE structure in the format chunk, but treated it as a WAVEFORMATEX. The former is 40 bytes long, while the latter may be 16 or 18 bytes long. This is in violation of the spec in multiple ways, but we can workaround it in the same way as read_fmt_pcm by skipping over the extra bytes (see here). If the chunk is not 16, 18, or 40 bytes long, then it should still be an error. It is not sufficient to simply delete the warning because there are still extra bytes present in the file. By ignoring them the reader becomes de-synchronized (test it with RUST_LOG=trace and notice the warnings). It just so happens that the smallest size for a chunk is 8 bytes, so after 3 failed chunk iterations it re-synchronizes properly (40 byte format chunk - 16 bytes already read = 24 extra bytes).

Ah! Sorry for my wrong assumption, i could have checked if it actually stayed in sync. Since it passed everything i assumed it did. I'll work on a proper fix!

Thanks!

mxw-meta commented 7 months ago

Awesome turnaround! Really appreciate the work.