Nabeel Shaheen
2008-May-19 13:08 UTC
[Flac-dev] Memory leaks due to Metadata object vorbis comment API ???
Hi List, 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. 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, #if HAVE_CONFIG_H> # include <config.h> > #endif > > #include <stdio.h> > #include <stdlib.h> > #include <string.h> > #include "FLAC_metadata.h" > #include "FLAC_stream_encoder.h" > > static char outFileName[256] = {"encAudio.flac"}; > static unsigned char FLAC_tagTitle[] = {"Title"}; > static const unsigned char FLAC_tagArtist[] = {"Artist"}; > static const unsigned char FLAC_tagAlbum[] = {"Album"}; > static const unsigned char FLAC_tagYear[] = {"2008"}; > static const unsigned char FLAC_tagGenre[] = {"Audio Track"}; > static const unsigned char defFileName[16] = {"encAUDIO.flac"}; > > 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); > > #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__int32 pcm[READSIZE/*samples*/ * 2/*channels*/]; > > int main(int argc, char *argv[]) > { > FLAC__bool ok = true; > FLAC__StreamEncoder *encoder = 0; > FLAC__StreamEncoderInitStatus init_status; > FLAC__StreamMetadata *metadata[2]; > FLAC__StreamMetadata_VorbisComment_Entry entry; > FILE *fin; > unsigned sample_rate = 0; > unsigned channels = 0; > unsigned bps = 0; > > if(argc != 3) { > fprintf(stderr, "usage: %s infile.wav outfile.flac\n", argv[0]); > return 1; > } > > if((fin = fopen(argv[1], "rb")) == NULL) { > fprintf(stderr, "ERROR: opening %s for output\n", argv[1]); > return 1; > } > > /* read wav header and validate it */ > if( > fread(buffer, 1, 44, fin) != 44 || > memcmp(buffer, "RIFF", 4) || > memcmp(buffer+8, "WAVEfmt \020\000\000\000\001\000\002\000", 16) || > memcmp(buffer+32, "\004\000\020\000data", 8) > ) { > 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]; > printf("sampleRate:%d\n", sample_rate); > channels = 2; > bps = 16; > total_samples = (((((((unsigned)buffer[43] << 8) | buffer[42]) << 8) | > buffer[41]) << 8) | buffer[40]) / 4; > > /* allocate the encoder */ > if((encoder = FLAC__stream_encoder_new()) == NULL) { > fprintf(stderr, "ERROR: allocating encoder\n"); > fclose(fin); > return 1; > } > > 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); > ok &= FLAC__stream_encoder_set_sample_rate(encoder, sample_rate); > ok &= FLAC__stream_encoder_set_total_samples_estimate(encoder, > total_samples); > > if( (metadata[0] > FLAC__metadata_object_new(FLAC__METADATA_TYPE_PADDING)) == NULL || > (metadata[1] > FLAC__metadata_object_new(FLAC__METADATA_TYPE_VORBIS_COMMENT)) == NULL) > ok = false; > > metadata[0]->length = 512; /* set the padding length */ > #if BUGGY > ok &> FLAC__metadata_object_vorbiscomment_entry_from_name_value_pair(&entry, > "TITLE", FLAC_tagTitle); > ok &= FLAC__metadata_object_vorbiscomment_append_comment(metadata[1], > entry, /*copy=*/true); > ok &> FLAC__metadata_object_vorbiscomment_entry_from_name_value_pair(&entry, > "ARTIST", FLAC_tagArtist); > ok &= FLAC__metadata_object_vorbiscomment_append_comment(metadata[1], > entry, /*copy=*/true); > ok &> FLAC__metadata_object_vorbiscomment_entry_from_name_value_pair(&entry, > "ALBUM", FLAC_tagAlbum); > ok &= FLAC__metadata_object_vorbiscomment_append_comment(metadata[1], > entry, /*copy=*/true); > ok &> FLAC__metadata_object_vorbiscomment_entry_from_name_value_pair(&entry, > "GENRE", FLAC_tagGenre); > ok &= FLAC__metadata_object_vorbiscomment_append_comment(metadata[1], > entry, /*copy=*/true); > ok &> FLAC__metadata_object_vorbiscomment_entry_from_name_value_pair(&entry, > "DATE", FLAC_tagYear); > ok &= FLAC__metadata_object_vorbiscomment_append_comment(metadata[1], > entry, /*copy=*/true); > #endif > if (!FLAC__stream_encoder_set_metadata(encoder, metadata, 2)) > return FLAC__STREAM_ENCODER_CLIENT_ERROR; > > /* initialize encoder */ > if(ok) { > init_status = FLAC__stream_encoder_init_file(encoder, argv[2], > progress_callback, /*client_data=*/NULL); > if(init_status != FLAC__STREAM_ENCODER_INIT_STATUS_OK) { > fprintf(stderr, "ERROR: initializing encoder: %s\n", > FLAC__StreamEncoderInitStatusString[init_status]); > ok = false; > } > } > > /* read blocks of samples from WAVE file and feed to encoder */ > if(ok) { > size_t left = (size_t)total_samples; > while(ok && left) { > size_t need = (left>READSIZE? (size_t)READSIZE : (size_t)left); > if(fread(buffer, channels*(bps/8), need, fin) != need) { > fprintf(stderr, "ERROR: reading from WAVE file\n"); > ok = false; > } > else { > /* 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]); > } > /* feed samples to encoder */ > ok = FLAC__stream_encoder_process_interleaved(encoder, pcm, > need); > } > left -= need; > } > } > > ok &= FLAC__stream_encoder_finish(encoder); > > fprintf(stderr, "encoding: %s\n", ok? "succeeded" : "FAILED"); > fprintf(stderr, " state: %s\n", > FLAC__StreamEncoderStateString[FLAC__stream_encoder_get_state(encoder)]); > > /* now that encoding is finished, the metadata can be freed */ > FLAC__metadata_object_delete(metadata[0]); > FLAC__metadata_object_delete(metadata[1]); > FLAC__stream_encoder_delete(encoder); > > fclose(fin); > > return 0; > } > > 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) > { > (void)encoder, (void)client_data; > > #ifdef _MSC_VER > fprintf(stderr, "wrote %I64u bytes, %I64u/%u samples, %u/%u frames\n", > bytes_written, samples_written, total_samples, frames_written, > total_frames_estimate); > #else > fprintf(stderr, "wrote %llu bytes, %llu/%u samples, %u/%u frames\n", > bytes_written, samples_written, total_samples, frames_written, > total_frames_estimate); > #endif > } >Then I compiled it with -g option and fed it to valgrind using tool=memcheck. and here is the output of Valgrind, ... wrote 634814 bytes, 292570/292570 samples, 72/72 frames> encoding: succeeded > state: FLAC__STREAM_ENCODER_UNINITIALIZED > ==21449=> ==21449== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 13 from 1) > ==21449== malloc/free: in use at exit: 97 bytes in 5 blocks. > ==21449== malloc/free: *78 allocs*, *73 frees*, 477,278 bytes allocated. > ==21449== For counts of detected errors, rerun with: -v > ==21449== searching for pointers to 5 not-freed blocks. > ==21449== checked 67,296 bytes. > ==21449=> ==21449== 97 bytes in 5 blocks are definitely lost in loss record 1 of 1 > ==21449== at 0x401A826: malloc (vg_replace_malloc.c:149) > ==21449== by 0x804C66F: > FLAC__metadata_object_vorbiscomment_entry_from_name_value_pair (in > /home/XXX/workspace/FLACEnc/FLACEncoder) > ==21449== by 0x80488AC: (within /home/XXX/workspace/FLACEnc/FLACEncoder) > ==21449=> ==21449== LEAK SUMMARY: > ==21449== *definitely lost: 97 bytes in 5 blocks.* > ==21449== possibly lost: 0 bytes in 0 blocks. > ==21449== still reachable: 0 bytes in 0 blocks. > ==21449== suppressed: 0 bytes in 0 blocks. >Above results are produced with libFLAC(vanilla) available with FLAC v1.2.1 on a linux distro. 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. I guess by asking the question, I have solved my problem of leak but the question itself stands still. I dont know if my question is right or not, "is this behavior desired, should we get a memory leak if copy == true, when should we set copy and when should we unset it"? 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. Thanks all for your time. Regards, Nabsha -------------- next part -------------- An HTML attachment was scrubbed... URL: http://lists.xiph.org/pipermail/flac-dev/attachments/20080519/be4da3b7/attachment.htm