Image processing for black level and lens shading correction

I don't have a lot of experience in C++; I'm more of a C# guy. I'm trying to convert some Matlab image processing code to C++. I would appreciate any feedback about C++ coding conventions, which data structures to use, what kills performance, and anything which is considered to be good or bad coding style for C++. I'm kind of mixing C and C++ and C# so I wanted to get some feedback.

Preprocessor.h

```
#pragma once
#include
#include
#include
#include
#include "ToolBoxExports.h"
#include "PreprocessorResult.h"

using namespace std;

#include

typedef enum TOOLBOX_EXPORT
{
GrRBGb = 0,
RGrGbB,
BGbGrR,
GbBRGr
} color_order;

typedef class TOOLBOX_EXPORT
{
public:
uint16_t width;
uint16_t height;
color_order order;
uint16_t* raw_data;
uint16_t bit_depth;
} bayer_raw_image;

typedef class TOOLBOX_EXPORT
{
public :
double exposure_time;
double analog_gain;
std::vector black_level;
} black_level_lut;

typedef class TOOLBOX_EXPORT
{
public:

std::vector black_level_luts;
} black_level;

class TOOLBOX_EXPORT Preprocessor
{
public:

PreprocessorResult Function1(bayer_raw_image rawBayerImage, black_level blackLevelDataNative, std::vector saturationLevel, double analogGain, double exposureTime);
PreprocessorResult Function3b(bayer_raw_image rawBayerImage, bayer_raw_image colorCheckerImage, int sensorColorOrder, std::vector saturationLevel);
void Process(std::vector& data, int width, int height, std::vector& output);
void CalculateBlackLevel(black_level* blackLevelDataNative, double analogGain, double exposureTime, double(&output)[4]);
void SeparateChannels(uint16_t* _image, std::vector& gr, std::vector& r, std::vector& b, std::vector& gb, int width, int height, int colorOrder);
void ScaleLscGrid(std::vector& gr, std::vector& r, std::vector& b, std::vector& gb, uint16_t height, uint16_t width, uint16_t desiredWidth, uint16_t desiredHeight, std::string method);
void ApplyLensShadingCorrection

Preprocessor.h

using namespace std;

is always bad style in C++, but especially when you do it in a header (.h) file, because of how C++ handles #include via textual inclusion. When you using namespace std; in a header file, you're forcing that decision on every .cc file that includes your header, which can often result in changed or confusing semantics in those .cc files.

Never use using namespace... in a .h file. Prefer not to use it in a .cc file. Prefer to spell out std::vector, std::sort, et cetera, on every reference, so that your code is clear to the local (human) reader as well as to the compiler.

typedef class TOOLBOX_EXPORT
{
public:
    uint16_t width;
    uint16_t height;
    color_order order;
    uint16_t* raw_data;
    uint16_t bit_depth;
} bayer_raw_image;

Your style here is highly unusual. First of all, you should be aware that in C++, a class is just like a struct in most languages, except that class members (and bases) are private by default instead of public by default. So, instead of writing class Foo { public: ..., it is often more readable to write struct Foo { ....

Also be aware that in C++, unlike in C, you can refer to any type Foo directly; you don't have to qualify the type's name with struct Foo, enum Foo, etc. This means that the C practice of typedef struct _Foo { ... } Foo; is frowned upon in C++.

Lastly, notice that your struct members will be laid out in order; so you could eliminate some padding bytes by shuffling them differently. Result:

struct TOOLBOX_EXPORT bayer_raw_image {
    uint16_t width;
    uint16_t height;
    uint16_t* raw_data;
    uint16_t bit_depth;
    color_order order;
};

I strongly suspect that class Preprocessor should actually be namespace Preprocessor; a class with no data members is highly suspicious. Remember that in C++, unlike Java, it's perfectly fine and normal to have "free functions" existing outside of any class.

Preprocessor.cpp

for (int i = 0; i < channelCCHeight; i++)
{
    for (int j = 0; j < channelCCWidth; j++)
    {
        floatChannel_gr[i*channelCCWidth + j] = floatChannel_gr[i*channelCCWidth + j] * channel_gr_cc[i*channelCCWidth + j];
        floatChannel_r[i*channelCCWidth + j] = floatChannel_r[i*channelCCWidth + j] * channel_r_cc[i*channelCCWidth + j];
        floatChannel_b[i*channelCCWidth + j] = floatChannel_b[i*channelCCWidth + j] * channel_b_cc[i*channelCCWidth + j];
        floatChannel_gb[i*channelCCWidth + j] = floatChannel_gb[i*channelCCWidth + j] * channel_gb_cc[i*channelCCWidth + j];
    }
}

You should know that in basically all curly-brace languages, x = x y can be rewritten as x = y.

In C++, operators can be overloaded; including the ++ prefix and postfix operators. The prefix version means "increment my value and return me"; the postfix version means "copy me, increment my value, and return the copy." Therefore, in general prefix ++ will perform fewer operations and be more optimizable than postfix ++ (and the same goes for --). It certainly won't matter for primitive types like int, but if you get in the habit of writing ++it in general, then you'll never have to stop and think about this issue ever again. Result:

for (int i = 0; i < channelCCHeight; ++i) {
    for (int j = 0; j < channelCCWidth; ++j) {
        floatChannel_gr[i*channelCCWidth + j] *= channel_gr_cc[i*channelCCWidth + j];
        floatChannel_r[i*channelCCWidth + j] *= channel_r_cc[i*channelCCWidth + j];
        floatChannel_b[i*channelCCWidth + j] *= channel_b_cc[i*channelCCWidth + j];
        floatChannel_gb[i*channelCCWidth + j] *= channel_gb_cc[i*channelCCWidth + j];
    }
}

No different in the code generated, but significantly easier on the eyes, and there's several fewer places for typos to lurk. You could make it even simpler, of course:

for (int i = 0; i < channelCCHeight * channelCCWidth; ++i) {
    floatChannel_gr[i] *= channel_gr_cc[i];
    floatChannel_r[i] *= channel_r_cc[i];
    floatChannel_b[i] *= channel_b_cc[i];
    floatChannel_gb[i] *= channel_gb_cc[i];
}

Again, this shouldn't result in any better codegen (for any halfway decent compiler); but your human reader will thank you.

void Preprocessor::ShowImage(std::vector channel, uint16_t width, uint16_t height, std::string title)
{
    cv::Mat image(height, width, CV_32F);
    for (int i = 0; i (i, j) = channel[i*width + j] / 1023.0;
        }
    }
    cv::imshow(title, image);
    cv::waitKey(0);
}

The expression channel[i*width + j] / 1023.0 struck me as odd. For one thing, 1023.0 is a double; if you meant to do the arithmetic in single-precision, you should have said 1023.0f or simply 1023 (which would be an int, which would promote to double).

For another thing, I think that if you're trying to convert an integer in the range [0,1023] to a float in the range [0,1], that expression isn't what you want; for the same reason that if you w

StackExchange Code Review Q#140418, answer score: 5