C++ program that replaces given color in an image using OpenCV

Question

I have created a C++ program that changes the color of a pixel to another color. But the main feature is that it not only changes 100% accurate colors; instead it matches two colors by using a method in which it takes the difference between the two colors' BGR values and then, the differences are squared to compare it with the square of tolerance rate.

I'm quite new to OpenCV, so if I'm using any sort of bad practices or if there's a more efficient way to do that - or any suggestion - please let me know.

I'm using GCC v11.2.0 on Arch Linux and OpenCV v4.5.5.

Here's the formula:

$$ \left( \Delta red^{2} \right) + \left( \Delta green^{2} \right) +\left( \Delta blue^{2} \right) \leqslant tolerance^{2} $$ $$ tolerance \propto \Delta pixels $$

Here's the code:

#include <iostream>
#include <opencv2/opencv.hpp>

static bool Color_Comparison(const cv::Vec3b &c1, const cv::Vec3b &c2, const unsigned int &tolerance)
{
    unsigned int blue = (c1[0] - c2[0]);
    unsigned int green = (c1[1] - c2[1]);
    unsigned int red = (c1[2] - c2[2]);
    return (((red * red) + (green * green) + (blue * blue)) <= (tolerance * tolerance));
}

static cv::String change_filename(const cv::String &fl)
{
    std::size_t ind = fl.find_last_of('.');
    if (ind == cv::String::npos)
        return fl;
    cv::String name;
    name.append(fl.begin(), fl.begin() + ind);
    name.append("_replace");
    name.append(fl.begin() + ind, fl.end());
    return name;
}

int main(int argc, char const **argv)
{
    using namespace cv;
    if (argc == 1)
    {
        std::cerr << "bad format: format is <INPUT_FILENAME>" << std::endl;
        return EXIT_FAILURE;
    }
    Mat img = imread(String(argv[1]));
    if (img.empty() == false)
    {
        Vec3b lightsaber = Vec3b(192, 196, 191), lightsaber_replace = Vec3b(10, 10, 255);
        unsigned int tolerance = 30;
        std::size_t delta_pixel = 0;
        for (int x = 0; x < img.rows; x++)
        {
            for (int y = 0; y < img.cols; y++)
            {
                Vec3b &temp_clr = img.at<Vec3b>(x, y);
                if (Color_Comparison(temp_clr, lightsaber, tolerance) == true)
                {
                    temp_clr = lightsaber_replace;
                    delta_pixel++;
                }
            }
        }
        String save_name = change_filename(String(argv[1]));
        if (imwrite(save_name, img) == true)
        {
            std::cout << "number of pixels modified: " << delta_pixel << "\n";
            std::cout << "File saved at: " << save_name << std::endl;
            return EXIT_SUCCESS;
        }
        else
        {
            std::cerr << "file not saved" << std::endl;
            return EXIT_FAILURE;
        }
    }
    else
    {
        std::cerr << "image not found or could not be opened: " << argv[1] << std::endl;
        return -1;
    }
    return 0;
}

Here's some results (on a particular image):

Before:

After:

I have compiled the above program using this command:

g++ -O3 -Ofast -Og -s -Os $(pkg-config --cflags --libs opencv4)  main.cc -o main

And the text output on the terminal was:

number of pixels modified: 3260
File saved at: ./luke_replace.jpg

Benchmark using time ./main ./luke.jpg in zsh (on Intel i5-7200U):

 0.25s user 0.02s system 99% cpu 0.270 total

_{image source}

Answer 1

Be consistent

You have many inconsistencies in your code. For example, you use EXIT_SUCCESS and EXIT_FAILURE in some places, but you return -1 in another case. Use EXIT_FAILURE in that case (which by the way is 1, not -1).

You use std::endl in some places, "\n" in others. I recommend you use the latter, as it is equivalent except the former also flushes the output, which is usually not necessary and negatively impacts performance.

Simplify expressions

A statement like if ((something) == true) can be written simpler as if (something).

Some of your expressions have a lot of unnecessary parentheses, consider removing them as an abundance of parentheses usually reduces legibility of your code. For example:

unsigned int blue = (c1[0] - c2[0]);
...
return (((red * red) + (green * green) + (blue * blue)) <= (tolerance * tolerance));

Can be rewritten as:

unsigned int blue = c1[0] - c2[0];
...
return red * red + green * green + blue * blue <= tolerance * tolerance;

You can also avoid explicit casts or mentioning of types in some cases. For example, you can write:

Mat img = imread(argv[1]);
...
Vec3b lightsaber = {192, 196, 191};

You might even consider using auto in some places to further avoid having to explicitly write type names.

Also try to avoid unnecessary nesting of code inside if-statements.

Let the user specify the output filename

Instead of trying to derive the filename for the output from the filename of the input, consider letting the user specify both filenames. That simplifies your code a lot, and also avoids some peculiar behavior. If the input filename doesn't contain a dot for example, your code would overwrite the input instead of creating a new file, without any warning.

Make better use of library functions

There are two ways to further reduce the amount of code that needs to be written. The first is by using the fact that cv::Mat provides begin() and end()-like functions, so you can use STL algorithms like std::transform() to iterate over all the pixels. Second, OpenCV itself has a lot of functionality, including the ability to calculate the \$L^2\$-squared norm of the distance between two vectors.

Example rewrite

Here is what the code could look like:

#include <iostream>
#include <opencv2/opencv.hpp>

int main(int argc, char const **argv)
{
    using namespace cv;

    if (argc != 3) {
        std::cerr << "Wrong number of arguments: format is <INPUT_FILENAME> <OUTPUT_FILENAME\n";
        return EXIT_FAILURE;
    }

    Mat img = imread(argv[1]);

    if (img.empty()) {
        std::cerr << "Error reading " << argv[1] << '\n';
        return EXIT_FAILURE;
    }

    const Vec3b lightsaber = {192, 196, 191};
    const Vec3b lightsaber_replace = {10, 10, 255};
    const unsigned int tolerance = 30;
    std::size_t delta_pixel = 0;

    std::transform(img.begin<Vec3b>(), img.end<Vec3b>(), img.begin<Vec3b>(), [&](auto& pixel){
        if (norm(pixel, lightsaber, NORM_L2SQR) < tolerance * tolerance) {
            delta_pixel++;
            return lightsaber_replace;
        } else {
            return pixel;
        }
    });

    if (!imwrite(argv[2], img)) {
        std::cerr << "Error writing " << argv[2] << '\n';
        return EXIT_FAILURE;
    }

    std::err << "Number of pixels modified: " << delta_pixel << '\n';
    return EXIT_SUCCESS;
}