9
\$\begingroup\$

This code is supposed to learn colors from many many data images, and then recognize color using an algorithm that I made. Eventually, I want the program to make its own algorithm.

Data input and test cases are on GitHub.

And now, the actual code:

// By Dat HA

#include "stdafx.h" // visual studio mandatory include file

#include <opencv2\opencv.hpp> // open cv libraries
#include <opencv2/core/core.hpp>
#include <opencv2/imgcodecs.hpp>
#include <opencv2/highgui/highgui.hpp>

#include <Windows.h> // windows library

#include <fstream> // std libraries
#include <ios>
#include <iostream>
#include <string>
#include <vector>

const int NUM_FILE = 10; // number of images per color, NUM_FILE & NUM_COLOR are temporary until I get Boost working
const int NUM_COLOR = 7; // number of colors
const float NUM_VERSION = 1.3; // version number
const std::string TRAIN_DATA_FOLDER = "../TrainData/"; // training data location

struct Color { // define a new color that we learned of compare
    std::string colorName; // name of the color, ex. red, blue
    cv::Scalar bgr; // blue, green, and red values in that order
    cv::Scalar difference; // what is the difference between, blue and green, green and red, and red and blue
};

cv::Scalar getAvg(std::vector<cv::Scalar> imgData) { // get the average BGR of a vector of images BGR value
    cv::Scalar avg = { 0,0,0,0 }; // new scalar
    int num = imgData.size(); // size of vector
    for (int rgb = 0; rgb < 3; rgb++) { // cycle through the colors
        for (int i = 0; i < num; i++) // cycle through the pictures
            avg[rgb] += imgData[i][rgb]; // add them up
        avg[rgb] /= num; // divide them by the total
    }
    return avg; // return the average
}

cv::Scalar getBgrDifference(cv::Scalar bgr) { // get the difference between, blue and green, green and red, and red and blue
    cv::Scalar difference; // new scalar
    difference[0] = bgr[0] - bgr[1]; // difference between blue and green
    difference[1] = bgr[1] - bgr[2]; // difference between green and red
    difference[2] = bgr[2] - bgr[0]; // difference between red and blue
    return difference; // return the difference scalar
}

void training(std::vector<Color> &color) { // train the neural network
    for (int j = 0; j < NUM_COLOR; j++) { // cycle through the colors
        std::ifstream file; // new file
        std::string nfname = TRAIN_DATA_FOLDER + std::to_string(j) + "/name.txt"; // file name, contains the color name
        file.open(nfname); // open text file
        std::string colorName; // create string for the color name
        file >> colorName; // get the string frmo text file to color name variable
        file.close(); // close text file
        std::vector<cv::Scalar> imgData; // vector of image BGRs in a scalar format
        for (int i = 0; i < NUM_FILE; i++) { // cycle through the images' data
            std::string fname = TRAIN_DATA_FOLDER + std::to_string(j) + "/" + std::to_string(i) + ".jpg"; // get the image file name
            cv::Mat image = cv::imread(fname, cv::IMREAD_COLOR); // read the image
            cv::Scalar imgBgr = cv::mean(image); // get the image's average BGR value
            imgData.push_back(imgBgr); // add it to vector
            cv::waitKey(1); // wait a bit
        }
        Color currentColor; // create a temporary new color
        currentColor.colorName = colorName; // set its name
        currentColor.bgr = getAvg(imgData); // set its BGR value
        currentColor.difference = getBgrDifference(currentColor.bgr); // set its difference value
        color.push_back(currentColor); // add temporary color to our main color vector
        std::cout << color[j].colorName << " : " << color[j].bgr << std::endl; // print the color name and its BGR value
    }
    std::cout << std::endl; // jump a line
}

double getColorAccuracy(cv::Scalar color1, cv::Scalar color2) { // get, in percentage, the ressemblance between 2 color
    double accuracy = 0; // create a double for our accuracy
    for (int i = 0; i < 3; i++) // cycle throught all 3 differences
        accuracy += fabs(color1[i] - color2[i]); // add them up
    return 1 - ((accuracy / 3) / 255); // divide and conquer them!, just kidding, divide and return it
}

