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

Wave Header Incorrect #43

Merged
merged 1 commit into from
Dec 18, 2019
Merged

Wave Header Incorrect #43

merged 1 commit into from
Dec 18, 2019

Conversation

mbmleone
Copy link
Contributor

First of all many thanks for this great module.

When trying to use it we discovered an issue with .wav-file that was generated.
Android refused to play the file.
After some comparisons we found the header of generated files are incorrect.
While a some music players accept that, others like on Android do not.

The documentation we found shows that file length is calculated like this:
"Size of the overall file - 8 bytes, in bytes (32-bit integer). "
So that means that line 48 of AudioFunctions should be:
writer.Write (audioLength - 1);

Then the data length is filelength minus the header.
And line 74:
writer.Write (audioLength-44);

Best regards,
Marco

@NateRickard
Copy link
Owner

This very well could be the case... most of that code I adapted from the public domain. Could you point me to the docs you mention above?

@mbmleone
Copy link
Contributor Author

That's described on many pages, but this is one of them:
https://wiki.fileformat.com/audio/wav/

@NateRickard
Copy link
Owner

Based on that spec it seems to already be compliant:

audioLength is the data length, not the file length. It's the byte count of the captured audio data.

5 - 8 | File size (integer) | Size of the overall file - 8 bytes, in bytes (32-bit integer). Typically, you'd fill this in after creation.

The overall file length would be audioLength + header length (44 bytes). The 36 being used now is 44 - 8 bytes. So:

Size of the overall file - 8 bytes == audioLength + 44 - 8

Also, since audioLength is a byte count, and they want this value in bytes, we stick in that unit. audioLength - 1 would in essence just be subtracting 1 byte from the total count.

41-44 | File size (data) | Size of the data section.

Since audioLength is the data length (and not file length), no need to subtract 44 for the header.

How were you having issues playing the files? The built in audio player uses the MediaPlayer API on Android and I've never seen issues there.

@mbmleone
Copy link
Contributor Author

mbmleone commented Dec 18, 2019

The Android default media players simply stated they couldn't load the file.
Also the MediaPlayer API couldn't play it normally.

Please check the WriteWavHeader () method.
The header code overwrites the first 44 bytes to store the header.
So it's not adding 44 bytes to the file length.

Line 48 I gave was in the description was incorrect.
The source changes contain the correct change:
writer.Write ((Int32)(audioLength - 8));

@NateRickard
Copy link
Owner

So weird... I've never seen issues with MediaPlayer in my testing. But great catch! audioLength actually IS the file length here, so my basic assumption was wrong.

@NateRickard NateRickard merged commit 01e3b76 into NateRickard:master Dec 18, 2019
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

Successfully merging this pull request may close these issues.

2 participants