Merge pull request #4233 from alalek:fix_4006

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[],

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);
     }
 

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

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;