Color getColorGuest(std::vector<Color> color, cv::Mat image) { // guest the color
    cv::Scalar imgBgr = cv::mean(image); // get average BGR value of image
    cv::Scalar imgDifference = getBgrDifference(imgBgr); // get BGR's difference
    std::vector<double> accuracy; // create a vector for accuracy, higher the better

    for (int i = 0; i < color.size(); i++) // cycle through colors that we learned
        accuracy.push_back((getColorAccuracy(imgDifference, color[i].difference))); // add that color's ressemblence, in percentage, to the know color

    Color bestColor = color[std::distance(accuracy.begin(), std::find(accuracy.begin(), accuracy.end(), *std::max_element(accuracy.begin(), accuracy.end())))]; // get the best match color

    std::cout << imgBgr << std::endl; // print the image average BGR value
    for (int i = 0; i < color.size(); i++) // cycle through the colors that we learned
        std::cout << color[i].colorName << " : " << accuracy[i] << std::endl; // print color name and how accurate it is
    std::cout << bestColor.colorName << std::endl << std::endl; // print out the best match
    return bestColor; // return the best match color
}

// main
int main() {
    std::cout << NUM_VERSION << std::endl << std::endl; // print code version

    std::vector<Color> color; // color vector
    training(color);          // train neural net and store learned color in vector

    // TESTTING SEGMENTS

    getColorGuest(color, cv::imread("../TestData/yellow.jpg", cv::IMREAD_COLOR)); // get the best match color for our image
    while (1);
}
\$\endgroup\$

2 Answers 2

8
\$\begingroup\$

Fix the includes

Don't include headers you're not using. I had to remove a bunch of headers that aren't available here, simply to get your code to compile.

I then had to include the math header for the use of fabs in getColorAccuracy() - which I then changed to std::abs, so <cmath>.

I then had:

#include <opencv2/core/core.hpp>
#include <opencv2/highgui/highgui.hpp>

#include <cmath>
#include <fstream>
#include <iostream>
#include <string>
#include <vector>

Comments should explain the code

Throughout the program, your comments duplicate what the code says. Good comments explain why rather than what, and most of the commentary is through good use of names. For example:

cv::Scalar getAvg(std::vector<cv::Scalar> imgData) { // get the average BGR of a vector of images BGR value
    cv::Scalar avg = { 0,0,0,0 }; // new scalar
    int num = imgData.size(); // size of vector
    for (int rgb = 0; rgb < 3; rgb++) { // cycle through the colors
        for (int i = 0; i < num; i++) // cycle through the pictures
            avg[rgb] += imgData[i][rgb]; // add them up
        avg[rgb] /= num; // divide them by the total
    }
    return avg; // return the average
}

We know that imgData.size() returns the size of a vector, so no need to comment that. It's much more important, for example, to explain why we loop over the RGB components in the outer loop rather than in the inner loop.

Use range-based for on containers

In getAvg(), we don't need to count elements if we use range-based for (and this eliminates a dubious conversion in int num = imgData.size()):

cv::Scalar getAvg(const std::vector<cv::Scalar>& imgData)
{
    cv::Scalar avg{ 0 };
    for (auto const& img: imgData) {
        avg += img;
    }

    double const n = imgData.size();
    return avg / n;
}

Here, I've also made use of the operator overloads += and / provided by cv::Scalar to perform elementwise arithmetic without needing to loop over the RGBA components within the Scalar.

Note also that this function can accept the vector by const reference, as we are not modifying it and do not need to copy.

Create and open ifstream in a single step

Instead of creating a default-constructed stream, we can start with it open, by passing the filename to its constructor:

    std::string colorName;
    {
        std::ifstream file{TRAIN_DATA_FOLDER + std::to_string(j) + "/name.txt"};
        file >> colorName;
    }

It's not necessary to explicitly call file.close(), as the destructor takes care of that for us.

Reduce copying of Color objects

We can use std::vector::emplace_back to construct a Color directly into the vector:

    color.emplace_back(colorName,
                       getAvg(imgData),
                       getBgrDifference(getAvg(imgData)));

