Batch extracting differences between two images with enhancement using opencv

Question

I am attempting to extract the absolute differences image between two input images and then enhance the intensity (brightness) channel in HSV color space with the given times. The workflow is like below.

The processing of a batch of files is considered. Thus, the input is path strings input_path1 and input_path2. The third parameter is the times for multiplying on intensity channel. As the result, with the input paths ./1/ and ./2/, the image pair ./1/1.png and ./2/1.png will be taken as the inputs and the next image pair is ./1/2.png and ./2/2.png.

The experimental implementation

template<typename T>
void difference_and_enhancement(std::string input_path1, std::string input_path2, T enhancement_times,
     std::string output_path, bool output_leading_zero, std::string filename_extension = ".png")
{
    //  Clear output
    std::filesystem::remove_all(output_path);
    std::filesystem::create_directory(output_path);

    for (std::size_t i = 1; i <= 1000; i++)
    {
        std::string filename1 = input_path1 + std::to_string(i) + filename_extension;
        if (std::filesystem::is_regular_file(filename1) == false)
        {
            std::cout << filename1 << " is not a regular file." << "\n";
            return;
        }

        std::string filename2 = input_path2 + std::to_string(i) + filename_extension;
        if (std::filesystem::is_regular_file(filename2) == false)
        {
            std::cout << filename2 << " is not a regular file." << "\n";
            return;
        }

        std::cout << "Read " << filename1 << "\n";
        auto image1 = cv::imread(filename1, cv::ImreadModes::IMREAD_COLOR);

        std::cout << "Read " << filename2 << "\n";
        auto image2 = cv::imread(filename2, cv::ImreadModes::IMREAD_COLOR);

        char buff[100];
        if (output_leading_zero)
        {
            snprintf(buff, sizeof(buff), "%s%03d%s", output_path.c_str(), i, filename_extension.c_str());
        }
        else
        {
            snprintf(buff, sizeof(buff), "%s%d%s", output_path.c_str(), i, filename_extension.c_str());
        }
        std::cout << "Output filename = " << buff << "\n";
        
        //  Difference
        cv::Mat diffImage;
        cv::absdiff(image1, image2, diffImage);
        cv::Mat img_hsv;
        cvtColor(diffImage, img_hsv, CV_RGB2HSV);
        
        std::vector<cv::Mat> hsv_channels;
        cv::split(img_hsv, hsv_channels);
        cv::Mat h_image = hsv_channels[0];
        cv::Mat s_image = hsv_channels[1];
        cv::Mat v_image = hsv_channels[2];

        cv::Mat output_hsv;
        std::vector<cv::Mat> hsv_channels_output;
        hsv_channels_output.push_back(h_image);
        hsv_channels_output.push_back(s_image);

        v_image.forEach<double>([&](double element, const int* position) { return element * static_cast<decltype(element)>(enhancement_times); });
        hsv_channels_output.push_back(v_image);
        cv::merge(hsv_channels_output, output_hsv);

        cv::Mat output;
        cvtColor(output_hsv, output, CV_HSV2RGB);

        std::cout << "size = " << output.size() << "\n";
        cv::imwrite(std::string(buff), output);
    }
}

The usage example

int main()
{
    difference_and_enhancement(
        "./1/",
        "./2/",
        100,
        "./OutputImages/",
        false);
    return 0;
}

All suggestions are welcome.

Answer 1

The biggest simplification and speed improvement would be to avoid using the split function. I would suggest just multiplying the 3-channel image directly. Something like this:

cv::multiply(img_hsv, Scalar(1, 1, enhancement_times), img_hsv);

Of course, we can’t know for sure which is faster until we compare the running time for each of the alternatives.

In general, try to avoid creating new image variables when you transform the image. Use the same memory block for both the input and the output of the function, like I did above. You have diffImage, img_hsv, output_hsv and output, as well as the two arrays with single-channel images. But all these operations are single-pixel operations that can be performed in-place, without needing the additional memory. In-place operations not only save on memory, they are faster too, as there is less memory being mapped into the processor’s cache.

If you do want to use the split (it might be faster?), make sure you don’t create a second array for the modified images:

cv::split(img_hsv, hsv_channels);
hsv_channels[2].forEach<double>([&](double element, const int* position) { return element * static_cast<decltype(element)>(enhancement_times); });
cv::merge(hsv_channels, img_hsv);

The multiplication can be written more concisely (for better readability) as this:

hsv_channels[2] *= enhancement_times;

(Apparently the *= operator doesn’t work for multi-channel images.)

Instead of "\n" use '\n', since this is a single character.

cv::imread() returns an invalid image if the reading fails for whatever reason. You should always check the image to make sure it was read.

I am confused as to why enhancement_times needs a templated type. If you make it double, your function doesn’t need to be a template. You later cast it to double anyway (decltype(element) is double). I don’t see anyone using an integer multiplier here that cannot be exactly represented in a double either.

cvtColor is used twice without the cv:: namespace decorator. This works because one of the inputs is a type defined in that namespace, but it’s nice to be explicit about what function you use. Likewise snprintf doesn’t have a namespace.

Answer 2

This function has too many responsibilities, as is evident by the number and variety of arguments. We have mixed filesystem and image processing, making a function that's hard to reuse (e.g. in a GUI program, where we might want to preview the result before saving) and hard to unit-test. If we split it, we have smaller, more readable functions.

