diff --git a/modules/core/src/ocl.cpp b/modules/core/src/ocl.cpp index 0378b66c0..3fa3bd0e3 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,7 +4516,7 @@ public: } else { - // TODO Is it really needed for clCreateBuffer 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), @@ -4545,20 +4547,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 +4607,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 +4662,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 +4744,7 @@ public: } u->data = 0; u->markDeviceCopyObsolete(false); - u->markHostCopyObsolete(false); + u->markHostCopyObsolete(true); return; } #endif @@ -4755,7 +4767,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 9a991d7f3..742e3a2eb 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; @@ -604,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 0e25b6ba9..3640d930f 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); @@ -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[] (unsigned char*)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) @@ -819,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 { @@ -857,9 +936,8 @@ 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++) { @@ -885,7 +963,8 @@ TEST(UMat, DISABLED_bug_with_unmap) } } -TEST(UMat, DISABLED_bug_with_unmap_in_class) + +TEST(UMat, unmap_in_class) { class Logic { @@ -926,7 +1005,29 @@ TEST(UMat, DISABLED_bug_with_unmap_in_class) } } -TEST(UMat, Test_same_behaviour_read_and_read) + +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, DISABLED_Test_same_behaviour_read_and_read) { bool exceptionDetected = false; try @@ -992,3 +1093,6 @@ TEST(UMat, DISABLED_Test_same_behaviour_write_and_write) } ASSERT_TRUE(exceptionDetected); // data race } + + +} } // namespace cvtest::ocl diff --git a/modules/imgproc/src/color.cpp b/modules/imgproc/src/color.cpp index 33ab41f4f..3c3e51255 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;