I'll change the Color constructor to accept two arguments and compute the difference, which will eliminate the second call to getAvg() - see the complete code at the end of answer.

A simple typo

I think that Guest should be Guess!

Inefficient search

The getColorGuess() is the only function that uses C++ algorithms, but this line looks quite dubious:

Color bestColor = color[std::distance(accuracy.begin(),
                                      std::find(accuracy.begin(),
                                                accuracy.end(), 
                                                *std::max_element(accuracy.begin(), accuracy.end())))];

Having found an iterator to the maximum value, there's no need to dereference it and pass it to find to get the same iterator back again. It's functionally equivalent to

Color bestColor = color[std::distance(accuracy.begin(),
                                      std::max_element(accuracy.begin(), accuracy.end()))];

We can do still better, though, as we can find the maximum value directly on the color vector, by telling std::max_element how to do the calculation:

auto it = std::max_element(color.begin(),
                           color.end(),
                           [imgDifference](const Color& a, const Color& b) {
                               return getColorAccuracy(imgDifference, a.difference) < getColorAccuracy(imgDifference, b.difference);
                           });
// *it is a reference to a const Color in the vector

Busy loop

This is rude to any other process (and to those of us who prefer a cooler environment):

while (1);

This never terminates. Just remove it.

Re-worked code

#include <opencv2/core/core.hpp>
#include <opencv2/highgui/highgui.hpp>

#include <algorithm>
#include <cmath>
#include <fstream>
#include <iostream>
#include <string>
#include <vector>

const int NUM_FILE = 10; // number of images per color, NUM_FILE & NUM_COLOR are temporary until I get Boost working
const int NUM_COLOR = 7; // number of colors
const float NUM_VERSION = 1.3; // version number
const std::string TRAIN_DATA_FOLDER = "../TrainData/"; // training data location

cv::Scalar getBgrDifference(const cv::Scalar& bgr);

struct Color {
    std::string colorName;
    cv::Scalar bgr;
    cv::Scalar difference;
    Color(std::string, cv::Scalar bgr)
        : colorName{colorName},
          bgr{bgr},
          difference{getBgrDifference(bgr)}
    {}
};

cv::Scalar getAvg(const std::vector<cv::Scalar>& imgData)
{
    cv::Scalar avg{ 0 };
    for (auto const& img: imgData) {
        avg += img;
    }

    double const n = imgData.size();
    return avg / n;
}

cv::Scalar getBgrDifference(const cv::Scalar& bgr) {
    // difference between each pair of components
    return {bgr[0] - bgr[1], // difference between blue and green
            bgr[1] - bgr[2], // difference between green and red
            bgr[2] - bgr[0]}; // difference between red and blue
}

std::vector<Color> training()
{
    std::vector<Color> color;
    for (int j = 0;  j < NUM_COLOR;  ++j) {
        std::string colorName;
        {
            std::ifstream file{TRAIN_DATA_FOLDER + std::to_string(j) + "/name.txt"};
            file >> colorName;
        }
        std::vector<cv::Scalar> imgData;
        imgData.reserve(NUM_FILE);
        for (int i = 0;  i < NUM_FILE;  ++i) {
            std::string const fname = TRAIN_DATA_FOLDER + std::to_string(j) + "/" + std::to_string(i) + ".jpg";
            cv::Mat const image = cv::imread(fname, cv::IMREAD_COLOR);
            imgData.push_back(cv::mean(image));
        }
        auto const mean = getAvg(imgData);
        color.emplace_back(colorName, mean);
        std::cout << color[j].colorName << " : " << color[j].bgr << std::endl;
    }
    std::cout << std::endl; // blank line to separate from next color files
    return color;
}

double getColorAccuracy(const cv::Scalar& color1, const cv::Scalar& color2)
{
    // similarity between two colors, on a scale of 0 (very different) to 1 (identical)
    double accuracy = 0;
    const auto diff = color1 - color2;
    for (int i = 0; i < 3; i++)
        accuracy += std::abs(diff[i]);
    return 1 - ((accuracy / 3) / 255); // divide and conquer them!, just kidding, divide and return it
}

