From b36f565d13558936c5634e72d76c62a20cee8be9 Mon Sep 17 00:00:00 2001 From: Alexander Alekhin Date: Fri, 24 Jul 2015 19:10:31 +0300 Subject: [PATCH] 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)