[Flac-dev] Memory leaks due to Metadata object vorbis comment API ???
Nabeel Shaheen
nabsha at gmail.com
Mon May 19 06:08:11 PDT 2008
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
More information about the Flac-dev
mailing list