From ed7907e46c8e2c496eb4559a61c72620a9acfe4e Mon Sep 17 00:00:00 2001 From: Vladimir Dudnik Date: Mon, 20 Jul 2015 15:22:23 +0300 Subject: [PATCH 1/6] add test for bug 4006: #4862 --- modules/core/test/test_umat.cpp | 78 +++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/modules/core/test/test_umat.cpp b/modules/core/test/test_umat.cpp index 0e25b6ba9a..f46fc8cee5 100644 --- a/modules/core/test/test_umat.cpp +++ b/modules/core/test/test_umat.cpp @@ -522,6 +522,84 @@ TEST_P(UMatTestUMatOperations, diag) INSTANTIATE_TEST_CASE_P(UMat, UMatTestUMatOperations, Combine(OCL_ALL_DEPTHS, OCL_ALL_CHANNELS, UMAT_TEST_SIZES, Bool())); + +/////////////////////////////////////////////////////////////// getUMat -> GetMat /////////////////////////////////////////////////////////////////// + +PARAM_TEST_CASE(getUMat, int, int, Size, bool) +{ + int type; + Size size; + + virtual void SetUp() + { + int depth = GET_PARAM(0); + int cn = GET_PARAM(1); + size = GET_PARAM(2); + useOpenCL = GET_PARAM(3); + + type = CV_MAKE_TYPE(depth, cn); + + isOpenCL_enabled = cv::ocl::useOpenCL(); + cv::ocl::setUseOpenCL(useOpenCL); + } + + virtual void TearDown() + { + cv::ocl::setUseOpenCL(isOpenCL_enabled); + } + +private: + bool useOpenCL; + bool isOpenCL_enabled; +}; + +// UMat created from user allocated host memory (USE_HOST_PTR) +TEST_P(getUMat, custom_ptr) +{ + void* pData = new unsigned char [size.area() * CV_ELEM_SIZE(type)]; + size_t step = size.width * CV_ELEM_SIZE(type); + + Mat m = Mat(size, type, pData, step); + m.setTo(cv::Scalar::all(2)); + + UMat u = m.getUMat(ACCESS_RW); + cv::add(u, cv::Scalar::all(2), u); + + Mat d = u.getMat(ACCESS_READ); + + Mat expected(m.size(), m.type(), cv::Scalar::all(4)); + double norm = cvtest::norm(d, expected, NORM_INF); + + EXPECT_EQ(0, norm); + + delete[] pData; +} + +TEST_P(getUMat, self_allocated) +{ + Mat m = Mat(size, type); + m.setTo(cv::Scalar::all(2)); + + UMat u = m.getUMat(ACCESS_RW); + cv::add(u, cv::Scalar::all(2), u); + + Mat d = u.getMat(ACCESS_READ); + + Mat expected(m.size(), m.type(), cv::Scalar::all(4)); + double norm = cvtest::norm(d, expected, NORM_INF); + + EXPECT_EQ(0, norm); +} + +INSTANTIATE_TEST_CASE_P(UMat, getUMat, Combine( + Values(CV_8U), // depth + Values(1, 3), // channels + Values(cv::Size(1, 1), cv::Size(255, 255), cv::Size(256, 256)), // Size + Bool() // useOpenCL +)); + + + ///////////////////////////////////////////////////////////////// OpenCL //////////////////////////////////////////////////////////////////////////// TEST(UMat, BufferPoolGrowing) From b36f565d13558936c5634e72d76c62a20cee8be9 Mon Sep 17 00:00:00 2001 From: Alexander Alekhin Date: Fri, 24 Jul 2015 19:10:31 +0300 Subject: [PATCH 2/6] fix OpenCV code (bug 4006: #4862) --- modules/core/src/ocl.cpp | 33 +++++++++++++++++++-------------- modules/core/src/umatrix.cpp | 6 ++++-- modules/core/test/test_umat.cpp | 2 +- 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/modules/core/src/ocl.cpp b/modules/core/src/ocl.cpp index 0378b66c04..9e54da391c 100644 --- a/modules/core/src/ocl.cpp +++ b/modules/core/src/ocl.cpp @@ -4320,6 +4320,7 @@ public: u->flags = flags0; u->allocatorFlags_ = allocatorFlags; CV_DbgAssert(!u->tempUMat()); // for bufferPool.release() consistency in deallocate() + u->markHostCopyObsolete(true); return u; } @@ -4460,6 +4461,7 @@ public: CV_Assert(u->handle != 0 && u->urefcount == 0); if(u->tempUMat()) { + CV_Assert(u->origdata); // UMatDataAutoLock lock(u); if( u->hostCopyObsolete() && u->refcount > 0 ) @@ -4514,14 +4516,7 @@ public: } else { - // TODO Is it really needed for clCreateBuffer with CL_MEM_USE_HOST_PTR? - cl_int retval = 0; - void* data = clEnqueueMapBuffer(q, (cl_mem)u->handle, CL_TRUE, - (CL_MAP_READ | CL_MAP_WRITE), - 0, u->size, 0, 0, 0, &retval); - CV_OclDbgAssert(retval == CL_SUCCESS); - CV_OclDbgAssert(clEnqueueUnmapMemObject(q, (cl_mem)u->handle, data, 0, 0, 0) == CL_SUCCESS); - CV_OclDbgAssert(clFinish(q) == CL_SUCCESS); + // nothing with CL_MEM_USE_HOST_PTR } } u->markHostCopyObsolete(false); @@ -4545,20 +4540,27 @@ public: clReleaseMemObject((cl_mem)u->handle); } u->handle = 0; + u->markDeviceCopyObsolete(true); u->currAllocator = u->prevAllocator; - if(u->data && u->copyOnMap() && !(u->flags & UMatData::USER_ALLOCATED)) + u->prevAllocator = NULL; + if(u->data && u->copyOnMap() && u->data != u->origdata) fastFree(u->data); u->data = u->origdata; if(u->refcount == 0) + { u->currAllocator->deallocate(u); + u = NULL; + } } else { + CV_Assert(u->origdata == NULL); CV_Assert(u->refcount == 0); - if(u->data && u->copyOnMap() && !(u->flags & UMatData::USER_ALLOCATED)) + if(u->data && u->copyOnMap() && u->data != u->origdata) { fastFree(u->data); u->data = 0; + u->markHostCopyObsolete(true); } if (u->allocatorFlags_ & ALLOCATOR_FLAGS_BUFFER_POOL_USED) { @@ -4598,8 +4600,11 @@ public: clReleaseMemObject((cl_mem)u->handle); } u->handle = 0; + u->markDeviceCopyObsolete(true); delete u; + u = NULL; } + CV_Assert(u == NULL || u->refcount); } void map(UMatData* u, int accessFlags) const @@ -4650,9 +4655,9 @@ public: return; } #endif - if (u->data) // FIXIT Workaround for UMat synchronization issue + if (!u->hostCopyObsolete()) // FIXIT Workaround for UMat synchronization issue { - //CV_Assert(u->hostCopyObsolete() == false); + CV_Assert(u->data); return; } @@ -4732,7 +4737,7 @@ public: } u->data = 0; u->markDeviceCopyObsolete(false); - u->markHostCopyObsolete(false); + u->markHostCopyObsolete(true); return; } #endif @@ -4755,7 +4760,7 @@ public: u->size, alignedPtr.getAlignedPtr(), 0, 0, 0)) == CL_SUCCESS ); } u->markDeviceCopyObsolete(false); - u->markHostCopyObsolete(false); + u->markHostCopyObsolete(true); } bool checkContinuous(int dims, const size_t sz[], diff --git a/modules/core/src/umatrix.cpp b/modules/core/src/umatrix.cpp index 48aa86635d..da2ee840a1 100644 --- a/modules/core/src/umatrix.cpp +++ b/modules/core/src/umatrix.cpp @@ -221,6 +221,7 @@ UMat Mat::getUMat(int accessFlags, UMatUsageFlags usageFlags) const UMat hdr; if(!data) return hdr; + accessFlags |= ACCESS_RW; UMatData* temp_u = u; if(!temp_u) { @@ -228,7 +229,6 @@ UMat Mat::getUMat(int accessFlags, UMatUsageFlags usageFlags) const if(!a) a = a0; temp_u = a->allocate(dims, size.p, type(), data, step.p, accessFlags, usageFlags); - temp_u->refcount = 1; } UMat::getStdAllocator()->allocate(temp_u, accessFlags, usageFlags); // TODO result is not checked hdr.flags = flags; @@ -584,7 +584,9 @@ Mat UMat::getMat(int accessFlags) const { if(!u) return Mat(); - u->currAllocator->map(u, accessFlags | ACCESS_READ); // TODO Support ACCESS_WRITE without unnecessary data transfers + // TODO Support ACCESS_READ (ACCESS_WRITE) without unnecessary data transfers + accessFlags |= ACCESS_RW; + u->currAllocator->map(u, accessFlags); CV_Assert(u->data != 0); Mat hdr(dims, size.p, type(), u->data + offset, step.p); hdr.flags = flags; diff --git a/modules/core/test/test_umat.cpp b/modules/core/test/test_umat.cpp index f46fc8cee5..2e7555494b 100644 --- a/modules/core/test/test_umat.cpp +++ b/modules/core/test/test_umat.cpp @@ -572,7 +572,7 @@ TEST_P(getUMat, custom_ptr) EXPECT_EQ(0, norm); - delete[] pData; + delete[] (unsigned char*)pData; } TEST_P(getUMat, self_allocated) From cd5c70691a6befa9b632a0a27fb752bd4daf3bdb Mon Sep 17 00:00:00 2001 From: Alexander Alekhin Date: Mon, 27 Jul 2015 14:55:06 +0300 Subject: [PATCH 3/6] ocl: add map tests --- modules/core/src/umatrix.cpp | 2 +- modules/core/test/test_umat.cpp | 31 +++++++++++++++++++++++++++---- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/modules/core/src/umatrix.cpp b/modules/core/src/umatrix.cpp index da2ee840a1..917b339a9e 100644 --- a/modules/core/src/umatrix.cpp +++ b/modules/core/src/umatrix.cpp @@ -606,7 +606,7 @@ void* UMat::handle(int accessFlags) const // check flags: if CPU copy is newer, copy it back to GPU. if( u->deviceCopyObsolete() ) { - CV_Assert(u->refcount == 0); + CV_Assert(u->refcount == 0 || u->origdata); u->currAllocator->unmap(u); } diff --git a/modules/core/test/test_umat.cpp b/modules/core/test/test_umat.cpp index 2e7555494b..ce0a6ab399 100644 --- a/modules/core/test/test_umat.cpp +++ b/modules/core/test/test_umat.cpp @@ -935,9 +935,7 @@ TEST(UMat, DISABLED_synchronization_map_unmap) } } -} } // namespace cvtest::ocl - -TEST(UMat, DISABLED_bug_with_unmap) +TEST(UMat, async_unmap) { for (int i = 0; i < 20; i++) { @@ -963,7 +961,7 @@ TEST(UMat, DISABLED_bug_with_unmap) } } -TEST(UMat, DISABLED_bug_with_unmap_in_class) +TEST(UMat, unmap_in_class) { class Logic { @@ -1004,6 +1002,28 @@ TEST(UMat, DISABLED_bug_with_unmap_in_class) } } + +TEST(UMat, map_unmap_counting) +{ + if (!cv::ocl::useOpenCL()) + { + std::cout << "OpenCL is not enabled. Skip test" << std::endl; + return; + } + std::cout << "Host memory: " << cv::ocl::Device::getDefault().hostUnifiedMemory() << std::endl; + Mat m(Size(10, 10), CV_8UC1); + UMat um = m.getUMat(ACCESS_RW); + { + Mat d = um.getMat(ACCESS_RW); + d.release(); + } + void* h = NULL; + EXPECT_NO_THROW(h = um.handle(ACCESS_RW)); + std::cout << "Handle: " << h << std::endl; +} + + + TEST(UMat, Test_same_behaviour_read_and_read) { bool exceptionDetected = false; @@ -1070,3 +1090,6 @@ TEST(UMat, DISABLED_Test_same_behaviour_write_and_write) } ASSERT_TRUE(exceptionDetected); // data race } + + +} } // namespace cvtest::ocl From 9bcccb028b1321f02b228451db41321b449cb98c Mon Sep 17 00:00:00 2001 From: Alexander Alekhin Date: Mon, 27 Jul 2015 17:04:19 +0300 Subject: [PATCH 4/6] fixes --- modules/core/src/ocl.cpp | 9 ++++++++- modules/core/test/test_umat.cpp | 7 +++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/modules/core/src/ocl.cpp b/modules/core/src/ocl.cpp index 9e54da391c..3fa3bd0e37 100644 --- a/modules/core/src/ocl.cpp +++ b/modules/core/src/ocl.cpp @@ -4516,7 +4516,14 @@ public: } else { - // nothing with CL_MEM_USE_HOST_PTR + // CL_MEM_USE_HOST_PTR (nothing is required) and OTHER cases + cl_int retval = 0; + void* data = clEnqueueMapBuffer(q, (cl_mem)u->handle, CL_TRUE, + (CL_MAP_READ | CL_MAP_WRITE), + 0, u->size, 0, 0, 0, &retval); + CV_OclDbgAssert(retval == CL_SUCCESS); + CV_OclDbgAssert(clEnqueueUnmapMemObject(q, (cl_mem)u->handle, data, 0, 0, 0) == CL_SUCCESS); + CV_OclDbgAssert(clFinish(q) == CL_SUCCESS); } } u->markHostCopyObsolete(false); diff --git a/modules/core/test/test_umat.cpp b/modules/core/test/test_umat.cpp index ce0a6ab399..958f0c3d12 100644 --- a/modules/core/test/test_umat.cpp +++ b/modules/core/test/test_umat.cpp @@ -897,8 +897,9 @@ TEST(UMat, ReadBufferRect) EXPECT_MAT_NEAR(t, t2, 0); } + // Use iGPU or OPENCV_OPENCL_DEVICE=:CPU: to catch problem -TEST(UMat, DISABLED_synchronization_map_unmap) +TEST(UMat, synchronization_map_unmap) { class TestParallelLoopBody : public cv::ParallelLoopBody { @@ -935,6 +936,7 @@ TEST(UMat, DISABLED_synchronization_map_unmap) } } + TEST(UMat, async_unmap) { for (int i = 0; i < 20; i++) @@ -961,6 +963,7 @@ TEST(UMat, async_unmap) } } + TEST(UMat, unmap_in_class) { class Logic @@ -1024,7 +1027,7 @@ TEST(UMat, map_unmap_counting) -TEST(UMat, Test_same_behaviour_read_and_read) +TEST(UMat, DISABLED_Test_same_behaviour_read_and_read) { bool exceptionDetected = false; try From 4eef486afec42bb1ffcc97e8b82e112acfe8af26 Mon Sep 17 00:00:00 2001 From: Alexander Alekhin Date: Tue, 28 Jul 2015 11:48:20 +0300 Subject: [PATCH 5/6] tapi: datarace fixup for cvtColor --- modules/imgproc/src/color.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/imgproc/src/color.cpp b/modules/imgproc/src/color.cpp index 33ab41f4f8..3c3e512555 100644 --- a/modules/imgproc/src/color.cpp +++ b/modules/imgproc/src/color.cpp @@ -8425,6 +8425,7 @@ void cv::cvtColor( InputArray _src, OutputArray _dst, int code, int dcn ) CV_Assert( dcn == 1 ); CV_Assert( scn == 2 && depth == CV_8U ); + src.release(); // T-API datarace fixup extractChannel(_src, _dst, code == CV_YUV2GRAY_UYVY ? 1 : 0); } break; From 1704aea6a0420e4f507553b57162496589757cf7 Mon Sep 17 00:00:00 2001 From: Alexander Alekhin Date: Thu, 30 Jul 2015 16:02:58 +0300 Subject: [PATCH 6/6] tapi: enable some disabled tests --- modules/core/test/test_umat.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/core/test/test_umat.cpp b/modules/core/test/test_umat.cpp index 958f0c3d12..3640d930f5 100644 --- a/modules/core/test/test_umat.cpp +++ b/modules/core/test/test_umat.cpp @@ -175,11 +175,11 @@ TEST_P(UMatBasicTests, base) TEST_P(UMatBasicTests, DISABLED_copyTo) { - UMat roi_ua; - Mat roi_a; int i; if(useRoi) { + UMat roi_ua; + Mat roi_a; roi_ua = UMat(ua, roi); roi_a = Mat(a, roi); roi_a.copyTo(roi_ua); @@ -230,7 +230,7 @@ TEST_P(UMatBasicTests, DISABLED_copyTo) } } -TEST_P(UMatBasicTests, DISABLED_GetUMat) +TEST_P(UMatBasicTests, GetUMat) { if(useRoi) { @@ -284,7 +284,7 @@ PARAM_TEST_CASE(UMatTestReshape, int, int, Size, bool) } }; -TEST_P(UMatTestReshape, DISABLED_reshape) +TEST_P(UMatTestReshape, reshape) { a = randomMat(size,type, -100, 100); a.copyTo(ua);