Memory Allocation Problems

Hi everybody,

I'm writing a program that generates a music score for a program called CSound. My code reads in a 24 bit bitmap, converts pixels from RGB to HSI color space, and then translates the resultant data into
a score. I get runtime errors related to memory allocation.

If I include free() statements to de-allocate malloc'd memory, I get segmentation faults at run-time. The version of code I have attached includes these free() statements. If I remove the free() statements, then
I get the following runtime error message when I input images larger than some threshold (for example, I get errors for a 30x50 pixel image but not a10x4):

Code:

---

alex@laptop:~/Music/MUS88/final$ ./bmpscore instr1v2.bmp instr1.txt 1 5 0 20
bmpscore: malloc.c:3074: sYSMALLOc: Assertion `(old_top == (((mbinptr) (((char *)
 &((av)->bins[((1) - 1) * 2])) - __builtin_offsetof (struct malloc_chunk, fd)))) && old_size == 0) ||
((unsigned long) (old_size) >= (unsigned long)((((__builtin_offsetof (struct malloc_chunk,
fd_nextsize))+((2 * (sizeof(size_t))) - 1)) & ~((2 * (sizeof(size_t))) - 1))) && ((old_top)->size & 0x1)
&& ((unsigned long)old_end & pagemask) == 0)' failed.

Aborted

---

Here's the code. I'm running Ubuntu and compiled the program with GCC. Can someone please help me figure out why it's not working?

Code:

---

/* file for Comp. Mus. Final Project */
/* Alex McAuley 2009 */
/* transforms input 24-bit .bmp into Csound score */

/* input: <.bmp in> <.txt out> <instr #> <sec/pixel> <start time> <%overlap>
  example output line: i<instr #> <time> <duration> <volume (0-1)> <pitch parameter (0-1)> <timbral parameter (0-1)> */

/* return states:
        0: successful
        1: error
*/

#include <stdio.h>
#include <stdlib.h>
#include <math.h>
#include <string.h>

/* define structures */
typedef struct{                                //file header
        unsigned short type;               
        unsigned long size;                       
        unsigned short reserved1;       
        unsigned short reserved2;       
        unsigned long offset;               
} FILE_HEADER;

typedef struct{                                //info header
        unsigned long headersize;
        unsigned long width;
        unsigned long height;
        unsigned short colorplanes;
        unsigned short bitsperpixel;
        unsigned long compression;
        unsigned long bmpsize;
        unsigned long horizontalres;
        unsigned long verticalres;
        unsigned long colors;
        unsigned long importantcolors;
} INFO_HEADER;

typedef struct{                                //RGB values
        unsigned char blue;
        unsigned char green;
        unsigned char red;
} PIXEL;


/* function prototypes */
int rgbhsi(unsigned char *rav, unsigned char *gav, unsigned char *bav, float *huepix, float *satpix,
                        float *intenspix, unsigned long pixlength);
float        minrgb(float r, float g, float b);
int        avrgb(unsigned char *rpixels, unsigned char *gpixels, unsigned char *bpixels, unsigned char *rav,
                    unsigned char *gav, unsigned char *bav, unsigned        long width, unsigned long height);



int main(int argc,char* argv[])
{
        /* Declare variables */
        int i,n;
        float dur, time, volume, pitch, timbre, start, overlap;
        PIXEL pixel;
        FILE_HEADER header;
        INFO_HEADER info;
        unsigned char *rpixels, *gpixels, *bpixels, *rav, *gav, *bav;        //vectors of pixels
        float        *huepix, *satpix, *intenspix;        ; //hue, saturation, intenspixity of pixels
        FILE *fpin, *fpout;                //input and output file pointers

        if (argc != 7){
                printf("\nbmpscore: 'bmpscore <.bmp in> <.txt out> <instr #> <sec/pixel> <start time> <%%overlap>'\n\n");
                return 1;
        }

        /* Open files */
        if ( (fpin = fopen(argv[1], "rb")) == NULL ){
                printf("\nERROR: Cannot open input file.\n\n");
                return 1;
        }

        if ( (fpout = fopen(argv[2], "w")) == NULL ){
                printf("\nERROR: Cannot open/create output file.\n\n");
                return 1;
        }

        /* Read headers */
        /* NOTE: Since some compilers force FILE_HEADER struct to be 16 bits when it
        should really be 14 bits (not a multiple of 4 bytes), read data into
        each FILE_HEADER element individually */

        fread(&header.type, sizeof(header.type), 1, fpin);
        fread(&header.size, sizeof(header.size), 1, fpin);
        fread(&header.reserved1, sizeof(header.reserved1), 1, fpin);
        fread(&header.reserved2, sizeof(header.reserved2), 1, fpin);
        fread(&header.offset, sizeof(header.offset), 1, fpin);
        fread(&info, sizeof(INFO_HEADER), 1, fpin);

        /* Make sure file is a 24-bit bitmap */
        if((header.type != 19778) || (info.bitsperpixel != 24))        //not a 24-bit bitmap
        {
                printf("\nERROR. File is not a 24-bit bitmap.\n\n");
                return 1;
        }


        fseek(fpin, header.offset, SEEK_SET);

        rpixels = malloc(info.width*info.height*sizeof(unsigned char));
        memset(rpixels, 0, info.width*info.height*sizeof(unsigned char));

        gpixels = malloc(info.width*info.height*sizeof(unsigned char));
        memset(gpixels, 0, info.width*info.height*sizeof(unsigned char));

        bpixels = malloc(info.width*info.height*sizeof(unsigned char));
        memset(bpixels, 0, info.width*info.height*sizeof(unsigned char));


        /* read in pixel data */
        for(i=0; i<info.height; i++){                                        //iterate through rows
                for(n=0; n<info.width; n++){                                //iterate through columns
                        fread(&pixel,3,1,fpin);                                        //read pixel into pixels array
                        *(rpixels+i*info.width+n) = pixel.red;
                        *(gpixels+i*info.width+n) = pixel.green;
                        *(bpixels+i*info.width+n) = pixel.blue;
                }
                if((info.width*3)%4 != 0){                                                //handle padding
                        fseek(fpin, 4-(info.width*3)%4, SEEK_CUR);        //skip padding bytes
                }
        }


        /* average pix values */
        rav = malloc(info.width*sizeof(unsigned char));
        memset(rav, 0, info.width*sizeof(unsigned char));

        gav = malloc(info.width*sizeof(unsigned char));
        memset(gav, 0, info.width*sizeof(unsigned char));

        bav = malloc(info.width*sizeof(unsigned char));
        memset(bav, 0, info.width*sizeof(unsigned char));


        avrgb(rpixels,gpixels,bpixels,rav,gav,bav, info.width, info.height);
        /* */

        /* free r,g,b pixels */
        free(rpixels);
        free(gpixels);
        free(bpixels);

        huepix = malloc(info.width*sizeof(float));
        memset(huepix, 0, info.width*sizeof(float));

        satpix = malloc(info.width*sizeof(float));
        memset(satpix, 0, info.width*sizeof(float));

        intenspix = malloc(info.width*sizeof(float));
        memset(intenspix, 0, info.width*sizeof(float));


        /* convert RGB to HSI */
        rgbhsi(rav, gav, bav, huepix, satpix, intenspix, info.width);
        /* */

        /* free rav,gav,bav */
        free(rav);
        free(gav);
        free(bav);

        /* compute parameters, write output */
        dur = atof(argv[4]);
        time = atof(argv[5]);
        overlap        = atof(argv[6]);

        fprintf(fpout, ";instr        time        dur        vol        pitch        timbre\n");

        for(i=0; i<info.width; i++){
                volume = *(intenspix+i)/255;        //all values are between 0 and 1
                pitch = *(huepix+i)/360;
                timbre = *(satpix+i);
                //i<instr #> <duration> <time> <volume (0-1)> <pitch parameter (0-1)> <timbral parameter (0-1)>

                fprintf(fpout, "i%d        %5.4f        %5.4f        %5.4f        %5.4f        %5.4f\n",atoi(argv[3]),time,dur,volume,pitch,timbre);

                time += dur*(1-overlap/100);
        }


        /* free remaining memory */
        free(huepix);
        free(satpix);
        free(intenspix);

        /* close files */
        fclose(fpin);
        fclose(fpout);

        return 0;
}



int rgbhsi(unsigned char *rav, unsigned char *gav, unsigned char *bav, float *huepix, float *satpix,
                  float *intenspix, unsigned long pixlength){

        /* conversion algorithm based on http://fourier.eng.hmc.edu/e161/lectures/color_processing/node3.html (Prof. Wang)*/
        long i;
        unsigned char R,G,B;
        float r,g,b,H,S,I;

        for(i=0; i<pixlength; i++){
                R = *(rav + i);
                G = *(gav + i);
                B = *(bav + i);

                I = (R+G+B)/3.0;

                r = R/(3*I);
                g = G/(3*I);
                b = B/(3*I);

                if(I ==0){
                        r = 1;
                        g = 1;
                        b = 1;
                }

                H = acos((2*R - G - B)/(2*sqrt(pow((R-G),2) + (R-B)*(G-B))))*180/3.14159265; //in DEGREES

                if(isnan(H)){        //deal with nan problem */
                        H = 360;
                }

                if(B>G){
                        H = 360 - H;
                }

        S = 1-3*minrgb(r,g,b);

        *(huepix+i) = H;
        *(satpix+i) = S;
        *(intenspix+i) = I;
        }

        return 0;
}


float        minrgb(float r, float g, float b){
        /* returns the lowest value of r, g, b */
       
        if((r<=g)&&(g<=b))
                return r;
        else if((r<=b)&&(b<=g))
                return r;
        else if((b<=r)&&(r<=g))
                return b;
        else if((b<=g)&&(g<=r))
                return b;
        else if((g<=r)&&(r<=b))
                return g;
        else //((g<=b)&&(b<=r))
                return g;
}



int        avrgb(unsigned char *rpixels, unsigned char *gpixels, unsigned char *bpixels, unsigned char *rav,
                  unsigned char *gav, unsigned char *bav, unsigned        long width, unsigned long height){

        int i, n;
        unsigned long r, g, b;

        for(n=0;n<width;n++){        //iterate through columns
                r=0;
                g=0;
                b=0;

                for(i=0;i<height;i++){
                        r += *(rpixels + i*width+n);
                        g += *(gpixels + i*width+n);
                        b += *(bpixels + i*width+n);                       
                        *(rav+i*width+n) = r/(width*height);
                        *(gav+i*width+n) = g/(width*height);
                        *(bav+i*width+n) = b/(width*height);
                }
        }

        return 0;
}

---

No one's going to take the time to read that double-spaced pile of fantastically ugly crap. Condense it to save us time.

Aren't you playing fast and loose with malloc? No sizeof(YourDataType) * NumberYouWant.

I'm not sure, but I don't believe malloc() knows or looks up your data type for you. You tell it the size of your data type, or you're just taking a chance the default will be OK for your use.

Itsme86:

You've got to quit holding back how you feel, about posts. :p :p

I believe that's the interaction of his editor and the forum's software. My program posts used to do that, also. Since I switched to all char's, instead of tabs, it has cured the problem. Not sure just why,

Quote:

---

Originally Posted by Adak

Aren't you playing fast and loose with malloc? No sizeof(YourDataType) * NumberYouWant.

I'm not sure, but I don't believe malloc() knows or looks up your data type for you. You tell it the size of your data type, or you're just taking a chance the default will be OK for your use.

Itsme86:

You've got to quit holding back how you feel, about posts. :p :p

I believe that's the interaction of his editor and the forum's software. My program posts used to do that, also. Since I switched to all char's, instead of tabs, it has cured the problem. Not sure just why,

---

Since sizeof(unsigned char) = 1 (at least on my coumputer/compiler/whatever), I left off the sizeof(unsigned char) part in malloc(). Haha, yeah, the code looks fine in the post editing window but comes out double-spaced when I submit it. I'll probably go back and try to fix it when I have time later tonight.

Oh you wish! :p :p

I thought info.width and info.height referred to these:

unsigned long width;
unsigned long height;

from your info_header, used to make your info struct.

and these look suspiciously like what you're malloc'ing, here:

Code:

---

        rpixels = malloc(info.width*info.height);        //sizeof(unsigned char) = 1
        gpixels = malloc(info.width*info.height);
        bpixels = malloc(info.width*info.height);

---

You comment about unsigned char = 1, but they appear to be unsigned long's.

Quote:

---

Originally Posted by Adak

...
You comment about unsigned char = 1, but they appear to be unsigned long's.

---

I'm not sure I see the problem here. They are unsigned longs, but their product is just a number I pass to malloc(), specifically the number of bytes I want allocated. I tried replacing the

Code:

---

malloc(info.width*info.height)

---

statements with

Code:

---

malloc(info.width*info.height*sizeof(unsigned char))

---

statements, but there are still seg faults when I try to run the code (which I have edited in the first post to eliminate annoying double-spaces.).

sizeof(char) or sizeof(unsigned char) is ALWAYS 1. So you've done nothing.

As the first poster said, condense it down.

Code:

---

        huepix = malloc(info.width*sizeof(float));
        memset(huepix, 0, info.width*sizeof(float));

        satpix = malloc(info.width*sizeof(float));
        memset(satpix, 0, info.width*sizeof(float));

        intenspix = malloc(info.width*sizeof(float));
        memset(intenspix, 0, info.width*sizeof(float));

---

Are you wanting to store the hue, saturation and intensity of each row of pixels, or per pixel?

Quote:

---

Originally Posted by zacs7

Are you wanting to store the hue, saturation and intensity of each row of pixels, or per pixel?

---

I want to store the average hue, saturation, and intensity of each column. There are info.width columns of pixels in an image.

*EDIT*

I think I solved the problem. My avrgb function was not taking the average properly. I fixed it and now there are no more errors.

Code:

---

        for(n=0;n<width;n++){        //iterate through columns
                r=0;
                g=0;
                b=0;
                for(i=0;i<height;i++){
                        r += *(rpixels + i*width+n);
                        g += *(gpixels + i*width+n);
                        b += *(bpixels + i*width+n);                       
                }
                *(rav+n) = r/height;
                *(gav+n) = g/height;
                *(bav+n) = b/height;
        }

---

Thanks for trying to help, guys. I'm impressed by the response-to-views ratio on this forum -- helpful bunch.

This looks completely wrong:

Code:

---

int        avrgb(unsigned char *rpixels, unsigned char *gpixels, unsigned char *bpixels, unsigned char *rav,
                  unsigned char *gav, unsigned char *bav, unsigned        long width, unsigned long height){

        int i, n;
        unsigned long r, g, b;

        for(n=0;n<width;n++){        //iterate through columns
                r=0;
                g=0;
                b=0;

                for(i=0;i<height;i++){
                        r += *(rpixels + i*width+n);
                        g += *(gpixels + i*width+n);
                        b += *(bpixels + i*width+n);                       
                        *(rav+i*width+n) = r/(width*height);
                        *(gav+i*width+n) = g/(width*height);
                        *(bav+i*width+n) = b/(width*height);
                }
        }

        return 0;
}

---

I'm pretty sure you meant for those three red lines to be OUTSIDE of that inner i loop. This makes me feel much better:

Code:

---

int        avrgb(unsigned char *rpixels, unsigned char *gpixels, unsigned char *bpixels, unsigned char *rav,
                  unsigned char *gav, unsigned char *bav, unsigned        long width, unsigned long height){

        int i, n;
        unsigned long r, g, b;

        for(n=0;n<width;n++){        //iterate through columns
                r=0;
                g=0;
                b=0;

                for(i=0;i<height;i++){
                        r += *(rpixels + i*width+n);
                        g += *(gpixels + i*width+n);
                        b += *(bpixels + i*width+n);
                }

                rav[n] = r/height;
                gav[n] = g/height;
                bav[n] = b/height;
        }

        return 0;
}

---

Of course, I'm assuming that the wonderfully named avrgb() is supposed to calculate the average RGB values for the pixels in each column.

[edit]
ROFL, looks like I posted a couple seconds too late! :)
[/edit]

Code:

---

        huepix = malloc(info.width*sizeof(float));
        memset(huepix, 0, info.width*sizeof(float));

        satpix = malloc(info.width*sizeof(float));
        memset(satpix, 0, info.width*sizeof(float));

        intenspix = malloc(info.width*sizeof(float));
        memset(intenspix, 0, info.width*sizeof(float));

---

You could use calloc here, calloc does the same thing as malloc except it sets the allocated memory to 0s...calloc - C++ Reference
i.e.
intenspix = calloc(info.width, sizeof(float));