---

Other specific issues

We probably don't want to abandon processing when one of the image pairs is unsuccessful - just keep a note of the error and continue.

When we compose the output filename, we're using the wrong format specifier for i - it's a std::size_t, so we should be using %zu (that said, unsigned should be sufficient for the small numbers we're using).

buff is far to generic a name for the variable, and the arbitrary size (100 chars) may be insufficient. It's easy to check the return from std::snprintf() to see whether truncation occurred, yet this isn't done. Personally, I'd use std::ostringstream composition instead.

Logging output should go to std::clog and error messages to std::cerr.

We should cast the result of the multiplication, not the multiplicand. Otherwise values such as 1.5 won't have the expected outcome (and the function wouldn't need to be a template). And we know the type of element is double, so we can use that directly, rather than decltype(). That said, I don't see any reason not to simply use cv::Mat's operator*=().

std::array is more efficient than std::vector for the cv::split.

diffImage follows a different naming pattern than the rest of the variables.

---

Modified code

Incorporating the above advice, but not making full reuse of the image storage:

#include <array>
#include <filesystem>
#include <iostream>
#include <string>

#include <opencv2/core.hpp>
#include <opencv2/imgcodecs.hpp>
#include <opencv2/imgproc.hpp>

cv::Mat read_file(const std::string& filename)
{
    if (!std::filesystem::is_regular_file(filename)) {
        std::clog << filename << " is not a regular file.\n";
        return {};
    }
    auto image = cv::imread(filename, cv::ImreadModes::IMREAD_COLOR);
    if (image.empty()) {
        std::cerr << filename << " could not be decoded.\n";
    }
    return image;
}

template<typename T>
cv::Mat difference_and_enhancement(const cv::Mat& image1, const cv::Mat& image2, T enhancement_times)
{
    //  Difference
    cv::Mat img_diff;
    cv::absdiff(image1, image2, img_diff);
    cv::Mat img_hsv;
    cv::cvtColor(img_diff, img_hsv, cv::COLOR_RGB2HSV);

    std::array<cv::Mat,3> hsv_channels;
    cv::split(img_hsv, hsv_channels);

    hsv_channels[2] *= enhancement_times; // V channel

    cv::merge(hsv_channels, img_hsv);
    cv::cvtColor(img_hsv, img_diff, cv::COLOR_HSV2RGB);
    return img_diff;
}

template<typename T>
unsigned difference_and_enhancement(std::string input_path1, std::string input_path2,
                                    T enhancement_times,
                                    std::string output_path,
                                    bool output_leading_zero, std::string filename_extension = ".png")
{
    //  Clear output
    std::filesystem::remove_all(output_path);
    std::filesystem::create_directory(output_path);

    unsigned error_count = 0;

    for (unsigned i = 1;  i <= 1000;  ++i) {
        auto image1 = read_file(input_path1 + std::to_string(i) + filename_extension);
        if (image1.empty()) continue;
        auto image2 = read_file(input_path2 + std::to_string(i) + filename_extension);
        if (image2.empty()) continue;

        auto output = difference_and_enhancement(image1, image2, enhancement_times);

        std::ostringstream out_name(output_path, std::ios_base::ate);
        if (output_leading_zero) {
            out_name << std::setfill('0') << std::setw(3);
        }
        out_name << i << filename_extension;
        std::clog << out_name.str() << " size = " << output.size() << '\n';
        cv::imwrite(out_name.str(), output);
    }

    return error_count;
}

Answer 3

Naming

The technique looks like unsharp mask, but performed on the most intense channel of RGB pixel (V component). Although unbrighten_mask might sound like a German word (I don't know any German), I believe it better fits since the code performs similar actions.

Unclear usage

At the moment the code has contradictory interface. On one hand, it seems like an utility program, but then the arguments are hardcoded. On the other hand, it cannot be used as a library function as it is too specific.

Usually a function that does something similar would be expected to return by value, but since OpenCV does not really play well with standard C++, it is probably better to just modify the input cv::Mat, as Cris suggested. I'm also unsure about usefulness of the template parameter. If linkage was an issue, static specifier would fit well (the function is too big for inline for my taste).

static void unbrighten_mask(cv::Mat lhs_image, cv::Mat rhs_image, double v_coefficient) {
    /* implementation */
}

Since OpenCV does shallow copy anyway (without atomic reference counting, by the way!), accepting by value should do just fine since users can either pass the object itself or use .clone()

Input validation

The difference function expects the type and the dimensions to be the same. At the moment it seems like the responsibility falls on the user. If OpenCV doesn't have internal validation and good error messages, then notifying about the error would be great.

if (lhs_image.type() != rhs_image.type() || lhs_image.size != rhs_image.size)
{
    throw std::invalid_argument("Either types or dimensions of input images are not the same");
}

Notice that in the original code, type mismatch case is not possible (I believe cv::imread always returns CV_8UC3 typed matrix for colored images), but size check is still needed.

Implementation

Notice that the optimization Cris suggested can only be performed if the input is interleaved layout. It is worth adding documentation to specify that the image is expected to be in RGB interleaved form. YUV formats, for example, do not have interleaved counterparts, they are always planar.

I would go one step further from what Toby said and would use std::format.