Thread: Sobel Edge Detection

Hi all,

I'm trying to apply the horizontal filter to a small piece of my image to then apply to the whole image. My read and write functions are fine, but right now, all I get is a black image. Please let me nkow if is should post the .c file i'm using for read/write functions. Thank you all!

Code:

#include <stdio.h>
#include <stdlib.h>
#include "iplib2New.c"

#define n 3

//Function prototypes
image_ptr read_pnm(char *filename, int *rows, int *cols, int *type);
int getnum(FILE *fp);
void write_pnm(image_ptr ptr, char *filename, int rows, int cols, int type);
unsigned char apply_filter(unsigned char small_block[n][n], int sobel_h[3][3], int sobel_v[3][3]);

int ROWS, COLS, TYPE;

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


    int rows, cols, type;
    int i=0, j=0, x=0, y=0; //Loop vars and conditions
    int tp; //File type
    double ave=0;
    char aveStr[10], *aveStrEnd;
    unsigned char small_block[n][n]; //sub block for image

    image_ptr imagePtr, imagePtr2; //image pointer types
    image_ptr imageNegPtr;

    //Creating sobel filters
    int sobel_h[3][3] = {-1, -2, -1, 0, 0, 0, 1, 2, 1}; //Horizontal
    int sobel_v[3][3] = {-1, 0, 1, -2, 0, 2, -1, 0, 1}; //Vertical

    /* check inputs */
    if (argc != 4)
    {
        printf("wrong inputs: use %s infile out1 out2 \n", argv[0]);  
        return 0;
    } 

    /* first read-in the image */
    printf("\nReading input image... \n");

    //Pointer to access the whole image
    imagePtr = read_pnm(argv[1], &rows, &cols, &type); //Returns ptr from read_pnm

    printf("\nYour image was read successfully!\n");
    printf("Info about your image: rows = %d, cols = %d, type = %d\n", rows, cols, type);
    unsigned char image2[rows][cols];  /* space for output image */

    //Going over the whole image
    for(i=0; i<rows; i++) //The height of the image 
    {
        for(j=0; j<cols; j++) //The width of the image
        {

            //Picking up our sub block of the image
            for(y=0; y<n; y++) //Width of the sub block - cols
            {
                for(x=0; x<n; x++) //Height of the sub block - rows
                {
                    small_block[x][y]=imagePtr[(i+x)*cols+(j+y)];
                }
            }
            //Analyze sub block here
            //printf("Analyzing a sub block...\n");
            small_block[x][y] = apply_filter(small_block, sobel_h, sobel_v);
            
            //Update output image here
            image2[i][j] = small_block[x][y];
        }
    }

    printf("\nPicked up a piece of image.\n");
     
    /*
    /* now  write a small image subblock  to a file - to argv[2] 
    /* simply use the first rr x cc pixel block  in the input image 
    */ 

    tp = 5;

    imagePtr2 = (image_ptr)image2;

    //...
    // you can do calculations on the image pixels here
    //...

    printf("\nNow writing to image file ... \n");
    printf("rows=%d, cols=%d, type=%d \n", rows, cols, tp);

    write_pnm(imagePtr2, argv[2], rows, cols, tp); //Creating the out1.pgm
 
    /* image negatives  */
    imageNegPtr=(image_ptr)malloc(rows*cols*(sizeof(unsigned char)) );

    // ... codes for image negative here 
 
    printf("\nNow writing negative image file ... \n"); 
    printf("rows=%d, cols=%d, type=%d \n", rows, cols, type); 
 
    write_pnm(imageNegPtr, argv[3], rows, cols, type);  //Creating the out2.pgm

    return 0;
}

unsigned char apply_filter(unsigned char small_block[n][n], int sobel_h[3][3], int sobel_v[3][3])
{
    int x = 0;
    int y = 0;

    for(x = 0; x<=3; x++)
    {
        for(y=0; y<=3; y++)
        {
            small_block[x][y] += sobel_h[x][y]*small_block[x][y];
        }
    }

    return small_block[x][y];
}

Oh! In fact my current project is a massive graphic library. I intended to implement Canny Image Detection, because it is the standard algorithm.
Add me in skype: psp-gen for collaboration if you like.

It would help immensely if you provided enough code for us to compile your program. Namely, iplib2New.c.

Also, you should never #include a .c file (assuming you named it properly and it contains runable code). The correct way is to define a header file (make sure to use include guards) that contains necessary declarations, and #include that. Compile the corresponding .c file to an object file and link them together in your final binary. Exactly how to do this will depend on your compiler, etc.

