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

Fixed Wave file writer writing wrong header values #1281

Merged
merged 1 commit into from Oct 12, 2017

Conversation

MarioLiebisch
Copy link
Member

@MarioLiebisch MarioLiebisch commented Sep 9, 2017

Fixes #1280.

Previously when updating the header fields, SFML assumed the number of samples written would be the number of samples per channel, which wasn't the case. Therefore for stereo files the written file length was actually twice the correct value. This fix uses the file size written as a basis, no longer counting the samples written altogether.

@MarioLiebisch MarioLiebisch added this to the 2.5 milestone Sep 9, 2017
@MarioLiebisch MarioLiebisch self-assigned this Sep 9, 2017
@eXpl0it3r eXpl0it3r added this to Review & Testing in SFML 2.5.0 Sep 25, 2017
Copy link
Member

@LaurentGomila LaurentGomila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@@ -191,8 +188,9 @@ void SoundFileWriterWav::close()
m_file.flush();

// Update the main chunk size and data sub-chunk size
Uint32 dataChunkSize = static_cast<Uint32>(m_sampleCount * m_channelCount * 2);
Uint32 mainChunkSize = dataChunkSize + 36;
Uint32 fileSize = m_file.tellp();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No compiler warning about type conversion here?

Copy link
Member

@eXpl0it3r eXpl0it3r Sep 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our VS CI builds have warnings for it.

src\SFML\Audio\SoundFileWriterWav.cpp(191) : warning C4244: 'initializing' : conversion from 'std::streamoff' to 'sf::Uint32', possible loss of data

@tgnottingham
Copy link
Contributor

Look good, but I think you can remove m_channelCount as well.

@MarioLiebisch
Copy link
Member Author

You're right, didn't notice!

@eXpl0it3r eXpl0it3r moved this from Review & Testing to Ready in SFML 2.5.0 Oct 11, 2017
Previously when updating the header fields, SFML assumed the number of
samples written would be the number of samples per channel, which wasn't
the case. Therefore for stereo files the written file length was actually
twice the correct value. This fix uses the file size written as a basis,
no longer counting the samples written alltogether.

This fixes issue #1280.
@eXpl0it3r eXpl0it3r merged commit b3d6e48 into master Oct 12, 2017
@eXpl0it3r eXpl0it3r deleted the bugfix/wav-writer-size branch October 12, 2017 18:03
@eXpl0it3r eXpl0it3r moved this from Ready to Merged / Superseded in SFML 2.5.0 Oct 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
SFML 2.5.0
  
Merged / Superseded
Development

Successfully merging this pull request may close these issues.

error in the SoundFileWriterWav :: close () method
4 participants