Skip to content

UMat constructors and UMat::create() discard UMatUsageFlags; resets to USAGE_DEFAULT #19807

@diablodale

Description

@diablodale

The UMatUsageFlags default param for UMat::create() combined with the use of create()
throughout OpenCV apis results in the usage flag for a UMat given to
these OpenCV apis to be reset to USAGE_DEFAULT. Examples of apis which fail
include the UMat constructors themselves 😬, cv::flip(), cv::cvtColor(), etc.

Naturally, this is undesired. If a usage has been previously specified on
a UMat, its usage flag should be retained. I believe this is both a code and
design flaw. We need to first resolve the design ambiguity/error -- determine
the semantic intention. Then we can explore a fix.

UMat::usageFlags cause substantial change in the execution of OpenCL.
For example, using USAGE_ALLOCATE_HOST_MEMORY causes the OpenCL flag
CL_MEM_ALLOC_HOST_PTR to be used and different memory buffer pools
to be used (ctxImpl.getBufferPoolHostPtr() vs ctxImpl.getBufferPool()).

Some fixes for this issue should be done with careful consideration.
Why? Because fixing the code will cause changes in performance/memory behavior
for many OpenCL apps across a wide set of OpenCV apis. Performance and memory
usage might be better or worse. It might be large or small. It will vary based
on the app's usage of OpenCV.

Setup

  • OpenCV 3.x and 4.x
  • All compilers and platforms

Repro

  1. Write an OpenCV OpenCL app.
  2. Insert code similar to the following
    cv::UMat test1(cv::USAGE_ALLOCATE_HOST_MEMORY);
    test1.create(10,10, CV_32F);
    // after create() inspect in debugger the value of test1.usageFlags
    
    cv::UMat flip_in(10, 10, CV_32F, cv::USAGE_ALLOCATE_HOST_MEMORY);
    cv::UMat flip_out(cv::USAGE_ALLOCATE_HOST_MEMORY);
    cv::flip(flip_in, flip_out, 1);
    // after flip() inspect in debugger the value of flip_out.usageFlags
    
    cv::UMat construct1(64, 64, CV_32F, cv::USAGE_ALLOCATE_DEVICE_MEMORY);
    // after the constructor inspect in debugger the value of construct1.usageFlags
  3. Compile a debug build
  4. Run debugger on your app and inspect the value of .usageFlags after
    each three examples

Result

Value are all cv::USAGE_DEFAULT

Expected

Values are all cv::USAGE_ALLOCATE_HOST_MEMORY

Cause

OpenCV OpenCL apps construct UMat objects often and in many ways. A developer
could use cv::UMat one() or cv::Mat two(); auto three = two.getUMat(); or
complex chained series of api calls and classes which results in a UMat
variable with allocated UMat::UMatData.

During the codepath in these apps, a developer can chose to use the default
cv::USAGE_DEFAULT or one of the three others USAGE_ALLOCATE_HOST_MEMORY,
USAGE_ALLOCATE_DEVICE_MEMORY, USAGE_ALLOCATE_SHARED_MEMORY. When a choice
has been made by the developer, that choice needs to be honored and retained.

The current OpenCV core api for UMat::create() does not honor the developers
choice of usageFlags. Many OpenCV apis use create() without explicitly
passing usageFlags -- causing the usage to always be reset to USAGE_DEFAULT
due to default parameter handling.

😬 Most UMat constructors also fail to honor the usageFlags parameter passed
to them. UMat(rows, cols, type, usageFlags) fails and will always
set usageFlags to USAGE_DEFAULT. The default constructor UMat(usageFlags)
works correctly, but it is quickly reset to USAGE_DEFAULT by most OpenCV apis.

For example, here is one of the UMat constructors from OpenCV 4.5.1. Read
the comments in the code for analysis.

UMat::UMat(int _rows, int _cols, int _type, UMatUsageFlags _usageFlags)
: flags(MAGIC_VAL), dims(0), rows(0), cols(0), allocator(0),
  usageFlags(_usageFlags), u(0), offset(0), size(&rows) // usageFlags is correctly set
{
    create(_rows, _cols, _type); // usageFlags is reset to USAGE_DEFAULT
                                 // because of the bug in create() that always
                                 // resets to USAGE_DEFAULT
}

If we fix the larger problem with create(), then the UMat constructors will
be automatically fixed. If we instead want to only fix the constructors, we can
do this by explicitly passing the usageFlags parameter like the following:

UMat::UMat(int _rows, int _cols, int _type, UMatUsageFlags _usageFlags)
: flags(MAGIC_VAL), dims(0), rows(0), cols(0), allocator(0),
  usageFlags(_usageFlags), u(0), offset(0), size(&rows)
{
    create(_rows, _cols, _type, _usageFlags); // explicitly pass usage param
}

Fixing the UMat constructors is insignificant improvement. 😕 Why?
Because most of the OpenCV apis will reset the usageFlags to USAGE_DEFAULT
because of the UMat::create() bug.

OpenCV APIs often require an output variable to be passed as a output parameter
instead of returning the output as a function result, e.g. cv::flip(),
cv::cvtColor() are like this. Usually, these APIs are written to use
OutputArray for that output parameter. Inside the core api code that
implements flip(), cvtColor(), etc. they all call UMat::create(...)
on that output variable. This results in widespread errant behavior that
resets the UMat usageFlags to USAGE_DEFAULT.

cv::flip(InputArray _src, OutputArray _dst, int flip_mode ) {
    ...
    CV_OCL_RUN( _dst.isUMat(), ocl_flip(_src, _dst, flip_mode))
    ...
    _dst.create( size, type );  // usageFlags is reset to USAGE_DEFAULT
                                // because of the bug in create() that always
                                // resets to USAGE_DEFAULT. However, this
                                // codepath is only when CV_OCL_RUN() fails
                                // It isn't using OpenCL and therefore
                                // the usageFlags for OpenCL memory is not
                                // important.
}

