Segmentation failure during image to grayscale conversion

Question

I am writing code that should receive a color image file (PPM format) and turn it into grayscale (PGM format in ASC II code) After having done functions to read, convert the colors and save the file, the code is returning a fault Segmentation error right at the start of the main execution. Could you help me identify the mistake? That is the code:

# include <stdlib.h>
# include <stdio.h>
# define MAX 500 
typedef struct {
int r, g, b;
} pixel;

void ler_ascii(pixel imagem[MAX][MAX], char *code, int *max, int *coluna, int *linha);
void salvar_ascii(pixel imagem[MAX][MAX], char *code, int max, int coluna, int linha);
void gray_scale(pixel imagem[MAX][MAX], int coluna, int linha);

 int main(int argc, char** argv) {

     pixel imagem[MAX][MAX]; //cria uma matriz de pixeis para armazenar a imagem
     char code[3]; // codigo que identifica se a imagem é ascii ou binária
     int max; //o valor máximo de tonalidade de cada pixel
     int larg, alt; // largura e altura da imagem em pixeis

     ler_ascii(imagem, code, &max, &larg, &alt);
     gray_scale(imagem, larg, alt);
     salvar_ascii(imagem, code, max, larg, alt);

return 0;
} 

void ler_ascii(pixel imagem[MAX][MAX], char *code, int *max, int *coluna, int *linha) {
int i, j;
FILE *arquivo;

char nome_arq[50];
printf("entre com o nome do arquivo: \n");
scanf("%s", nome_arq);

if ((arquivo = fopen(nome_arq, "r")) == NULL) {
    printf("Erro ao abrir o arquivo %s\n", nome_arq);
    exit(1);
}

fscanf(arquivo, "%s", code);
fscanf(arquivo, "%d", coluna);
fscanf(arquivo, "%d", linha);
fscanf(arquivo, "%d", max);

for (i = 0; i < *linha; i++) {
    for (j = 0; j < *coluna; j++) {
        fscanf(arquivo, "%d", &imagem[i][j].r);
        fscanf(arquivo, "%d", &imagem[i][j].g);
        fscanf(arquivo, "%d", &imagem[i][j].b);
    }
}

fclose(arquivo);
}

void salvar_ascii(pixel imagem[MAX][MAX], char *code, int max, int coluna, int linha) {
int i, j;
FILE *arquivo;

char nome_arq[50];
printf("entre com o nome que deseja salvar: \n");
scanf("%s", nome_arq);

arquivo = fopen(nome_arq, "w");

fprintf(arquivo, "P3\n");
fprintf(arquivo, "%d\n ", coluna);
fprintf(arquivo, "%d\n", linha);
fprintf(arquivo, "%d\n", max);

for (i = 0; i < linha; i++) {
    for (j = 0; j < coluna; j++) {
        fprintf(arquivo, "%d ", imagem[i][j].r);
        fprintf(arquivo, "%d ", imagem[i][j].g);
        fprintf(arquivo, "%d\n", imagem[i][j].b);
    }
}

fclose(arquivo);
}

void gray_scale(pixel imagem[MAX][MAX], int coluna, int linha) {
int i, j;
for (i = 0; i < linha; i++) {
    for (j = 0; j < coluna; j++) {
        imagem[i][j].r = (int) ((0.299 * imagem[i][j].r) + (0.587 * imagem[i][j].g) + (0.144 * imagem[i][j].b)); //calcula o valor para conversão
        imagem[i][j].g = imagem[i][j].r; //copia o valor para
        imagem[i][j].b = imagem[i][j].r; //todas componentes

        //testa o valor para ver se o mesmo não passou de 255
        if (imagem[i][j].r > 255) {
            imagem[i][j].r = 255;
            imagem[i][j].g = 255;
            imagem[i][j].b = 255;

        }

    }
}
}´

Thank you

Answer 1

You are trying to allocate 250,000 pixels (500 * 500) into the stack (which is 3 MB if each int occupies 4 bytes), and then you pass this huge array to other functions. I believe this blows the stack, so the solution is to allocate the pixel array dynamically, So the battery only holds one pointer.

Change:

void ler_ascii(pixel imagem[MAX][MAX], char *code, int *max, int *coluna, int *linha);
void salvar_ascii(pixel imagem[MAX][MAX], char *code, int max, int coluna, int linha);
void gray_scale(pixel imagem[MAX][MAX], int coluna, int linha);

for:

void ler_ascii(pixel **imagem, char *code, int *max, int *coluna, int *linha);
void salvar_ascii(pixel **imagem, char *code, int max, int coluna, int linha);
void gray_scale(pixel **imagem, int coluna, int linha);

Update the function settings to be compatible with the new prototypes shown above.

Thus, instead of the functions receiving the entire matrix, they receive only the pointer to the first column of the first row of the matrix. With that the battery does not burst.

Switch (in main function):

pixel imagem[MAX][MAX]; //cria uma matriz de pixeis para armazenar a imagem

for:

pixel **imagem = (pixel**)malloc(MAX * sizeof(pixel*));
for (int i = 0; i < MAX; ++i)
     imagem[i] = (pixel*)malloc(MAX * sizeof(pixel));

Thus, you allocate an arrangement of pointers, in which each position of that arrangement is an arrangement of pixels.

Since you simply use the matrix in 3 consecutive functions and then the program ends, there’s not much need to free up the space that was allocated to the image, but it’s good practice to do it anyway, because later you may want to add more functionality to your program and in that case may arise the need to free the allocated memory, so it is good that the code that frees the memory is already present.

Add that right after you call salvar_ascii and before the return 0;:

for (int i = 0; i < MAX; ++i)
     free(imagem[i]);
 free(imagem);

Thus, each arrangement of pixels in imagem is released and then the arrranjo imagem is released.

After making these changes, the program should work.