Then, make sure you compile at maximum warning level (-Wall option for gcc, else consult your compiler docs). Make sure you resolve all warnings and errors.

It might also help if you tell us how to run your program and provide us with sample input that will help us reproduce your problem.

Some other observations:

• Don't cast malloc: FAQ > Casting malloc - Cprogramming.com
• Avoid magic numbers. If the filter size is 3, define a constant FILTER_SIZE with a value 3. Use it everywhere that it's appropriate. If your small_block size may be different from the filter size (or may be the same), define a separate constant for it, and use that where you intend to iterate over small_block. Even if they have the same value, you should have multiple constants since they have different meanings/purposes and you may wish to change their values later.
• It's generally not advisable to typecast away a pointer (e.g. image_ptr). Usually it only serves to add confusion or make it more difficult to keep track of what type something is and how many levels of direction there really are. An exception is an opaque type, but you don't seem to have that case.
• You should check the return value of malloc to make sure it succeeds (yes, it can fail, especially when allocating memory for a larger object like an image).
• You potentially access your arrays out of bounds (i.e. access memory you don't own) in several places. This results in undefined behavior: Question 11.33
• You may not be properly accounting for edge cases on line 66. I don't know for sure since I don't know what imagePtr points to since you don't provide read_pnm. However, if i or j is at the last row/col and you then add an x or y > 0 to them, you have gone outside the bounds of your array. Hello undefined behavior.
• small_block is an n * n array, meaning it's valid indexes are 0..n-1. What is the value of x and y when the loops end on line 63? What index are you accessing on line 66? Hello more undefined behavior.
• Study your loops on line 111 and 113 carefully. See any problem with the range x and y take, given the sizes of small_block and sobel_h?

Here is the other file.

Code:

/*************************************************************************** 
 * File: iplib.c                                                           *
 *                                                                         *
 * Desc: general routines for reading and writing ppm files.               *
 * Modified from: Crane R, "A simplified approach to image processing",1997*
 ***************************************************************************/

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

/* file types*/
#define PBM 4
#define PGM 5
#define PPM 6

typedef unsigned char *image_ptr;


image_ptr read_pnm(char *filename, int *rows, int *cols, int *type);
int getnum(FILE *fp);
void write_pnm(image_ptr ptr, char *filename, int rows, int cols, int type);


int ROWS, COLS, TYPE;

/***************************************************************************
 * Func: read_pnm                                                          *
 *                                                                         *
 * Desc: reads a portable bitmap file                                      *
 *                                                                         *
 * Params: filename - name of image file to read                           *
 *         rows - number of rows in the image                              *
 *         cols - number of columns in the image                           *
 *         type - file type                                                *
 *                                                                         *
 * Returns: pointer to the image just read into memory                     *
 ***************************************************************************/

image_ptr read_pnm(char *filename, int *rows, int *cols, int *type)
    {
    int i;                     /* index variable */
    int row_size;              /* size of image row in bytes */
    int maxval;                /* maximum value of pixel */
    FILE *fp;                  /* input file pointer */
    int firstchar, secchar;    /* first 2 characters in the input file */
    image_ptr ptr;             /* pointer to image buffer */
    unsigned long offset;      /* offset into image buffer */
    unsigned long total_size;  /* size of image in bytes */
    unsigned long total_bytes; /* number of total bytes written to file */
    float scale;               /* number of bytes per pixel */

 
    /* open input file */
     if((fp = fopen(filename, "rb")) == NULL)
    {
    printf("Unable to open %s for reading\n",filename);
    exit(1);
    }

    firstchar = getc(fp);
    secchar = getc(fp);
    printf("Second char %d \n", secchar); 
    
    if(firstchar != 'P')
    {
    printf("Sorry... This is not a PPM file!\n");
    exit(1);
    }


     *cols = getnum(fp);
     *rows = getnum(fp);
     *type = secchar - '0';
    
    ROWS = *rows; COLS = *cols; TYPE = *type; 
 
    switch(secchar)
    {
    case '4':            /* PBM */
        scale = 0.125;
        maxval = 1;
        break;
    case '5':            /* PGM */
        scale = 1.0;
        maxval = getnum(fp);
        break;
    case '6':             /* PPM */
        scale = 3.0;
        maxval = getnum(fp);
        break;
    default :             /* Error */
        printf("read_pnm: This is not a Portable bitmap RAWBITS file\n");
        exit(1);
        break;
    }
    
    row_size = (*cols) * scale;
    total_size = (unsigned long) (*rows) * row_size;

    ptr = (image_ptr) malloc(total_size);

    if(ptr == NULL)
    {
    printf("Unable to malloc %lu bytes\n",total_size);
    exit(1);
    }

    total_bytes=0;
    offset = 0;
    for(i=0; i<(*rows); i++)
    {
    total_bytes+=fread(ptr+offset, 1, row_size, fp);
    offset += row_size;
    }

    if(total_size != total_bytes)
    {
    printf("Failed miserably trying to read %ld bytes\nRead %ld bytes\n",
        total_size, total_bytes);
    exit(1);
    }

    fclose(fp);
    return ptr;
    }

/***************************************************************************
 * Func: getnum                                                            *
 *                                                                         *
 * Desc: reads an ASCII number from a portable bitmap file header          *
 *                                                                         *
 * Param: fp - pointer to file being read                                  *
 *                                                                         *
 * Returns: the number read                                                *
 ***************************************************************************/

int getnum(FILE *fp)
    {
    char c;               /* character read in from file */
    int i;                /* number accumulated and returned */

    do
    {
    c = getc(fp);
    }
    while((c==' ') || (c=='\t') || (c=='\n') || (c=='\r'));

    if((c<'0') || (c>'9'))
    if(c == '#')                   /* chew off comments */
        {
        while(c == '#')
        {
        while(c != '\n')
            c = getc(fp);
        c = getc(fp);
        }
        }
    else
        {
        printf("Garbage in ASCII fields\n");
        exit(1);
        }
    i=0;
    do
    {
    i=i*10+(c-'0');         /* convert ASCII to int */
    c = getc(fp);
    }
    while((c>='0') && (c<='9'));
  
    return i;
    }

/***************************************************************************
 * Func: write_pnm                                                         *
 *                                                                         *
 * Desc: writes out a portable bitmap file                                 *
 *                                                                         *
 * Params: ptr - pointer to image in memory                                *
 *         filename _ name of file to write image to                       *
 *         rows - number of rows in the image                              *
 *         cols - number of columns in the image                           *
 *         magic_number - number that defines what type of file it is      *
 *                                                                         *
 * Returns: nothing                                                        *
 ***************************************************************************/

void write_pnm(image_ptr ptr, char *filename, int rows, 
           int cols, int magic_number)
    {
    FILE *fp;             /* file pointer for output file */
    long offset;          /* current offset into image buffer */
    long total_bytes;     /* number of bytes written to output file */
    long total_size;      /* size of image buffer */
    int row_size;         /* size of row in bytes */
    int i;                /* index variable */
    float scale;          /* number of bytes per image pixel */

    switch(magic_number)
    {
    case 4:            /* PBM */
        scale = 0.125;
        break;
    case 5:            /* PGM */
        scale = 1.0;
        break;
    case 6:             /* PPM */
        scale = 3.0;
        break;
    default :             /* Error */
        printf("write_pnm: This is not a Portable bitmap RAWBITS file\n");
        exit(1);
        break;
    }

    /* open new output file */
    if((fp=fopen(filename, "wb")) == NULL)
    {
    printf("Unable to open %s for output\n",filename);
    exit(1);
    }

    /* print out the portable bitmap header */
    fprintf(fp, "P%d\n%d %d\n", magic_number, cols, rows);
    if(magic_number != 4)
    fprintf(fp, "255\n");

    row_size = cols * scale;
    total_size = (long) row_size *rows;
    offset = 0;
    total_bytes = 0;
    for(i=0; i<rows; i++)
    {
    total_bytes += fwrite(ptr+offset, 1, row_size, fp);
    offset += row_size;
    }

    if(total_bytes != total_size)
    printf("Tried to write %ld bytes...Only wrote %ld\n",
        total_size, total_bytes);

    fclose(fp);
    }

Originally Posted by anduril462

It would help immensely if you provided enough code for us to compile your program. Namely, iplib2New.c.

Also, you should never #include a .c file (assuming you named it properly and it contains runable code). The correct way is to define a header file (make sure to use include guards) that contains necessary declarations, and #include that. Compile the corresponding .c file to an object file and link them together in your final binary. Exactly how to do this will depend on your compiler, etc.

Then, make sure you compile at maximum warning level (-Wall option for gcc, else consult your compiler docs). Make sure you resolve all warnings and errors.

It might also help if you tell us how to run your program and provide us with sample input that will help us reproduce your problem.

Some other observations:

• Don't cast malloc: FAQ > Casting malloc - Cprogramming.com
• Avoid magic numbers. If the filter size is 3, define a constant FILTER_SIZE with a value 3. Use it everywhere that it's appropriate. If your small_block size may be different from the filter size (or may be the same), define a separate constant for it, and use that where you intend to iterate over small_block. Even if they have the same value, you should have multiple constants since they have different meanings/purposes and you may wish to change their values later.
• It's generally not advisable to typecast away a pointer (e.g. image_ptr). Usually it only serves to add confusion or make it more difficult to keep track of what type something is and how many levels of direction there really are. An exception is an opaque type, but you don't seem to have that case.
• You should check the return value of malloc to make sure it succeeds (yes, it can fail, especially when allocating memory for a larger object like an image).
• You potentially access your arrays out of bounds (i.e. access memory you don't own) in several places. This results in undefined behavior: Question 11.33
• You may not be properly accounting for edge cases on line 66. I don't know for sure since I don't know what imagePtr points to since you don't provide read_pnm. However, if i or j is at the last row/col and you then add an x or y > 0 to them, you have gone outside the bounds of your array. Hello undefined behavior.
• small_block is an n * n array, meaning it's valid indexes are 0..n-1. What is the value of x and y when the loops end on line 63? What index are you accessing on line 66? Hello more undefined behavior.
• Study your loops on line 111 and 113 carefully. See any problem with the range x and y take, given the sizes of small_block and sobel_h?

Thank you for your advice. I appreciate it.

I don't know how to create it as a .c file. MY professor gave it to me like that.

I have no warnings or errors.

Sample input after compiling is: a.out originalImage.pgm out1.pgm out2.pgm where originalimage.pgm is the file you read in and out1.pgm is the image with a filter applied to it, and out2.pgm is the negative image of the original.

- I will read about malloc. Thank you.
- What do you mean by magic numbers? SO I should have this instead:
#define FILTER_SIZE 3
- Typecast away a pointer? What should I Have instaed here then?
- Edge cases as in the border of the image? I Thought I wanted the whole image?
- I see what you mean about accessing my small_block out of bounds in multiple places. I am trying to pick up a 3 x 3 piece of the image inside the total image, apply a horizontal and vertical filter to the small block, and then place the small block in my out1.pgm to create my filtered image.
- I can iterate through the entire image fine, but I am having trouble seeing how to apply the filters to my small block once I've picked it out.
- I also don't understand how to write the small block to a big image that will become the output image of the original.

Does this make sense? Do you have other obersvations in making my code better?

Thank you for your help, by the way.

This is definitely not what you want.

Hahaha, yeah, I figured, but this is more than just a black image which is what I was getting before. Small progress.

Originally Posted by Al3

Your sobel operator is wrong, Kati (considering the fact I don't see its appearing at all. The correct effect of sobel is as much the edge is contrasted as much it is enlighten, which is achieved using the formula for 2-dimensional convolution on a first point.
You can use that kind of a 4D loop:

Code:

 for (y = 1; y < y_size1 - 1; y++) {
    for (x = 1; x < x_size1 - 1; x++) {
      pixel_value = 0.0;
      for (j = -1; j <= 1; j++) {
          for (i = -1; i <= 1; i++) {
            pixel_value += weight[j + 1][i + 1] * image1[y + j][x + i];
          }
      }
      if (pixel_value < min) min = pixel_value;
      if (pixel_value > max) max = pixel_value;
    }
  }

I thought you took a 3 x3 from the total image and applied the horizontal and vertical filters on the the 3 x 3 of the image to get a summation of those values for the center block? I know it's called convolution, but I don't know what that means, mathematically. I was only trying to apply hte horizontal filter, but I guess I'm applying it incorrectly. I don't understand how , though. THis is more a problem in not understanding C than the Sobel Operator.

Hahaha, you make a very good point, yes.

Normally, my code is a lot cleaner, but we were given some sample code from my professor to work with in doing this problem, so I have been injecting my own comments and code into what he gave us. This is a past due assignment. I didn't get to finish it, but I'd still like to know how to do it.

- Why do your for-loops start at -1?
- I assume they go to y_size1 - 1 to exclude the border of the image?
- Why are the 2 if statements after your inner for loops to set the pixel_value? I don't understand why you need this Maximum values calculation.
- The middle set of 2 for loops are to create space for your output image?
- The last set of for loops are for putting your values in that image that you allocated space for?

I'm fairly new to EDA's, but this is the one I'm most familiar with right now which isn't saying much. Haha, thank you for your help.

Because its easier. Example:

Code:

for(i = (-1) i<4; i++) whatever[i + 1]...

instead of

Code:

for (i = 0; i <5; i++) whatever[(i-1) + 1]..

Yes.

Maximum values calculation after filtering is part of the alg. I didn't invent that algorithm so I don't know exactly why.. but its just sticking to standards you know.

They are for initialization of the image, something like that.

Generation of image2 after linear transofmration