cudaarithm: use only even number for inplace flip#18348
cudaarithm: use only even number for inplace flip#18348opencv-pushbot merged 1 commit intoopencv:3.4from
Conversation
| CUDA_TEST_P(Flip, AccuracyInplace) | ||
| { | ||
| size.width = (size.width >> 1) << 1; // in-place version only accepts even number | ||
| size.height = (size.height >> 1) << 1; // in-place version only accepts even number |
There was a problem hiding this comment.
I propose to split tests for in-place and general cases and instantiate in-place cases with even size. It's more obvious behavior.
There was a problem hiding this comment.
@asmorkalov , do you mean create DIFFERENT_EVEN_SIZES list here and instantiate inplace tests separately? I think it would make this test unnecessary complex.
There was a problem hiding this comment.
That's something debatable. I first had the same feeling with @asmorkalov , since the log shows 113x113 which is not correct.
Then I saw the the line that @mshabunin pointed, and thought the same thing. "It's only a single test which uses DIFFERENT_EVEN_SIZES "
Merging the idea will lead me to add single define in the test_core.cpp rather in cuda_test.hpp, but I need someone as an arbitor.
There was a problem hiding this comment.
Sorry, but I again changed my mind.
I thought it would be simple, but it's really making a another copy of flip test code just for the in-place version.
Now, there are one Flip test, but asmorkalov suggestion is to create Flip and Flip_inplace test case, which I don't prefer.
Can I keep the current implementation as-is, since it'll make things much easier ?
There was a problem hiding this comment.
Another idea is to keep sizes intact but add bad args handling to the test:
if (/* size is not even */)
{
EXPECT_THROW(...);
return;
}
// current test body| CV_Error(Error::BadROISize, "In-place version of flip only accepts even width/height"); | ||
|
|
||
| if (src.refcount != dst.refcount) | ||
| if (src.data != dst.data) |
There was a problem hiding this comment.
BTW, refcount is a pointer to reference counter which is allocated with underlying buffer/data and it is shared between GpuMats.
At least, it is more reliable check in case of image ROIs.
But it doesn't work if refcount == NULL (non-managed buffers).
Better check should compare both values:
bool isInplace = (src.data == dst.data) || (src.refcount == dst.refcount);
(however there are still some non-covered cases)
resolves #18347
Pull Request Readiness Checklist
See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request
Patch to opencv_extra has the same branch name.