Hi List,<br><br>I recently was assigned a task to port FLAC Encoder to our embedded platform. Thanks to OO-like design of the libFLAC and throught documentation, that porting went like a charm. I had some problems with chmod/chown like routines while porting but I was able to safely remove that piece of code without any trouble. <br>
<br>I have observed that the my application FLAC Encoder failes in restartability test after about 1500 restarts. Narrowing down the problem shows that I am having problem with multiple calls of FLAC__metadata_object_vorbiscomment_entry_from_name_value_pair() and/or FLAC__metadata_object_vorbiscomment_append_comment() routines, i.e. If I have vorbis comment in my metadata with copy flag == true, my application's restartability is affected. To test my observation, I took the simple encoder example available with the FLAC package and added a couple of vorbis comments in it as follows,<br>
<br><br><blockquote style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;" class="gmail_quote">#if HAVE_CONFIG_H<br># include <config.h><br>#endif<br><br>#include <stdio.h><br>
#include <stdlib.h><br>#include <string.h><br>#include "FLAC_metadata.h"<br>#include "FLAC_stream_encoder.h"<br><br>static char outFileName[256] = {"encAudio.flac"};<br>static unsigned char FLAC_tagTitle[] = {"Title"};<br>
static const unsigned char FLAC_tagArtist[] = {"Artist"};<br>static const unsigned char FLAC_tagAlbum[] = {"Album"};<br>static const unsigned char FLAC_tagYear[] = {"2008"};<br>static const unsigned char FLAC_tagGenre[] = {"Audio Track"};<br>
static const unsigned char defFileName[16] = {"encAUDIO.flac"};<br><br>static void progress_callback(const FLAC__StreamEncoder *encoder, FLAC__uint64 bytes_written, FLAC__uint64 samples_written, unsigned frames_written, unsigned total_frames_estimate, void *client_data);<br>
<br>#define READSIZE 1024<br><br>static unsigned total_samples = 0; /* can use a 32-bit number due to WAVE size limitations */<br>static FLAC__byte buffer[READSIZE/*samples*/ * 2/*bytes_per_sample*/ * 2/*channels*/]; /* we read the WAVE data into here */<br>
static FLAC__int32 pcm[READSIZE/*samples*/ * 2/*channels*/];<br><br>int main(int argc, char *argv[])<br>{<br> FLAC__bool ok = true;<br> FLAC__StreamEncoder *encoder = 0;<br> FLAC__StreamEncoderInitStatus init_status;<br>
FLAC__StreamMetadata *metadata[2];<br> FLAC__StreamMetadata_VorbisComment_Entry entry;<br> FILE *fin;<br> unsigned sample_rate = 0;<br> unsigned channels = 0;<br> unsigned bps = 0;<br><br> if(argc != 3) {<br>
fprintf(stderr, "usage: %s infile.wav outfile.flac\n", argv[0]);<br> return 1;<br> }<br><br> if((fin = fopen(argv[1], "rb")) == NULL) {<br> fprintf(stderr, "ERROR: opening %s for output\n", argv[1]);<br>
return 1;<br> }<br><br> /* read wav header and validate it */<br> if(<br> fread(buffer, 1, 44, fin) != 44 ||<br> memcmp(buffer, "RIFF", 4) ||<br> memcmp(buffer+8, "WAVEfmt \020\000\000\000\001\000\002\000", 16) ||<br>
memcmp(buffer+32, "\004\000\020\000data", 8)<br> ) {<br> fprintf(stderr, "ERROR: invalid/unsupported WAVE file, only 16bps stereo WAVE in canonical form allowed\n");<br> fclose(fin);<br>
return 1;<br> }<br> sample_rate = ((((((unsigned)buffer[27] << 8) | buffer[26]) << 8) | buffer[25]) << 8) | buffer[24];<br> printf("sampleRate:%d\n", sample_rate);<br> channels = 2;<br>
bps = 16;<br> total_samples = (((((((unsigned)buffer[43] << 8) | buffer[42]) << 8) | buffer[41]) << 8) | buffer[40]) / 4;<br> <br> /* allocate the encoder */<br> if((encoder = FLAC__stream_encoder_new()) == NULL) {<br>
fprintf(stderr, "ERROR: allocating encoder\n");<br> fclose(fin);<br> return 1;<br> }<br><br> ok &= FLAC__stream_encoder_set_verify(encoder, true);<br> ok &= FLAC__stream_encoder_set_compression_level(encoder, 5);<br>
ok &= FLAC__stream_encoder_set_channels(encoder, channels);<br> ok &= FLAC__stream_encoder_set_bits_per_sample(encoder, bps);<br> ok &= FLAC__stream_encoder_set_sample_rate(encoder, sample_rate);<br>
ok &= FLAC__stream_encoder_set_total_samples_estimate(encoder, total_samples);<br><br> if( (metadata[0] = FLAC__metadata_object_new(FLAC__METADATA_TYPE_PADDING)) == NULL ||<br> (metadata[1] = FLAC__metadata_object_new(FLAC__METADATA_TYPE_VORBIS_COMMENT)) == NULL)<br>
ok = false;<br><br> metadata[0]->length = 512; /* set the padding length */<br>#if BUGGY<br> ok &= FLAC__metadata_object_vorbiscomment_entry_from_name_value_pair(&entry, "TITLE", FLAC_tagTitle);<br>
ok &= FLAC__metadata_object_vorbiscomment_append_comment(metadata[1], entry, /*copy=*/true);<br> ok &= FLAC__metadata_object_vorbiscomment_entry_from_name_value_pair(&entry, "ARTIST", FLAC_tagArtist);<br>
ok &= FLAC__metadata_object_vorbiscomment_append_comment(metadata[1], entry, /*copy=*/true);<br> ok &= FLAC__metadata_object_vorbiscomment_entry_from_name_value_pair(&entry, "ALBUM", FLAC_tagAlbum);<br>
ok &= FLAC__metadata_object_vorbiscomment_append_comment(metadata[1], entry, /*copy=*/true);<br> ok &= FLAC__metadata_object_vorbiscomment_entry_from_name_value_pair(&entry, "GENRE", FLAC_tagGenre);<br>
ok &= FLAC__metadata_object_vorbiscomment_append_comment(metadata[1], entry, /*copy=*/true);<br> ok &= FLAC__metadata_object_vorbiscomment_entry_from_name_value_pair(&entry, "DATE", FLAC_tagYear);<br>
ok &= FLAC__metadata_object_vorbiscomment_append_comment(metadata[1], entry, /*copy=*/true);<br>#endif <br> if (!FLAC__stream_encoder_set_metadata(encoder, metadata, 2))<br> return FLAC__STREAM_ENCODER_CLIENT_ERROR;<br>
<br> /* initialize encoder */<br> if(ok) {<br> init_status = FLAC__stream_encoder_init_file(encoder, argv[2], progress_callback, /*client_data=*/NULL);<br> if(init_status != FLAC__STREAM_ENCODER_INIT_STATUS_OK) {<br>
fprintf(stderr, "ERROR: initializing encoder: %s\n", FLAC__StreamEncoderInitStatusString[init_status]);<br> ok = false;<br> }<br> }<br><br> /* read blocks of samples from WAVE file and feed to encoder */<br>
if(ok) {<br> size_t left = (size_t)total_samples;<br> while(ok && left) {<br> size_t need = (left>READSIZE? (size_t)READSIZE : (size_t)left);<br> if(fread(buffer, channels*(bps/8), need, fin) != need) {<br>
fprintf(stderr, "ERROR: reading from WAVE file\n");<br> ok = false;<br> }<br> else {<br> /* convert the packed little-endian 16-bit PCM samples from WAVE into an interleaved FLAC__int32 buffer for libFLAC */<br>
size_t i;<br> for(i = 0; i < need*channels; i++) {<br> /* inefficient but simple and works on big- or little-endian machines */<br> pcm[i] = (FLAC__int32)(((FLAC__int16)(FLAC__int8)buffer[2*i+1] << 8) | (FLAC__int16)buffer[2*i]);<br>
}<br> /* feed samples to encoder */<br> ok = FLAC__stream_encoder_process_interleaved(encoder, pcm, need);<br> }<br> left -= need;<br> }<br> }<br>
<br> ok &= FLAC__stream_encoder_finish(encoder);<br><br> fprintf(stderr, "encoding: %s\n", ok? "succeeded" : "FAILED");<br> fprintf(stderr, " state: %s\n", FLAC__StreamEncoderStateString[FLAC__stream_encoder_get_state(encoder)]);<br>
<br> /* now that encoding is finished, the metadata can be freed */<br> FLAC__metadata_object_delete(metadata[0]);<br> FLAC__metadata_object_delete(metadata[1]);<br> FLAC__stream_encoder_delete(encoder);<br> <br>
fclose(fin);<br><br> return 0;<br>}<br><br>void progress_callback(const FLAC__StreamEncoder *encoder, FLAC__uint64 bytes_written, FLAC__uint64 samples_written, unsigned frames_written, unsigned total_frames_estimate, void *client_data)<br>
{<br> (void)encoder, (void)client_data;<br><br>#ifdef _MSC_VER<br> fprintf(stderr, "wrote %I64u bytes, %I64u/%u samples, %u/%u frames\n", bytes_written, samples_written, total_samples, frames_written, total_frames_estimate);<br>
#else<br> fprintf(stderr, "wrote %llu bytes, %llu/%u samples, %u/%u frames\n", bytes_written, samples_written, total_samples, frames_written, total_frames_estimate);<br>#endif<br>}<br></blockquote><br>Then I compiled it with -g option and fed it to valgrind using tool=memcheck. and here is the output of Valgrind,<br>
<br><blockquote style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;" class="gmail_quote">... wrote 634814 bytes, 292570/292570 samples, 72/72 frames<br>encoding: succeeded<br> state: FLAC__STREAM_ENCODER_UNINITIALIZED<br>
==21449== <br>==21449== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 13 from 1)<br>==21449== malloc/free: in use at exit: 97 bytes in 5 blocks.<br>==21449== malloc/free: <b>78 allocs</b>, <b>73 frees</b>, 477,278 bytes allocated.<br>
==21449== For counts of detected errors, rerun with: -v<br>==21449== searching for pointers to 5 not-freed blocks.<br>==21449== checked 67,296 bytes.<br>==21449== <br>==21449== 97 bytes in 5 blocks are definitely lost in loss record 1 of 1<br>
==21449== at 0x401A826: malloc (vg_replace_malloc.c:149)<br>==21449== by 0x804C66F: FLAC__metadata_object_vorbiscomment_entry_from_name_value_pair (in /home/XXX/workspace/FLACEnc/FLACEncoder)<br>==21449== by 0x80488AC: (within /home/XXX/workspace/FLACEnc/FLACEncoder)<br>
==21449== <br>==21449== LEAK SUMMARY:<br>==21449== <b>definitely lost: 97 bytes in 5 blocks.</b><br>==21449== possibly lost: 0 bytes in 0 blocks.<br>==21449== still reachable: 0 bytes in 0 blocks.<br>==21449== suppressed: 0 bytes in 0 blocks.<br>
</blockquote><br>Above results are produced with libFLAC(vanilla) available with FLAC v1.2.1 on a linux distro.<br><br>I have observed that If I use VORBIS_COMMENT metadata with copy == true, I get memory leaks, but with copy==false, I DONT get memory leaks.<br>
<br>I guess by asking the question, I have solved my problem of leak but the question itself stands still. <br><br>I dont know if my question is right or not,<br>"is this behavior desired, should we get a memory leak if copy == true, when should we set copy and when should we unset it"?<br>
<br>Just before pressing Send button, I also tested If this behavior is useful for String pointers to the FLAC__metadata_object_vorbiscomment_append_comment() routine but It didnt help the leak. <br><br>Thanks all for your time.<br>
<br>Regards,<br>Nabsha<br>