Added WAV capturing option #49
Conversation
Added option to capture output into a wav file
Moved `wav_file` variable to the only file that uses is and fixes a compiler issue.
src/ebmusv2.h
Outdated
| BOOL is_playing(); | ||
| BOOL start_playing(); | ||
| void stop_playing(); | ||
| BOOL is_capturing_audio(); | ||
| BOOL start_capturing_audio(); | ||
| void stop_capturing_audio(); |
There was a problem hiding this comment.
Add prototypes for these (argument list of (void) instead of ())
(Might also want to do the same for the definitions of the functions, but here matters the most.)
src/main.c
Outdated
| if (current_tab == 1) { | ||
| stop_capturing_audio(); | ||
| } else { |
There was a problem hiding this comment.
May want to add a comment here explaining that 1 means the instrument tab and why we don't want to stop_playing() or enable the play button. Same goes for capturing audio below
src/sound.c
Outdated
| if (file) | ||
| wav_file = fopen(file, "wb"); | ||
|
|
||
| if (wav_file) { |
There was a problem hiding this comment.
This seems like it could behave weirdly if wav_file was somehow initialized prior to calling this function, because it will start capturing audio even if the user cancels the dialogue. Maybe it would be a good idea to add an assert or a less violent alternative to check that, somewhere? Like after line 75, or so. I dunno.
Also, the code as it currently stands doesn't have much error handling, and a lot of things can go wrong that are relatively simple to catch and test, so it would be good to notify the user of errors if wav_file is null or the initial writes to the file fail before they listen to the song play for 3 minutes.
src/sound.c
Outdated
| DWORD size_placeholder = 0; | ||
| DWORD format_header_size = 16; | ||
| WORD formatTag = WAVE_FORMAT_PCM; | ||
| WORD num_channels = 2; | ||
| DWORD sample_rate = mixrate; | ||
| DWORD avg_byte_rate = mixrate*4; | ||
| WORD block_alignment = 4; | ||
| WORD bit_depth = 16; | ||
|
|
||
| fputs("RIFF", wav_file); | ||
| fwrite(&size_placeholder, sizeof size_placeholder, 1, wav_file); | ||
| fputs("WAVE", wav_file); | ||
| fputs("fmt ", wav_file); | ||
| fwrite(&format_header_size, sizeof format_header_size, 1, wav_file); | ||
| fwrite(&formatTag, sizeof formatTag, 1, wav_file); | ||
| fwrite(&num_channels, sizeof num_channels, 1, wav_file); | ||
| fwrite(&sample_rate, sizeof sample_rate, 1, wav_file); | ||
| fwrite(&avg_byte_rate, sizeof avg_byte_rate, 1, wav_file); | ||
| fwrite(&block_alignment, sizeof block_alignment, 1, wav_file); | ||
| fwrite(&bit_depth, sizeof bit_depth, 1, wav_file); | ||
| fputs("data", wav_file); | ||
| fwrite(&size_placeholder, sizeof size_placeholder, 1, wav_file); |
There was a problem hiding this comment.
Probably a good idea to document the assumption that WAV files expect these fields to all be little endian and that we're on a little-endian architecture.
Thoughts about making a single struct for the WAV header and just passing that to a single fwrite call? I don't know if there's any objective benefit to it, besides maybe simplified error handling if we want to check how many bytes were written.
src/main.c
Outdated
| @@ -593,6 +592,18 @@ BOOL save_all_packs() { | |||
| return success; | |||
| } | |||
|
|
|||
| BOOL validate_playable() { | |||
There was a problem hiding this comment.
| BOOL validate_playable() { | |
| static BOOL validate_playable(void) { |
The parameter list thing doesn't really fit the style of the rest of the file, but since there's also no declaration in ebmusv2.h, it's still a good idea to add the proper parameter list here, right?
| @@ -45,6 +45,7 @@ extern HWND tab_hwnd[NUM_TABS]; | |||
| #define hwndInstruments tab_hwnd[1] | |||
| #define hwndEditor tab_hwnd[2] | |||
| #define hwndPackList tab_hwnd[3] | |||
| char *open_dialog(BOOL (WINAPI *func)(LPOPENFILENAME), char *filter, char *extension, DWORD flags); | |||
There was a problem hiding this comment.
How terrible of an idea is it to include <commdlg.h> here, instead of just in sound.c and main.c? This declaration adds a lot of warning noise, at least with the compiler flags I have CodeBlocks set to, because it thinks LPOPENFILENAME is a parameter name and not a type.
src/resource.rc
Outdated
| @@ -51,7 +51,8 @@ BEGIN | |||
| POPUP "&Song" | |||
| BEGIN | |||
| MENUITEM "&Play\tCtrl+P", ID_PLAY | |||
| MENUITEM "&Stop\tEsc", ID_STOP | |||
| MENUITEM "&Stop\tEsc", ID_STOP, GRAYED | |||
| MENUITEM "Capture &Audio", ID_CAPTURE_AUDIO | |||
There was a problem hiding this comment.
| MENUITEM "Capture &Audio", ID_CAPTURE_AUDIO | |
| MENUITEM "Capture &Audio...", ID_CAPTURE_AUDIO |
Since it opens a separate dialog to open a file. (Ditto for the instance in stop_capturing_audio)
src/sound.c
Outdated
| fseek(wav_file, 4, 0); | ||
| fwrite(&size, sizeof size, 1, wav_file); | ||
|
|
||
| fseek(wav_file, 40, 0); |
There was a problem hiding this comment.
Use SEEK_SET instead of hardcoding 0
Added option to capture output into a wav file