rochars / wavefile

Create, read and write wav files according to the specs. :star: :notes: :heart:
MIT License
230 stars 48 forks source link

mulaw extensible riff fmt header #21

Open RickeyWard opened 4 years ago

RickeyWard commented 4 years ago

I can't find any source stating that mu-law should require an extensible fmt header to to include the validbitspersample. including this in the header breaks several players, including windows media player and groove music. ffmpeg and soundforge do not put this extensible header in.

Is there some reason why it should be there?

RickeyWard commented 4 years ago

I've found this http://www-mmsp.ece.mcgill.ca/Documents/AudioFormats/WAVE/WAVE.html

Non-PCM Formats An extended Format chunk is used for non-PCM data. The cbSize field gives the size of the extension.

For all formats other than PCM, the Format chunk must have an extended portion. The extension can be of zero length, but the size field (with value 0) must be present. For float data, full scale is 1. The bits/sample would normally be 32 or 64. For the log-PCM formats (µ-law and A-law), the Rev. 3 documentation indicates that the bits/sample field (wBitsPerSample) should be set to 8 bits. The non-PCM formats must have a fact chunk.

but no software seems to follow it, sound forge, audacity, and ff-mpeg all write files without wBitsPerSample, and including it seems to break playback compatibility.

kookster commented 4 years ago

@rochars lists mu-law test files from here: http://mauvecloud.net/sounds/index.html

When I parse a mu-law file from there, http://mauvecloud.net/sounds/ulaw44s.wav , in wavefile, I get this fmt chunk:

  fmt:
   { chunkId: 'fmt ',
     chunkSize: 18,
     audioFormat: 7,
     numChannels: 2,
     sampleRate: 44100,
     byteRate: 88200,
     blockAlign: 2,
     bitsPerSample: 8,
     cbSize: 0,
     validBitsPerSample: 0,
     dwChannelMask: 0,
     subformat: [],
     headLayer: 0,
     headBitRate: 0,
     headMode: 0,
     headModeExt: 0,
     headEmphasis: 0,
     headFlags: 0,
     ptsLow: 0,
     ptsHigh: 0 },

So that seems to support your point - there's no validBitsPerSample set.

kookster commented 4 years ago

Also, from that same mcgill site: http://www-mmsp.ece.mcgill.ca/Documents/AudioFormats/WAVE/Samples.html

They list 2 versions of the sample files, those with WAVE_FORMAT_EXTENSIBLE and those without, and notes the ones with don't work for playback in Windows Media Player (and quite likely lots of other players).

So it seems like the WAVE_FORMAT_EXTENSIBLE bits should at least be optional, to be able to make the most compatible wav files.

Taking a look at the samples, I see the regular one http://www-mmsp.ece.mcgill.ca/Documents/AudioFormats/WAVE/Samples/AFsp/M1F1-mulaw-AFsp.wav doesn't set the bits:

  fmt:
   { chunkId: 'fmt ',
     chunkSize: 18,
     audioFormat: 7,
     numChannels: 2,
     sampleRate: 8000,
     byteRate: 16000,
     blockAlign: 2,
     bitsPerSample: 8,
     cbSize: 0,
     validBitsPerSample: 0,
     dwChannelMask: 0,
     subformat: [],
     headLayer: 0,
     headBitRate: 0,
     headMode: 0,
     headModeExt: 0,
     headEmphasis: 0,
     headFlags: 0,
     ptsLow: 0,
     ptsHigh: 0 },

The one with WAVE_FORMAT_EXTENSIBLE http://www-mmsp.ece.mcgill.ca/Documents/AudioFormats/WAVE/Samples/AFsp/M1F1-mulawWE-AFsp.wav does have the bits:

  fmt:
   { chunkId: 'fmt ',
     chunkSize: 40,
     audioFormat: 65534,
     numChannels: 2,
     sampleRate: 8000,
     byteRate: 16000,
     blockAlign: 2,
     bitsPerSample: 8,
     cbSize: 22,
     validBitsPerSample: 8,
     dwChannelMask: 3,
     subformat: [ 7, 1048576, 2852126848, 1905997824 ],
     headLayer: 0,
     headBitRate: 0,
     headMode: 0,
     headModeExt: 0,
     headEmphasis: 0,
     headFlags: 0,
     ptsLow: 0,
     ptsHigh: 0 },
kookster commented 4 years ago

I see @RickeyWard you commented this out in your fork - I wonder if there's a way to make it optional rather than always getting rid of them - though, to your point, why ever set the extensible fmt if they won't work anywhere...

RickeyWard commented 4 years ago

I see @RickeyWard you commented this out in your fork - I wonder if there's a way to make it optional rather than always getting rid of them - though, to your point, why ever set the extensible fmt if they won't work anywhere...

Other software doesn't seem to set it, the spec says that cbSize has to exist, but not that any fields need to be used. As far as I can tell, everything seems to be able to read it when validBitsPerSample isn't set, but a lot of stuff can't when it is.

I certainly think it should be there if it's "suppose" to be, But for my purposes the files that are produced are useless to me unless I remove it. I've been working with mulaw files for 5 years professionally as they are used in telephony systems and I've never seen that in the fmt chunk before. When I noticed this issue using this lib I found as many wav files from as many projects and customers as I could find and took a sampling of them and checked the fmt chunks with riffpad and none of them had it set.

kookster commented 4 years ago

that's amazingly thorough. I work in radio/podcasting, and I don't run into mu-law.

Yeah, that is very convincing. It seems like the default should be to never set these for mu-law, with some way to set them if someone really wants to.

I don't know him at all, but I worry about @rochars given his messages on this repo, and then no further messages or commits. I wanted to at least provide some dialogue on this issue as I too am using and enhancing wavefile and I hope that it gets further adoption and community support.

rochars commented 4 years ago

I included the header because it seemed like the right thing to do (and it worked on my tests / players), but this part of the header should be optional, really.

Unfortunately, I am not able to work on this software at the moment - the code will remain public for anyone interested in continuing to work with this library, as well as anyone interested in developing and releasing new versions of it.

Thank you all for your research on this topic. I wish you all the best.