I have a Canny Edge Detection Code written in C++. I would like to know how can I make it more robust in terms of detecting all the edges correctly and save it? Is this the best possible edge detection possible? Can the coders find any error in my code? Below is a picture of how the code looks like.

#include <opencv2/imgproc/imgproc.hpp>
#include <opencv2/highgui/highgui.hpp>
#include <stdlib.h>
#include <stdio.h>

using namespace cv;

/// Global variables

Mat src, src_gray;
Mat dst, detected_edges;

int edgeThresh = 1;
int lowThreshold;
int const max_lowThreshold = 100;
int ratio = 3;
int kernel_size = 3;
char* window_name = "Edge Map";

 * @function CannyThreshold
 * @brief Trackbar callback - Canny thresholds input with a ratio 1:3
void CannyThreshold(int, void*)
  /// Reduce noise with a kernel 3x3
  blur( src_gray, detected_edges, Size(3,3) );

  /// Canny detector
  Canny( detected_edges, detected_edges, lowThreshold, lowThreshold*ratio, kernel_size );

  /// Using Canny's output as a mask, we display our result
  dst = Scalar::all(0);

  src.copyTo( dst, detected_edges);
  imshow( window_name, dst );

/** @function main */
int main( int argc, char** argv )
  /// Load an image
  src = imread( argv[1] );

  if( !src.data )
  { return -1; }

  /// Create a matrix of the same type and size as src (for dst)
  dst.create( src.size(), src.type() );

  /// Convert the image to grayscale
  cvtColor( src, src_gray, CV_BGR2GRAY );

  /// Create a window
  namedWindow( window_name, CV_WINDOW_AUTOSIZE );

  /// Create a Trackbar for user to enter threshold
  createTrackbar( "Min Threshold:", window_name, &lowThreshold, max_lowThreshold, CannyThreshold );

  /// Show the image
  CannyThreshold(0, 0);

  /// Wait until user exit program by pressing a key

  return 0;

enter image description here

  • \$\begingroup\$ Hey mvr950, could you maybe tell us what version of C++ and OpenCV you are using? \$\endgroup\$
    – IEatBagels
    Dec 3, 2019 at 14:05
  • \$\begingroup\$ g++ -v gave me gcc version 6.3.0 20170516 (Debian 6.3.0-18+deb9u1) opencv on synaptic is lsb_release -a No LSB modules are available. Distributor ID: Debian Description: Debian GNU/Linux 9.3 (stretch) Release: 9.3 Codename: stretch \$\endgroup\$
    – mvr950
    Dec 3, 2019 at 14:28
  • \$\begingroup\$ There are better edge detectors that utilize machine learning but they are slower and harder to manage. Also there are other classic edge detectors. It is hard to say which detectors are better in general because some are suited to certain tasks while others are faster. \$\endgroup\$
    – ALX23z
    Dec 5, 2019 at 9:47

2 Answers 2


In terms of making more robust, a fairly obvious improvement would be to ensure that argv[1] exists before using it (that's what argc is for). If there's no argument, or the file can't be read, emit a useful error message to std::cerr and return EXIT_FAILURE - negative return values from main() can vary by platform.

Why do we declare CannyThreshold with two arguments, but never use them? Just declare it taking no arguments.

Your description mentions saving the result, but I don't see any code for that - did you perhaps show us the wrong version of the program?

  • \$\begingroup\$ I wanted to know about the best method to save an image in jpg or png format. \$\endgroup\$
    – mvr950
    Dec 3, 2019 at 14:42
  • 1
    \$\begingroup\$ That's not how Code Review works - you should have implemented to the best of your ability and shown that. We can't review code that's missing! \$\endgroup\$ Dec 3, 2019 at 15:05
  • \$\begingroup\$ Sorry. Thanks for pointing that out. \$\endgroup\$
    – mvr950
    Dec 3, 2019 at 15:10

I'm confused by this line of code:

src.copyTo( dst, detected_edges);

What you are doing is writing into dst, at the location of the detected edges, the pixel value of the original input image src.

By definition, the location of the edge is somewhere half-way the transition between two more or less uniformly colored regions. Half-way you should see a color half-way between the colors of those two regions. Why is this color interesting enough to preserve in your output? What if at the location of the transition the pixel value happens to be 0?

The purpose of the Canny edge detector is to detect the location of edges. It returns a binary image where the pixels at edges are set. These locations are found by using hysteresis thresholding (a two-threshold process) of an image where the pixel values were the edge strength. That is, there is an intermediate image where the intensity of the pixel is related to the contrast at the edge. Maybe you want to recover that image? If so, you'd have to copy-paste the code inside the Canny function, and leave out the last step, the thresholding.

This line:

blur( src_gray, detected_edges, Size(3,3) );

is not necessary. You should be able to get the same effect by increasing the apertureSize parameter to cv::Canny. Also, a box filter is the worst type of blur filter you can apply. Here I wrote up a bit explaining what is so bad about the box filter. One of the consequences of using a box filter before detecting edges is that you might find false edges.

For best precision you'd use a Gaussian filter, and then set the apertureSize parameter to 1 to avoid further smoothing inside the cv::Canny function.

You also get better results if you set the L2gradient input parameter to true. This makes the function use the correct definition of norm, which leads to a more rotationally-invariant filter (this means that you will get more similar results if you rotate your image by 45 degrees, apply the filter, and then rotate the result back).

Combining the previous point and this point, you'd end up doing:

GaussianBlur( src_gray, detected_edges, Size(0,0), 2 )
Canny( detected_edges, detected_edges, lowThreshold, lowThreshold*ratio, 1, true );

Regarding style:

Try to be consistent with spacing and so on. It makes it easier to read code. For example these three consecutive lines:

dst = Scalar::all(0);
src.copyTo( dst, detected_edges);
imshow( window_name, dst );

The first line has no spaces inside the parentheses, the second one only after the opening parenthesis, and the third one inside both.

A similar thing happens with the closing brackets: each one in your bit of code is on a different column.

Finally, I would suggest that you don't do

using namespace cv;

and instead explicitly write cv:: in front of each OpenCV function call. This makes it explicit where the function that you're calling comes from. This is a small program, and you only call one function that is not from OpenCV, but as your program grows, you'll have more self-written functions and maybe also call functions from a second library. Using explicit namespaces will make reading your code a lot easier.

Regarding "detecting all the edges correctly":

This depends on your definition of edge. The Canny edge detector detects all the edges correctly, using its definition of edge.

If you define "edges" as the edges of all objects in the image, then it is not possible to detect them all. You can only detect edges if the contrast between the object and the background is large enough. A black object on a black background will not have any "edges" (edges according to Canny's definition) along its "edges" (the actual object's edges). If you are interested in detecting these type of edges, I recommend that you closely follow the latest research publications in the field, maybe one day someone will discover how to do this. :)


Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.