libmp3lame encoding to char array slow

Question

I am trying to encode pcm audio that i generated using "mplayer -ao pcm:nowaveheader" into mp3 with a c program. I don't want to write the mp3 to a file, I want to keep in in an array until i need to write it to a file, I wrote this, and it appears to work in a short .9 second test file, but it is very slow. What exactly is wrong?

#include <stdio.h>
#include <stdlib.h>
#include <lame/lame.h>

lame_global_flags *gfp;
int loopcount;
int inputSize;
FILE *fp=NULL;
FILE *fpo=NULL;
char *mp3buffer;
int mp3buffersize;
int countsize;
int x=0;
int y=0;
short *pcmbuffer;
short *lpcmbuffer;
short *rpcmbuffer;

int parse()
{
    printf("loading PCM data...\n");
    pcmbuffer=malloc(inputSize);
    fread(pcmbuffer,2,(inputSize/2),fp);
    printf("data in buffer\n");
    printf("splitting left and right channels\n");
    lpcmbuffer=malloc(inputSize/2);
    countsize=((inputSize/4)-1);
    while (x<=countsize)
    {
        lpcmbuffer[x]=pcmbuffer[x*2];
        x++;
    }
    x=0;
    rpcmbuffer=malloc(inputSize/2);
    while (x<=countsize)
    {
        rpcmbuffer[x]=pcmbuffer[(x*2)+1];
        x++;
    }
    x=0;
    printf("starting lame\n");
    gfp=lame_init();
    lame_set_num_channels(gfp,2);
    lame_set_in_samplerate(gfp,44100);
    lame_set_brate(gfp,256);
    lame_set_mode(gfp,1);
    lame_set_quality(gfp,5);
    if (lame_init_params(gfp)<0)
    {
        return 1;
    }

}

encode()
{
    x=0;
    mp3buffersize=(1.25*countsize+7200);
    mp3buffer=malloc(mp3buffersize);
    while (x!=countsize)
    {
        lame_encode_buffer(gfp,lpcmbuffer,rpcmbuffer,x,mp3buffer,mp3buffersize);
        x++;
        y++;
        if(y==1000)
        {
            printf("%d     %d\n",countsize,x);
            y=0;
        }
    }
    x=0;
    lame_encode_flush(gfp,mp3buffer,mp3buffersize);
    fpo=fopen("test.mp3","w");
    fwrite(mp3buffer,1,countsize,fpo);
}

decode()
{
}

bounty()
{
    //the quicker picker upper
    printf("closing files\n");
    fclose(fpo);
    fclose(fp);
    printf("closing lame\n");
    lame_close(gfp);
    printf("freeing pcmbuffer\n");
    free(pcmbuffer);
    free(lpcmbuffer);
    free(rpcmbuffer);
    free(mp3buffer);
}

int main(int argc,char **argv)
{
    loopcount=atoi(argv[1]);
    fp=fopen(argv[2],"r");
    if (fp==NULL)
    {
        printf("File Read Error\n");
        return 0;
    }
    fseek(fp,0,SEEK_END);
    inputSize=ftell(fp);
    fseek(fp,0,SEEK_SET);
    printf("detected a %d byte(s) file\n",inputSize);
    printf("Proceeding with parsing and importing...\n");
    if (parse()==1)
    {
        printf("lame init error\n");
    }
    printf("loopcount is %d\n",loopcount);
    encode();
    //the Quicker Picker Upper
    bounty();
    return 0;
}

Answer 1

Short answer, make this your encode function:

void encode()
{
    mp3buffersize=(1.25*countsize+7200);
    mp3buffer=malloc(mp3buffersize);
    lame_encode_buffer(gfp, lpcmbuffer, rpcmbuffer, countsize, mp3buffer, mp3buffersize);
    lame_encode_flush(gfp,mp3buffer,mp3buffersize);
    fpo=fopen("test.mp3","w");
    fwrite(mp3buffer,1,countsize,fpo);
}

I've never used lame, but, it looked like in your encode() function you were calling lame_encode_buffer() again and again, overwriting the result each time, and doing from 0 to countsize as the number of samples per channel (argument 4).

Other comments:

Why aren't you using lame_encode_buffer_interleaved()? Much of your parse() function is just undoing the existing interleaving of your file, seems like a waste.

IMO, the mass of global variables you're using are UGLY. Ideally your encode() would look more like: encode(lame_global_flags *gfp, const short * lpcmbuffer, const short * rpcmbuffer, const int countsize) this way it is clear from reading the parameter list the type of the variables, and that they must have come from/been set by the caller. const is nice to clarify that they're only for reading.

Finally, you really should have done some profiling, e.g. printing time differences between entry and exit of functions, to figure where your time sink was, and posted what you'd found. I ventured a guess looking at your loops, the encode() function had the only loop with any meat in it. I never ran your program, maybe I'm 100% wrong.