Code Review

[Stephen-Seo]

Code Review

Handling of Alpha Channel

I'm having some troubles running the code. I figured out that I needed to convert images to .pxa format with the supplied Python scripts, but there are some issues I've noticed.

This bool constant is used in a way that doesn't actually check if the supplied image has an alpha channel:

https://github.com/matthague/imageProcessing/blob/2a1ee69dd6873cf81220a9ab0cd1a4c8e6eb7713/imageProcessing.cu#L46

As a result, the following snippet will use the RG channels in an RGB image by default (using a depth of 2 instead of 3):

https://github.com/matthague/imageProcessing/blob/2a1ee69dd6873cf81220a9ab0cd1a4c8e6eb7713/imageProcessing.cu#L294-L299

I think it would be better to count the number of channels, and from there determine based on the bool constant to remove a channel or not.

Buffer Overflow Invalidating Pointer

I've been getting segmentation faults trying out the program, and I've debugged it to the point where I think I found a buffer overflow.

https://github.com/matthague/imageProcessing/blob/2a1ee69dd6873cf81220a9ab0cd1a4c8e6eb7713/imageHandling.cu#L12-L24

I debugged the value of offset and discovered that pixelArrays is being assigned to 1 extra time, which overwrote the outputPixelArrays[0] pointer to an invalid value (on my machine), causing the segmentation fault. The following snippet should fix this issue:

void imageHandling::loadImage(FILE *file, double **pixelArrays, int width, int height, int depth) {
  int offset = 0;
  while (!feof(file)) {
      for (int i = 0; i < depth; i++) {
          double tval;
          fscanf(file, "%lf", &tval);
          if (feof(file)) {
              break;
          }
          pixelArrays[i][offset] = tval;
      }
      offset += 1;
  }
  fclose(file);
}

Note I've added a if (feof(file)) { break; } which should prevent that extra write from occurring (I've tested this to work).

Clarity

Looking through the code, there are comments within functions that help describe what's going on, but I think it is lacking comments that describe the purpose and usage of each function, which I find confusing. If I have to do some research on my own to understand the code, then I think more comments would be useful here (though I can't really say I'm not somewhat guilty of having the same issue in my code).

ETC

I tested all the available options and I'm not sure if I'm setting the parameters correctly as I end up with images with colored grain added to the original (I'm not sure if there's an option to use the denoiser without adding noise, and I'm not sure if that's necessary for this to work). I think it would be helpful to post reasonable default values as a reference.

Other than the buffer overflow, things look good. I'm just confused on how to use this properly though.

[matthague]

Hi Stephen, thanks for taking the time to look at my project. I've incorporated the changes that you suggested, and added comments that will hopefully better describe the purpose of each function.

Here are some reasonable test values, found through experimentation.

If you need to convert an image to .pxa format, you can do python3 pixelExtractor filename.format to get a filename.pxa file. I recommend using smaller images (up to 400x400), as some methods have complexity O(n^3).

To just add additive white gaussian noise w/ std. deviation 20 ./imageProcessing filename.pxa filename.out 1 0 20

To just add salt and pepper noise w/ rate 30 ./imageProcessing filename.pxa filename.out 2 0 30

To perform total variational reduction on an image with additive white gaussian noise w/ std. deviation 20 using lambda .1 ./imageProcessing filename.pxa filename.out 1 6 20 .1

To perform a median filter on an image with salt and pepper noise w/ rate 30 using lambda 1 ./imageProcessing filename.pxa filename.out 2 4 30 1

To view the processed images you can use python3 pixelCompressor filename.out filename.png to get a finished image.