Hi,
In the last days I've been taking as reference the example found in
examples/c/encode/file/main.c. With it I've been able to encode a 2ch,
16 bps, 44100 sample rate input WAV file to a FLAC file.
Now I've been trying to modify this example to encode a 2ch, 24 bps,
96000 sample rate WAV file. I have to say I'm a bit lost on how I
should read the input file in this case, and how should I pack the
data to feed the encoder. Any guidance regarding the use of libFLAC,
the understanding of the format, or anything else that you can notice
where I'm complete lost, I'll appreciate your help.
Here's the diff:
Index: main.c
==================================================================--- main.c
(revision 1)
+++ main.c (working copy)
@@ -39,7 +39,7 @@
#define READSIZE 1024
static unsigned total_samples = 0; /* can use a 32-bit number due to
WAVE size limitations */
-static FLAC__byte buffer[READSIZE/*samples*/ * 2/*bytes_per_sample*/
* 2/*channels*/]; /* we read the WAVE data into here */
+static FLAC__byte buffer[READSIZE/*samples*/ * 3/*bytes_per_sample*/
* 2/*channels*/]; /* we read the WAVE data into here */
static FLAC__int32 pcm[READSIZE/*samples*/ * 2/*channels*/];
int main(int argc, char *argv[])
@@ -71,13 +71,15 @@
memcmp(buffer+8, "WAVEfmt \020\000\000\000\001\000\002\000", 16) ||
memcmp(buffer+32, "\004\000\020\000data", 8)
) {
+ /* Ignoring for now as I work towards supporting 24bps files.
fprintf(stderr, "ERROR: invalid/unsupported WAVE file, only 16bps
stereo WAVE in canonical form allowed\n");
fclose(fin);
return 1;
+ */
}
sample_rate = ((((((unsigned)buffer[27] << 8) | buffer[26]) << 8)
|
buffer[25]) << 8) | buffer[24];
- channels = 2;
- bps = 16;
+ channels = ((unsigned)buffer[23] << 8) | buffer[22];
+ bps = ((unsigned)buffer[35] << 8) | buffer[34];
total_samples = (((((((unsigned)buffer[43] << 8) | buffer[42]) <<
8)
| buffer[41]) << 8) | buffer[40]) / 4;
/* allocate the encoder */
@@ -137,10 +139,17 @@
size_t i;
for(i = 0; i < need*channels; i++) {
/* inefficient but simple and works on big- or little-endian machines */
- pcm[i] = (FLAC__int32)(((FLAC__int16)(FLAC__int8)buffer[2*i+1] << 8)
| (FLAC__int16)buffer[2*i]);
+ if (bps == 16)
+ pcm[i] = (FLAC__int32)(((FLAC__int16)(FLAC__int8)buffer[2*i+1] << 8)
| (FLAC__int16)buffer[2*i]);
+ else if (bps == 24)
+ pcm[i] = (((FLAC__int32)(buffer[2*i+2] << 16))
|((FLAC__int32)(buffer[2*i+1] << 8)) | ((FLAC__int32)(buffer[2*i])));
}
/* feed samples to encoder */
ok = FLAC__stream_encoder_process_interleaved(encoder, pcm, need);
+ if (!ok) {
+ fprintf(stderr, "encoding: process_interleaved %s\n", ok?
"succeeded" : "FAILED");
+ fprintf(stderr, " state: %s\n",
FLAC__StreamEncoderStateString[FLAC__stream_encoder_get_state(encoder)]);
+ }
}
left -= need;
}
After running this I get:
encoding: process_interleaved FAILED
state: FLAC__STREAM_ENCODER_VERIFY_MISMATCH_IN_AUDIO_DATA
encoding: FAILED
state: FLAC__STREAM_ENCODER_UNINITIALIZED
Several runs to process_interleaved do succeed before falling in the
FLAC__STREAM_ENCODER_VERIFY_MISMATCH_IN_AUDIO_DATA state.
Here's the description of my input file:
ffprobe test_music_24_96.wav
avprobe version 0.8.10-4:0.8.10-0ubuntu0.12.04.1, Copyright (c)
2007-2013 the Libav developers
built on Feb 6 2014 20:56:59 with gcc 4.6.3
[wav @ 0x16719c0] max_analyze_duration reached
Input #0, wav, from 'test_music_24_96.wav':
Duration: 00:03:07.58, bitrate: 4608 kb/s
Stream #0.0: Audio: pcm_s24le, 96000 Hz, 2 channels, s32, 4608 kb/s
Thanks!
Op 14-08-14 om 02:15 schreef Jose Pablo Carballo:> Now I've been trying to modify this example to encode a 2ch, 24 bps, > 96000 sample rate WAV file. I have to say I'm a bit lost on how I > should read the input file in this case, and how should I pack the > data to feed the encoder. Any guidance regarding the use of libFLAC, > the understanding of the format, or anything else that you can notice > where I'm complete lost, I'll appreciate your help.I recommend taking a look at the API documentation: http://xiph.org/flac/api/group__flac__stream__encoder.html It looks like you haven't used FLAC__stream_encoder_set_sample_rate() and FLAC__stream_encoder_set_bits_per_sample() for example.
On Thu, Aug 14, 2014 at 1:13 AM, Martijn van Beurden <mvanb1 at gmail.com> wrote:> > Op 14-08-14 om 02:15 schreef Jose Pablo Carballo: >> Now I've been trying to modify this example to encode a 2ch, 24 bps, >> 96000 sample rate WAV file. I have to say I'm a bit lost on how I >> should read the input file in this case, and how should I pack the >> data to feed the encoder. Any guidance regarding the use of libFLAC, >> the understanding of the format, or anything else that you can notice >> where I'm complete lost, I'll appreciate your help. > > I recommend taking a look at the API documentation: > http://xiph.org/flac/api/group__flac__stream__encoder.html It > looks like you haven't used > FLAC__stream_encoder_set_sample_rate() and > FLAC__stream_encoder_set_bits_per_sample() for example. >Well, my modifications are over the encode encoder test program: https://git.xiph.org/?p=flac.git;a=blob;f=examples/c/encode/file/main.c;hb=HEAD . It does call those setters. What I'm trying to do is to convert this example to work with a 96000 sample rate and 24 bits per sample.
Ian's explanation below was helpful: On Thu, Aug 14, 2014 at 7:53 AM, Ian Nartowicz <ian at nartowicz.co.uk> wrote:> The sample rate should take care of itself. I suspect your difficulty is how > the 24 bit samples are represented in the WAV data and how to pack them into > the Flac buffer? > > WAV samples are stored in either 8, 16, or 32 bit words, using the smallest > word size that will fit the sample bits. The words are always little-endian > formatted and samples smaller than the word size are padded with zero bits. > Samples are stored with the channels interleaved. > > There are two formats to construct the buffer to be submitted to the Flac > encoder. The sample program uses the interleaved format which is submitted to > FLAC__stream_encoder_process_interleaved(). In this format, multiple channels > are all contained in the same buffer, with the the sample values for each > channel arranged one after the other, followed by the values for all the > channels of the next sample, etc. Aka interleaved. The other format is > non-interleaved, where the samples for each channel are stored in separate > buffers, and an array of those buffers is passed to > FLAC__stream_encoder_process(). Use whichever is more convenient for you to > construct from your input data. Note that the channel order for Flac is not > the same as for WAVE, although they happen to match for 2 channels. > > Each sample value (ie. the 8-24 bits representing a single channel for a single > sample) is constructed by storing the bits in the low order (the documentation > calls it "right-justified") of the smallest word that will contain them. In > the specific case of 24-bit samples, they need to be stored in the four bytes > of an int32 with the most significant byte being zero. This may look identical > to the WAVE input, but the endianness is native instead of little-endian so it > isn't safe to simply copy a WAVE sample into the Flac buffer unless you can > guarantee you are on a little-endian platform. > > A cross-platform 24-bit sample packing into a FLAC__int32 would look something > like this: > input_buffer[sample * 3 + 2] << 16 | > input_buffer[sample * 3 + 1] << 8 | > input_buffer[sample * 3 + 0] > > Or you may choose a different method for mapping the sample words from WAVE to > Flac. The sample word sizes should match and the interleaved buffer formats > should match (for two channels, not for larger channel counts), only the > endianness of each word is different. You may even write different code for the > two endianness cases, which would probably be more efficient. > > --ianBased on this, the code I wrote to fill the pcm buffer looks like this: pcm[i] = (FLAC__int32)buffer[3*i+2]; pcm[i] <<= 8; pcm[i] |= (FLAC__int32)buffer[3*i+1]; pcm[i] <<= 8; pcm[i] |= (FLAC__int32)buffer[3*i]; Thanks for your help.
> pcm[i] = (FLAC__int32)buffer[3*i+2]; > pcm[i] <<= 8; > pcm[i] |= (FLAC__int32)buffer[3*i+1]; > pcm[i] <<= 8; > pcm[i] |= (FLAC__int32)buffer[3*i];(I might be mistaken but) I think you should realize the 24 bit sample has a sign bit in the last of the bytes. Casting the FLAC__byte from the buffer to FLAC__int32 will not trat the sign bit as such leading to huge, incorrect values (and in a quick tes, to an encoding error). So you should change the cast in the first line to a FLAC__int8. The casts in the next lines aren't needed at all I think. I was writing up an example for you and came up with the following (this is extremely ugly and ineffecient, but should accept 24, 16, or any other bits-per-sample): size_t i; for (i = 0; i < need*channels; ++i) { ? pcm[i] = 0; ? size_t s; ? for (s = 0; s < (bps / 8 - 1); ++s) ??? pcm[i] |= buffer[(bps / 8) * i + s] << (8 * s); ? pcm[i] |= (FLAC__int8)buffer[(bps / 8) * i + s] << (8 * s); }
Jose Pablo Carballo <jose.carballo at ridgerun.com> wrote:> - channels = 2; > - bps = 16; > + channels = ((unsigned)buffer[23] << 8) | buffer[22]; > + bps = ((unsigned)buffer[35] << 8) | buffer[34]; > total_samples = (((((((unsigned)buffer[43] << 8) | buffer[42]) << 8) > | buffer[41]) << 8) | buffer[40]) / 4; >I suspect that the expression for total_samples should be not (.....) / 4 but (.....) / (channels * bps/8)
On Thu, Aug 14, 2014 at 12:34 PM, lvqcl <lvqcl.mail at gmail.com> wrote:> Jose Pablo Carballo <jose.carballo at ridgerun.com> wrote: > >> - channels = 2; >> - bps = 16; >> + channels = ((unsigned)buffer[23] << 8) | buffer[22]; >> + bps = ((unsigned)buffer[35] << 8) | buffer[34]; >> total_samples = (((((((unsigned)buffer[43] << 8) | buffer[42]) << 8) >> | buffer[41]) << 8) | buffer[40]) / 4; >> > > I suspect that the expression for total_samples should be not > > (.....) / 4 > > but > > (.....) / (channels * bps/8)Yes, that's correct. Here is the final diff I used to use the example to convert the 24bps file: diff --git a/examples/c/encode/file/main.c b/examples/c/encode/file/main.c index e3bdea8..b1cf374 100644 --- a/examples/c/encode/file/main.c +++ b/examples/c/encode/file/main.c @@ -40,8 +40,10 @@ static void progress_callback(const FLAC__StreamEncoder *encoder, FLAC__uint64 b #define READSIZE 1024 +#define BPS 24 /* Bits per sample */ + static unsigned total_samples = 0; /* can use a 32-bit number due to WAVE size limitations */ -static FLAC__byte buffer[READSIZE/*samples*/ * 2/*bytes_per_sample*/ * 2/*channels*/]; /* we read the WAVE data into here */ +static FLAC__byte buffer[READSIZE/*samples*/ * BPS/8 /*bytes_per_sample*/ * 2/*channels*/]; /* we read the WAVE data into here */ static FLAC__int32 pcm[READSIZE/*samples*/ * 2/*channels*/]; int main(int argc, char *argv[]) @@ -73,14 +75,18 @@ int main(int argc, char *argv[]) memcmp(buffer+8, "WAVEfmt \020\000\000\000\001\000\002\000", 16) || memcmp(buffer+32, "\004\000\020\000data", 8) ) { +#if BPS == 16 fprintf(stderr, "ERROR: invalid/unsupported WAVE file, only 16bps stereo WAVE in canonical form allowed\n"); fclose(fin); return 1; +#elif BPS == 24 + /* TODO: check wav header for 24bps */ +#endif } sample_rate = ((((((unsigned)buffer[27] << 8) | buffer[26]) << 8) | buffer[25]) << 8) | buffer[24]; - channels = 2; - bps = 16; - total_samples = (((((((unsigned)buffer[43] << 8) | buffer[42]) << 8) | buffer[41]) << 8) | buffer[40]) / 4; + channels = ((unsigned)buffer[23] << 8) | buffer[22]; + bps = ((unsigned)buffer[35] << 8) | buffer[34]; + total_samples = (((((((unsigned)buffer[43] << 8) | buffer[42]) << 8) | buffer[41]) << 8) | buffer[40]) / (channels * bps/8); /* allocate the encoder */ if((encoder = FLAC__stream_encoder_new()) == NULL) { @@ -89,7 +95,12 @@ int main(int argc, char *argv[]) return 1; } - ok &= FLAC__stream_encoder_set_verify(encoder, true); + if (bps == 16) { + /* TODO: Understand why verify doesn't work for 24bps - fails with + * FLAC__STREAM_ENCODER_VERIFY_MISMATCH_IN_AUDIO_DATA when calling + * FLAC__stream_encoder_process_interleaved().*/ + ok &= FLAC__stream_encoder_set_verify(encoder, true); + } ok &= FLAC__stream_encoder_set_compression_level(encoder, 5); ok &= FLAC__stream_encoder_set_channels(encoder, channels); ok &= FLAC__stream_encoder_set_bits_per_sample(encoder, bps); @@ -138,11 +149,23 @@ int main(int argc, char *argv[]) /* convert the packed little-endian 16-bit PCM samples from WAVE into an interleaved FLAC__int32 buffer for libFLAC */ size_t i; for(i = 0; i < need*channels; i++) { - /* inefficient but simple and works on big- or little-endian machines */ - pcm[i] = (FLAC__int32)(((FLAC__int16)(FLAC__int8)buffer[2*i+1] << 8) | (FLAC__int16)buffer[2*i]); + if (bps == 16) { + /* inefficient but simple and works on big- or little-endian machines */ + pcm[i] = (FLAC__int32)(((FLAC__int16)(FLAC__int8)buffer[2*i+1] << 8) | (FLAC__int16)buffer[2*i]); + } else if (bps == 24) { + pcm[i] = (FLAC__int32)buffer[3*i+2]; + pcm[i] <<= 8; + pcm[i] |= (FLAC__int32)buffer[3*i+1]; + pcm[i] <<= 8; + pcm[i] |= (FLAC__int32)buffer[3*i]; + } } /* feed samples to encoder */ ok = FLAC__stream_encoder_process_interleaved(encoder, pcm, need); + if (!ok) { + fprintf(stderr, "encoding: FLAC__stream_encoder_process_interleaved FAILED"); + fprintf(stderr, " state: %s\n", FLAC__StreamEncoderStateString[FLAC__stream_encoder_get_state(encoder)]); + } } left -= need; }