ocl_flip(InputArray _src, OutputArray _dst, int flipCode ) {
    ...
    _dst.create(size, type); // the OpenCL codepath fails. usageFlags is
                             // reset to USAGE_DEFAULT because of the bug in
                             // create()
    ...
}

Because OpenCV apis normally use UMat::create() without the explicit
usageFlags parameter, this exposes this UMat::create() bug throughout
most of the OpenCV apis.

Fixes

OpenCV would requirement multiple fixes. There are three main groups

  1. UMat constructors need to explicitely pass usageFlags to create()
  2. UMat::create(rows, cols, type, usageFlags) apply consistent
    test for same usageFlags. This will break the below partial workaround.
  3. UMat::create() default parameter handling for usageFlags.

Fix group 3 needs discussion. I believe that when all UMat::create() apis
are called without an explicit usageFlags parameter, that the api should
honor and retain the value of usageFlags that is currently within
the UMat. This means that the create() parameter signatures or their
defaults will need to change.

The apis need to know the difference between calling with no
explicit parameter and calling with USAGE_DEFAULT. The current OpenCV code
declares

void create(int rows, int cols, int type, UMatUsageFlags usageFlags = USAGE_DEFAULT);
example result
umat.create(4,4,CV_32F) honors and retains usageFlags already on the Umat
umat.create(4,4,CV_32F, USAGE_DEFAULT) resets usageFlags to USAGE_DEFAULT
umat.create(4,4,CV_32F, USAGE_ALLOCATE_HOST_MEMORY) resets usageFlags to USAGE_DEFAULT

Some possibilities for fix group 3:

  1. Change all the UMat::create(...) apis so that if they receive
    USAGE_DEFAULT, that it never changes usageFlags. This approach
    does not allow reset to USAGE_DEFAULT. This approach has the smallest
    set of code changes.

    UMat umat(4, 4, CV_32F, USAGE_ALLOCATE_HOST_MEMORY);
    
    // correctly retains USAGE_ALLOCATE_HOST_MEMORY
    umat.create(4, 4, CV_32F);
    
    // will not reset to USAGE_DEFAULT; this limitation doesn't concern me
    umat.create(4, 4, CV_32F, USAGE_DEFAULT);
    
    // easy alternative, and very cheap with c++11 move semantics
    umat = UMat(4, 4, CV_32F, USAGE_DEFAULT);
  2. Do not use default parameters. Split each create() into two apis

    void create(int rows, int cols, int type); // honors existing usageFlags
    void create(int rows, int cols, int type, UMatUsageFlags usageFlags); // resets usageFlags
  3. Add new UMatUsageFlags::USAGE_SAME and make it the new default for the
    UMat::create(...) apis. Somewhat ambiguous -- need to clearly
    document the difference between "SAME" and "DEFAULT".

    void create(int rows, int cols, int type, UMatUsageFlags usageFlags = USAGE_SAME);
    
    // honors existing usageFlags; often is USAGE_DEFAULT from the UMat constructor
    // is the most common scenario and works correctly on original repros
    umat.create(4, 4, CV_32F);
    
    // explicitely resets to USAGE_DEFAULT
    umat.create(4, 4, CV_32F, USAGE_DEFAULT);
    
    // explicitely resets to USAGE_ALLOCATE_HOST_MEMORY
    umat.create(4, 4, CV_32F, USAGE_ALLOCATE_HOST_MEMORY); 

Workaround

The UMat must be completely defined and preallocated like the following.
This is error prone and difficult to maintain. And it will no longer be
possible if fix group 2 is implemented.

cv::UMat input(64, 64, CV_32F, cv::USAGE_ALLOCATE_DEVICE_MEMORY);
// load data into input UMat here

cv::UMat output();
output.create(input.size(), input.type(), cv::USAGE_ALLOCATE_DEVICE_MEMORY);
// workaround requires to exactly set and pre-allocate the memory/UMatData
// for all UMats. Naturally, this is messy and not sustainable.

cv::flip(input, output, 1);
// after flip() inspect in debugger the value of output.usageFlags
// it will be the expected USAGE_ALLOCATE_DEVICE_MEMORY

🤹‍♀️ This limited workaround works because of yet another bug. In umatrix.cpp
is the following code

void UMat::create(int _rows, int _cols, int _type, UMatUsageFlags _usageFlags)
{
    _type &= TYPE_MASK;
    if( dims <= 2 && rows == _rows && cols == _cols && type() == _type && u )
        return;
    int sz[] = {_rows, _cols};
    create(2, sz, _type, _usageFlags);
}

The if() test above will check the rows, cols, type, and if u (the
UMatdata) has already been allocated. If all true, then it will immediately
return and not call create() which has the reset usageFlags bug.

Technically, it should check if the usageFlags are the same. It is a valid and
material change if the code requests a different memory usageFlags. It should
be consistent. Consider the scenario if the rows were different. In that
scenario, the code calls create() using the usageFlags parameter. So for
consistency, all code paths should respect _usageFlags.

That additional bug is beneficial to this limited workaround.
I do not recommend this additional bug be kept for the comprehensive
solution. The create() above should check for _usageFlags sameness.

Issue submission checklist

  • I report the issue, it's not a question
  • I checked the problem with documentation, FAQ, open issues,
    forum.opencv.org, Stack Overflow, etc and have not found solution
  • I updated to latest OpenCV version and the issue is still there
  • There is reproducer code and related data files: videos, images, onnx, etc

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions