Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mulaw extensible riff fmt header #21

Open
RickeyWard opened this issue May 22, 2020 · 7 comments
Open

mulaw extensible riff fmt header #21

RickeyWard opened this issue May 22, 2020 · 7 comments

Comments

@RickeyWard
Copy link

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
Copy link
Author

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
Copy link

kookster commented May 27, 2020

@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
Copy link

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
Copy link

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
Copy link
Author

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
Copy link

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
Copy link
Owner

rochars commented Sep 2, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants