Ask Your Question
-1

Unexpected Results at Subtracting Two Images

asked 2020-01-17 15:00:55 -0600

Darth Revan gravatar image

Hello there,

I try to implement subtraction of two images. I am sure there are too many people who are ready to shot me with their "Why don't you just use cv::subtract or simply cv::Mat Result = Image1 - Image2;" gun. Jokes aside, the reason is that I try to implement my own version to use it as a function which is going to run on separate threads. When I used

    #include <opencv2/opencv.hpp>
    #include <opencv2/core/core.hpp>
    #include <opencv2/highgui/highgui.hpp>
    #include <iostream>
    #include <string>
    #include <cstdlib>
    #include <cstdio>
    #include <new>
    #include <chrono>
    #include <omp.h>

void rgbToHsv(cv::Mat* inputImage, cv::Mat* outputImage);
void subtractImage(cv::Mat* inputImage, cv::Mat* outputImage, cv::Mat* imageMask);

int main(int argc, char const *argv[])
{
  cv::Mat inputImage = imread("lena.png", cv::IMREAD_UNCHANGED); //Input image

  //Converting image from RGB to HSV colorspace
  cv::Mat inputImageHsv = inputImage.clone();
  rgbToHsv(&inputImage, &inputImageHsv);

//Splitting V channel for later use
  cv::Mat inputImageHsvChannels[3];
  cv::split(inputImageHsv, inputImageHsvChannels);

  cv::Mat inputImageH = inputImageHsvChannels[0];
  cv::Mat inputImageS = inputImageHsvChannels[1];
  cv::Mat inputImageV = inputImageHsvChannels[2];

  cv::Mat blurredImage = inputImageV.clone();
  cv::Mat imageMask = inputImageV.clone();
  cv::Mat imageMask2 = inputImageV.clone();
  cv::Mat imageMask3 = inputImageV.clone();

  cv::GaussianBlur(inputImageV, blurredImage, cv::Size(5,5), 0, 0);


  cv::subtract(inputImageV,blurredImage, imageMask);
  imageMask2 = inputImageV - blurredImage;  
  subtractImage(&inputImageV, &blurredImage, &imageMask3);

  cv::imshow("Subtracted Image1 OPENCV", imageMask);
  cv::imshow("Subtracted Image2 OPENCV", imageMask2);
  cv::imshow("Subtracted Image3 MY FUNC", imageMask3);
  cv::waitKey();

  return 0;
}

void subtractImage(cv::Mat* inputImage1, cv::Mat* inputImage2, cv::Mat* outputImage)
{
  for(int i = 0; i < inputImage1->rows; ++i)
  {
    for(int j = 0; j < inputImage1->cols; ++j)
    {
      int iVal = inputImage1->at<uchar>(i,j) - inputImage2->at<uchar>(i,j);

      if(iVal < 0)
        outputImage->at<uchar>(i,j) = 0;

      outputImage->at<uchar>(i,j) = iVal;
    }
  }
}

void rgbToHsv(cv::Mat* inputImage, cv::Mat* outputImage)
{
  double redSc = 0, greenSc = 0, blueSc = 0; //Scaled R, G, B values of current pixel
  double h = 0, s = 0, v = 0; //R, G, B values of current pixel
  double cmin = 0, cmax = 0; //Min and max dummy variables
  double delta = 0; //Difference between min and max

  int channels = inputImage->channels();
  int nRows = inputImage->rows;
  int nCols = inputImage->cols*channels;

  if (inputImage->isContinuous())
  {
    nCols *= nRows;
    nRows = 1;
  }

  uchar* p;
  uchar* q;

  for(int i = 0; i < nRows; ++i){
    p = inputImage->ptr<uchar>(i);
    q = outputImage->ptr<uchar>(i);

    for(int j = 0; j < nCols; j+=3){    
      redSc = p[j+2] / 255.;
      greenSc = p[j+1] / 255.;
      blueSc = p[j] / 255.;

      cmin = std::min(std::min(redSc, greenSc), blueSc);
      cmax = std::max(std::max(redSc, greenSc), blueSc);
      delta = cmax - cmin;

      if(!delta){
        h = 0.;
        s = 0.;
        v = cmax * 255.;
      }
      else{
        if(cmax == redSc)
        h = 60. * ((greenSc - blueSc)/delta);

        if(cmax == greenSc)
        h = 120 + (60. * (((blueSc - redSc)/delta)));

        if(cmax == blueSc)
        h = 240 + (60. * (((redSc - greenSc)/delta)));

        if(h < 0)
        h += 360;

        h = (h/2);

        v = cmax* 255.;

        s = ((cmax==0)?0:((delta/cmax)*255.));

        q[j+2] = v;   //Red
        q[j+1] = s; //Green
        q[j] = h; //Blue   
      }
    }
  }
}

Here are the results.

OpenCV Style 1 OpenCV Style 2 My Implementation

edit retag flag offensive close merge delete

Comments

2

NEVER use pointers to cv::Mat (which is already a "smart pointer")

berak gravatar imageberak ( 2020-01-17 15:07:54 -0600 )edit

Oh many thanks for the knowledge. I didn't know that cv::Mat is already a "smart pointer", so I used cv::Mat* to reduce data size. On the other hand, changing it had no effect on solving my problem.

Darth Revan gravatar imageDarth Revan ( 2020-01-17 15:15:27 -0600 )edit

In my custom function void subtractImage(cv::Mat inputImage, cv::Mat outputImage, cv::Mat imageMask); I simply access each element of both images and subtract those values. So where is my mistake?

Darth Revan gravatar imageDarth Revan ( 2020-01-17 15:24:46 -0600 )edit

1 answer

Sort by ยป oldest newest most voted
0

answered 2020-01-17 16:18:29 -0600

Tetragramm gravatar image

updated 2020-01-22 23:06:57 -0600

EDIT: See comments about saturate_cast and the missing else.

Also, that is what references are for. Passing things into functions without making copies, that is.

edit flag offensive delete link more

Comments

Actually before creating this thread I also tried to cast inputImage1->at<uchar>(i,j) and inputImage2->at<uchar>(i,j) to integer but no changes. Still couldn't figure out the reason of this unexpected results. Someting about types of my input images?

Darth Revan gravatar imageDarth Revan ( 2020-01-18 03:24:20 -0600 )edit

Did you try that exact line?

Tetragramm gravatar imageTetragramm ( 2020-01-18 22:03:37 -0600 )edit

Yes I exactly did this one.

Darth Revan gravatar imageDarth Revan ( 2020-01-18 23:52:10 -0600 )edit
berak gravatar imageberak ( 2020-01-19 12:11:39 -0600 )edit

Oh, I see. saturate_cast will do what needs to be done, but your problem is your if(iVal<0). If it is less than 0, you set the output value to 0, then promptly overwrite it with iVal. You need to use the else.

Tetragramm gravatar imageTetragramm ( 2020-01-19 16:48:04 -0600 )edit

saturate_cast worked like a charm and also I noticed that I forgot to add else{...}. Tetragramm would you please edit your answer and I can mark it as "solution". Thanks for all you guys' help.

Darth Revan gravatar imageDarth Revan ( 2020-01-21 11:47:01 -0600 )edit

Okay, I marked this as "solution". Case closed but I still wonder that why did I get a down vote lol.

Darth Revan gravatar imageDarth Revan ( 2020-01-23 01:40:20 -0600 )edit

but I still wonder that why did I get a down vote lol.

sorry for being late, but it's my dv. and here is why:

  • you're defeating any optimisation (sse, parallelization, opencl) builtin in opencv's counterpart of your self-made "bad function"
  • it is already multithreaded. you don't respect, that they're trying to use a "data-parallel" approach, and you're trying to roll your own "thread-parallel" thing on top of it. bad idea. not recommended. don't show that to noobs. ;(
berak gravatar imageberak ( 2020-01-29 13:35:12 -0600 )edit

I understand your point, I try to re-write built-in OpenCV functions which are already benefit from parallelization and it is bad motive to re-write them but let's say I need to implement an algorithm or a method that doesn't provided by OpenCV's default library and I need to implement it multi-threaded. So didn't I need to write it all myself? At that point, only advantage that OpenCV can provide me is just the interface to access image pixels or colorspace conversion so I don't need to write any code to read/write or convert images right? So whole thing is just like a practice for further works. BUT I agree with you on the not to re-write the methods which are already implemented in OpenCV's default library.

Darth Revan gravatar imageDarth Revan ( 2020-01-29 14:16:38 -0600 )edit
1

Question Tools

1 follower

Stats

Asked: 2020-01-17 15:00:55 -0600

Seen: 591 times

Last updated: Jan 22 '20