CARVIEW |
Navigation Menu
-
-
Notifications
You must be signed in to change notification settings - Fork 56.2k
imgproc: fixed imread with output image argument #25703
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Hi @mshabunin, thanks for the fix! This looks perfect. ;) |
is it too late to change API.
|
@sturkmen72 , this PR doesn't change public API ( |
i mean if it is still possible ( it is newly added function and maybe ABI change policy allow ) |
@@ -462,16 +462,10 @@ imread_( const String& filename, int flags, OutputArray mat ) | |||
// grab the decoded type | |||
const int type = calcType(decoder->type(), flags); | |||
|
|||
if (mat.empty()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this change seems wrong to me.
we should keep
if (mat.empty())
{
mat.create( size.height, size.width, type );
}
else
{
CV_CheckEQ(size, mat.size(), "");
CV_CheckTypeEQ(type, mat.type(), "");
CV_Assert(mat.isContinuous());
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
create
should not reallocate data if requested image size fits into existing Mat.
Mat img(1000, 1000, CV_8UC3);
imread(f, img); // should not reallocate data if image size is less than (1000, 1000)
Without this change it will not be possible to read multiple images of different sizes efficiently, because user would need to pass either empty Mat or a Mat with exact image dimensions on each iteration.
I.e. with proposed change, following code should work and should perform reallocations only when necessary:
Mat img;
for (auto f : filenames)
{
imread(f, img); // will fail if images have different sizes
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry you are right. mat.create( size.height, size.width, type );
does the job.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TickMeter tm;
Mat im;
for (int i = 0; i < 5; i++)
{
tm.reset();
tm.start();
imread("read.png", im);
tm.stop();
printf("read time %f\n", tm.getTimeSec());
}
imshow("read.png", im);
waitKey();
read time 0.692092
read time 0.615938
read time 0.615163
read time 0.615883
read time 0.617650
good job!
@@ -328,7 +344,7 @@ static ImageEncoder findEncoder( const String& _ext ) | |||
} | |||
|
|||
|
|||
static void ExifTransform(int orientation, Mat& img) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
InputOutputArray
is better. I know, that they are synonyms, but it's simpler for reader.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that it won't work without changing public imread
to accept InputOutputArray
(https://docs.opencv.org/4.x/d0/d46/classcv_1_1__InputOutputArray.html). There is no obvious way to construct InputOutputArray
from OutputArray
(https://docs.opencv.org/4.x/d2/d9e/classcv_1_1__OutputArray.html).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Resolves #25694
Replaces #25695
OutputArray
down the call stackNote: we rely on decoders to not recreate the input Mat in their
readData(Mat & img)
method. Otherwise destination image will not be updated just like in the issue.