const Color& getColorGuess(const std::vector<Color>& color, const cv::Mat& image)
{ // guess the color
    cv::Scalar imgBgr = cv::mean(image);
    cv::Scalar imgDifference = getBgrDifference(imgBgr);

    auto it = std::max_element(color.begin(),
                               color.end(),
                               [imgDifference](const Color& a, const Color& b) {
                                   return getColorAccuracy(imgDifference, a.difference) < getColorAccuracy(imgDifference, b.difference);
                               });

    std::cout << imgBgr << " matches " << it->colorName << std::endl;
    return *it;
}

// main
int main() {
    std::cout << NUM_VERSION << std::endl << std::endl;

    std::vector<Color> color = training();

    getColorGuess(color, cv::imread("../TestData/yellow.jpg", cv::IMREAD_COLOR));
}

Further ideas

  • You might want to encapsulate the trained recogniser into an object.
  • Consider separating out the parts that write to std::cout so you can write a silent program that just does its job cleanly.
\$\endgroup\$
0
3
\$\begingroup\$

The first thing I do not get is here:

struct Color { // define a new color that we learned of compare
    std::string colorName; // name of the color, ex. red, blue
    cv::Scalar bgr; // blue, green, and red values in that order
    cv::Scalar difference; // what is the difference between, blue and green, green and red, and red and blue
};

How can three values be a scalar? how can the difference between these two be a scalar? It is obviously not your code, but it confuses me on the spot.


Please use descriptive names. The amount time saved by typing getAvg rather than getAverage is small compared to the time you need to read them properly once more getFoo functions fly around.


You are always passing the data by copy

cv::Scalar getAvg(std::vector<cv::Scalar> imgData)

This is really slow and you should definitely pass it by reference or const reference depending on your need.

Also the getAvg function should not modify your data so it should be const too. In Summary your function signature should look like this

cv::Scalar getAverage(const std::vector<cv::Scalar>& imgData) const

Not your library, but without words

cv::Scalar avg = { 0,0,0,0 }; // new scalar

Your getAverage function seems dubious. The strange part is

cv::Scalar getAvg(std::vector<cv::Scalar> imgData) { // get the average BGR of a vector of images BGR value
    cv::Scalar avg = { 0,0,0,0 }; // new scalar
    for (int rgb = 0; rgb < 3; rgb++) { // cycle through the colors
    ...
}

avg is a scalar of size 4 but you only iterate the first 3 elements? So why is it of size 4 when there are only 3 colors? You should either fix this or add a good explanation.


In your training routine you are loading the image data. This should be a separate function. Generally try to encapsulate your functionality better.


Your getColorAccuracy() function is kind of strange. You are passing a vector (scalar) and then an element to a scalar and then pushing that back into a vector. Just vectorize the whole function, so that it returns a std::vector<double>.


Cool but seems like an overkill:

Color bestColor = color[std::distance(accuracy.begin(), 
                                      std::find(accuracy.begin(), 
                                                accuracy.end(),
                                                *std::max_element(accuracy.begin(), 
                                                                  accuracy.end())))]; // get the best match color

Especially as max_element returns the iterator to the greatest element. So what you are doing is finding the pointer of the maximal element, dereferencing it to find that maximal element and get its pointer back. That should do it too:

Color bestColor = color[std::distance(accuracy.begin(),
                                      std::max_element(accuracy.begin(), 
                                                       accuracy.end())))]; // get the best match color

I assume you mean Guess?

Color getColorGuest(std::vector<Color> color, cv::Mat image) { // guest the color
\$\endgroup\$
2
  • \$\begingroup\$ The type Scalar is widely used in OpenCV for passing pixel values. It is not necessary to define the last argument if it is not going to be used. \$\endgroup\$
    – Maikel
    Commented May 9, 2017 at 14:39
  • 1
    \$\begingroup\$ Actually, the color struct was from me. I didn't copy it from anybody. \$\endgroup\$
    – Dat
    Commented May 9, 2017 at 15:35

Your Answer

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

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