1 | initial version |
The reason why you're getting a black image is because of your iteration.
By doing dst = Mat::zeros(host.rows, host.cols, CV_8UC3)
, you just created a black image with the same dimensions as host
with three channels.
Matrices are 2D containers i.e. they have rows and columns. With your current loop, you are just iterating from 0 to the product of the rows and cols. You're actually lucky that this iteration hasn't resulted in some sort of segmentation fault. My guess is that your rows is large enough to accommodate the iteration of rows * columns
How to resolve this?
General Case:
You need two loops. One for iterating over the rows and one for each column.
vector<vector<int>> 2dContainer;
for(size_t row = 0; row < 2dContainer.size(); ++row)
{
for(size_t col = 0; col < 2dContainer[row].size(); ++col)
{
cout << 2dContainer[row][col];
}
}
This is generally how you iterate over a 2D container.
OpenCV Case:
Unfortunately the Mat
class does not have the subscript operator; []
, to allow us to access elements as shown below. Instead they have the at instead. And to add to the complexity, your image has three channels so we need to take that into account as well.
This can be done as shown below
for(int row = 0; row < host.rows; ++row)
for(int col = 0; col < host.cols; ++col)
{
dst.at<Vec3b>(row, col)[0] = host.at<Vec3b>(row, col)[0]; // copy first channel
dst.at<Vec3b>(row, col)[1] = host.at<Vec3b>(row, col)[1]; // copy second channel
dst.at<Vec3b>(row, col)[2] = host.at<Vec3b>(row, col)[2]; // copy third channel
}
Simple Alternative:
OpenCV already has an API called copyTo()
which does exactly this for you. All you have to is host.copyTo(dst)
and voila, world peace is attained! :). Here's the documentation about this.
Happy coding!
2 | No.2 Revision |
The reason why you're getting a black image is because of your iteration.
By doing dst = Mat::zeros(host.rows, host.cols, CV_8UC3)
, you just created a black image with the same dimensions as host
with three channels.
Matrices are 2D containers i.e. they have rows and columns. With your current loop, you are just iterating from 0 to the product of the rows and cols. You're actually lucky that this iteration hasn't resulted in some sort of segmentation fault. My guess is that your rows is large enough to accommodate the iteration of rows * columns
How to resolve this?
General Case:
You need two loops. One for iterating over the rows and one for each column.
vector<vector<int>> 2dContainer;
for(size_t row = 0; row < 2dContainer.size(); ++row)
{
for(size_t col = 0; col < 2dContainer[row].size(); ++col)
{
cout << 2dContainer[row][col];
}
}
This is generally how you iterate over a 2D container.
OpenCV Case:
Unfortunately the Mat
class does not have the subscript operator; []
, to allow us to access elements as shown below. Instead they have the at instead. And to add to the complexity, your image has three channels so we need to take that into account as well.
This can be done as shown below
for(int row = 0; row < host.rows; ++row)
for(int col = 0; col < host.cols; ++col)
{
dst.at<Vec3b>(row, col)[0] = host.at<Vec3b>(row, col)[0]; // copy first channel
dst.at<Vec3b>(row, col)[1] = host.at<Vec3b>(row, col)[1]; // copy second channel
dst.at<Vec3b>(row, col)[2] = host.at<Vec3b>(row, col)[2]; // copy third channel
}
Simple Alternative:
OpenCV already has an API called copyTo()
which does exactly this for you. All you have to is host.copyTo(dst)
and voila, world peace is attained! :). Here's the documentation about this.
Code Review:
Why do you have so many waitKeys? Generally speaking a single one is more than enough. Display your stuff then add a single waitKey before your return.
You do not need the getchar()
anymore to leave your windows open. That's exactly what waitKey is doing already.
You do not need to use namedWindow()
for your case. A straight shot imshow()
would suffice.
No need to call destroyWindow()
for each and every newly created window. Display your stuff with imshow()
then call destroyAllWindows()
.
If OpenCV version < 3, please update
. OpenCV is shying away from using the CV
prefix. But if an update is out of the question, then its all good.
Should you choose to adapt the above recommendations, your display code can be trimmed down to just this
imshow("The Host Image", host);
imshow("The dest Image", dest);
JPEG_param.push_back(IMWRITE_JPEG_QUALITY);
JPEG_param.push_back(100);
imwrite("E://PO//123.jpg", dest, JPEG_param);
waitKey(0);
destroyAllWindows();
return 0;
Happy coding!
3 | No.3 Revision |
The reason why you're getting a black image is because of your iteration.
By doing dst = Mat::zeros(host.rows, host.cols, CV_8UC3)
, you just created a black image with the same dimensions as host
with three channels.
Matrices are 2D containers i.e. they have rows and columns. With your current loop, you are just iterating from 0 to the product of the rows and cols. You're actually lucky that this iteration hasn't resulted in some sort of segmentation fault. My guess is that your rows is large enough to accommodate the iteration of rows * columns
How to resolve this?
General Case:
You need two loops. One for iterating over the rows and one for each column.
vector<vector<int>> 2dContainer;
for(size_t row = 0; row < 2dContainer.size(); ++row)
{
for(size_t col = 0; col < 2dContainer[row].size(); ++col)
{
cout << 2dContainer[row][col];
}
}
This is generally how you iterate over a 2D container.
OpenCV Case:
Unfortunately the Mat
class does not have the subscript operator; []
, to allow us to access elements as shown below. above. Instead they have the at instead. . And to add to the complexity, your image has three channels so we need to take that into account as well.
This can be done as shown below
for(int row = 0; row < host.rows; ++row)
for(int col = 0; col < host.cols; ++col)
{
dst.at<Vec3b>(row, col)[0] = host.at<Vec3b>(row, col)[0]; // copy first channel
dst.at<Vec3b>(row, col)[1] = host.at<Vec3b>(row, col)[1]; // copy second channel
dst.at<Vec3b>(row, col)[2] = host.at<Vec3b>(row, col)[2]; // copy third channel
}
Simple Alternative:
OpenCV already has an API called copyTo()
which does exactly this for you. All you have to is host.copyTo(dst)
and voila, world peace is attained! :). Here's the documentation about this.
Code Review:
Why do you have so many waitKeys? Generally speaking a single one is more than enough. Display your stuff then add a single waitKey before your return.
You do not need the getchar()
anymore to leave your windows open. That's exactly what waitKey is doing already.
You do not need to use namedWindow()
for your case. A straight shot imshow()
would suffice.
No need to call destroyWindow()
for each and every newly created window. Display your stuff with imshow()
then call destroyAllWindows()
.
If OpenCV version < 3, please update
. OpenCV is shying away from using the CV
prefix. But if an update is out of the question, then its all good.
Should you choose to adapt the above recommendations, your display code can be trimmed down to just this
imshow("The Host Image", host);
imshow("The dest Image", dest);
JPEG_param.push_back(IMWRITE_JPEG_QUALITY);
JPEG_param.push_back(100);
imwrite("E://PO//123.jpg", dest, JPEG_param);
waitKey(0);
destroyAllWindows();
return 0;
Happy coding!