FLAC output and assorted wrsamp improvements #420
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 10 commits
September 2, 2022 17:07
When writing to an output file, it may be necessary to transform the data in various ways, which is often most efficient to do by modifying the array in-place. However, for API cleanliness, the wr_dat_file function shouldn't modify its argument. Instead, the function itself should make a copy of the array if necessary. (In the future, we may ultimately be able to avoid a copy if the array is already in the correct format.)
The d_signal argument is always a 2D array, and is only used if expanded is false. The e_d_signal argument is always a list of 1D arrays, and is only used if expanded is true.
Previously the variable "n_sig" was used confusingly to mean different things at different points in the function. Instead, set these variables consistently: n_sig is the number of signals, samps_per_frame is the number of samples per frame of each signal, and tsamps_per_frame is the sum of samps_per_frame (which is also the number of columns of the reshaped d_signal array.)
If expanded is true, then e_d_signal and samps_per_frame must have the same length (one entry in e_d_signal corresponds to one entry in samps_per_frame). The length of each channel must be a multiple of the number of samples per frame, and every channel must have the same number of frames. Add sanity checks to verify that the values provided by the caller make sense.
Writing FLAC formats, like reading FLAC formats, is done using the soundfile package, which is currently treated as an optional dependency (since on some systems it requires an external copy of libsndfile.) This code performs, in effect, the inverse of _rd_compressed_file. As with _rd_compressed_file, this has room to be optimized somewhat.
It is permitted and not unheard of for signal name strings to contain space characters. This should be allowed, provided that the string does not start or end with a space. All control characters should be disallowed.
It may be necessary to use multiple signal files in various cases: - signals are to be written in different formats (e.g. 8-bit signals stored in format 80 while 12-bit signals are stored in format 212) - multi-frequency signals are to be written in compressed format (FLAC supports only one frequency per signal file) - more than 8 signals are to be written in compressed format (FLAC supports a maximum of 8 channels) In each of these cases, if the caller didn't specify signal file names explicitly, generate signal file files by adding a numeric suffix after the record name.
When writing multiple signal files in expanded format, the samps_per_frame passed to wr_dat_file must be the list of samples-per-frame *for that particular dat file* (corresponding to the e_d_signal argument), not the samples-per-frame of the whole record.
bemoody
force-pushed
the
wrsamp-and-flac
branch
from
a161ed7 to
b0a3d86
Compare
bemoody
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is another mixed bag of changes to
wfdb.wrsampandRecord.wrsamp.The main goal here is to allow writing compressed (FLAC) signal files.
Just as with reading signal files, this requires rearranging the input data (
d_signalore_d_signal) into the format required by the soundfile package. Just as with reading signal files, this is not done as efficiently as it could be (in fact, none of the formats inwr_dat_fileare implemented as efficiently as they could be.)It's worth noting that
soundfiledoesn't provide a way to set the compression level or other parameters. Setting the compression level, at least, should be easy to implement if somebody wanted to. Thelibsndfiledefault appears to be level 5 (the same default aslibwfdband theflaccommand-line tool.)When writing a record in FLAC format, it may be necessary to use multiple signal files (either because there are more than 8 channels or there are multiple sampling frequencies.)
wfdb.wrsampdoesn't provide a way to set the signal file names (which is likely a good thing; we don't need to over-complicate this API.) On the other hand,wfdb.wrsampdoes accept a list of file formats, even though it will fail if you try to use different formats for one signal file. So here I have extended it to generate multiple signal files (record_1.dat,record_2.dat, etc.) if the formats require it.In order to test reading and writing the sample record "flacformats", it was also necessary to fix another long-standing issue (
check_fieldrejecting signal names containing spaces.)Additionally,
Record.wrsampwas broken for the case of expanded (multi-frequency) format and multiple signal files.wr_dat_filewill now sanity-check its arguments to try to catch such issues.