kometbomb / klystrack

A chiptune tracker
http://kometbomb.github.io/klystrack/
Other
485 stars 28 forks source link

export wav produces an erroneous file #293

Closed farvardin closed 4 years ago

farvardin commented 4 years ago

I've compiled the latest klystrack version (using the howto there: https://github.com/kometbomb/klystrack/wiki/HowToCompile) and while everything seems to work fine, when I export to wav, the resulting file is not correct. I can play it with VLC, but sonic-visualiser and audacity report this as an empty file.

When trying to convert it to flac, the flac command returns: ERROR: 'data' chunk has size of 0

But I can export wav for the same track from klystrack 1.7.2 for example.

I've tried with a fresh new project, it 's the same export problem.

kometbomb commented 4 years ago

It probably doesn't update the header with the correct data size. I did check with Audacity back when I added the feature so need to check what's happening as I don't think there are any changes since that version.

kometbomb commented 4 years ago

There of course is a change in 99635da7e1051d6baafb502989b7825fd47d2bd7. Successful exports do not call ww_finish so the header is not updated with the sizes and they remain zero. export.c:132 exits the function and only the abort branch will wrap up the wave write.

Hey, @nalquas, is this something that you checked that would cause the crash you fixed - if w_finish is called in both cases (it of course should be called as above)?

nalquas commented 4 years ago

Ah, I didn't know the code under the abort goto label was supposed to be run even when successful.

I've changed the code in a way that allows that code to run (including ww_finish), but that made the crash reappear. I think that crash happens because the file is closed inside of ww_finish (fclose(ww->file);) AND after export_wav returns (if(export_wav(&mused.song, mused.mus.cyd->wavetable_entries, f, -1)) fclose(f);).

I'll make some modifications and see if that's the issue.

nalquas commented 4 years ago

@farvardin I can confirm the issue you wrote about, Audacity seems to think files exported with the latest master are empty.

The fix I just pull-requested seems to make Audacity see the file content again, at least on my machine.

kometbomb commented 4 years ago

Can you check if it now works as expected?

farvardin commented 4 years ago

@kometbomb @nalquas Yes, I've compiled klystrack with the latest commit, and I can confirm that it's working correctly now, both in audacity and with the flac converter, the wav is ok now!

Thank you for the quick fix!

kometbomb commented 4 years ago

Thanks for confirming and thanks for fixing it, @nalquas!

ma 11. toukok. 2020 klo 10.30 farvardin (notifications@github.com) kirjoitti:

@kometbomb https://github.com/kometbomb @nalquas https://github.com/nalquas Yes, I've compiled klystrack with the latest commit, and I can confirm that it's working correctly now, both in audacity and with the flac converter, the wav is ok now!

Thank you for the quick fix!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kometbomb/klystrack/issues/293#issuecomment-626523961, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIIV2PP3RX3BUA5GBIOVX3RQ6SRRANCNFSM4M424PPQ .