-
Notifications
You must be signed in to change notification settings - Fork 2k
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
lib/formats/flacfile.cpp: Add support for cassette images in flac format. #11841
Conversation
Adding |
Thank you so much for implementing this! |
Thanks, that fixed the errors. |
src/lib/formats/flacfile.cpp
Outdated
cassette_image::error flacfile_identify(cassette_image *cassette, cassette_image::Options *opts) | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make this function static
as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made flacfile_identify static
src/lib/formats/flacfile.cpp
Outdated
std::unique_ptr<int16_t[]> samples[MAX_CHANNELS]; | ||
int16_t* channel_samples[MAX_CHANNELS]; | ||
for (int channel = 0; channel < channels; channel++) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent formatting – the right-associative unary *
qualifier should go to the right of the space.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
src/lib/formats/flacfile.cpp
Outdated
cassette_image::error err = cassette->put_samples(channel, 0.0, | ||
(double)total_samples/decoder.sample_rate(), total_samples, 2, | ||
channel_samples[channel], cassette_image::WAVEFORM_16BITLE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the decoder produce little Endian samples irrespective of host Endianness, or is it native word format? I’d rather avoid introducing more Endianness bugs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Looking at the code in flac.cpp, the flac library code produces data in host endianness. I have changed the decode call to swap endianness when the host is big endian so the generated samples should always be little endian.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code makes copies of the entire file into memory for identifying the format and decoding. Would it be better to add an accessor to cassette_image
to obtain a util::random_read &
so you can seek it to zero and give it to the FLAC decoder?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that would reduce some memory usage; updated the PR.
…oding and identifying flac files instead of reading the entire file into memory.
No description provided.