Question:
I'm writing code that should take a color image file (PPM format) and transform it to grayscale (PGM format in ASC II code) After I've done functions to read, convert colors and save the file, the code is returning a segmentation fault error right at the start of main execution. Could you help me identify the error? This 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;
}
}
}
}´
Thanks
Answer:
You're trying to allocate 250,000 pixels (500 * 500) on the stack (which is 3MB if each int occupies 4 bytes), and then you pass that huge array to other functions. I believe this overflows the stack, so the solution is to dynamically allocate the pixel array so that the stack 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 definitions to be compatible with the new prototypes shown above.
So, instead of the functions taking the entire array, they just take the pointer to the first column of the first row of the array. With that the pile doesn't overflow.
Change (in the 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));
So you allocate an array of pointers, where each position in that array is an array of pixel .
Since you simply use the array for 3 functions in a row and then the program finishes, there isn't much need to free up the space that has been allocated for the image, but it's good practice to do this anyway, as you may want to add more functionality to the image later. your program and in this case the need to free the allocated memory may arise, so it's good that the code that frees the memory is already present.
Add this right after calling salvar_ascii and before return 0; :
for (int i = 0; i < MAX; ++i)
free(imagem[i]);
free(imagem);
Thus, each array of pixel in the imagem is freed and then the array of imagem is freed.
After making these changes, the program should work.