Skip to content

add usageFlags to UMat static factories#20295

Merged
opencv-pushbot merged 1 commit intoopencv:masterfrom
diablodale:umat_factory_usageflags
Jun 23, 2021
Merged

add usageFlags to UMat static factories#20295
opencv-pushbot merged 1 commit intoopencv:masterfrom
diablodale:umat_factory_usageflags

Conversation

@diablodale
Copy link
Copy Markdown
Contributor

updates the UMat static factories to accept a UMatUsageFlags param

  • aligns the static factories (zeros, ones, eye, diag) with the UMat constructors
  • defaults to USAGE_DEFAULT making these changes backwards compatible
  • adds test case
  • only tested on 4.x branch. Do not apply to legacy 3.x because this PR is useless without the 4.x-only UMat usageFlags fixes opencv/opencv#19807 #20027

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

@diablodale
Copy link
Copy Markdown
Contributor Author

And it passed local unit tests...

...
[----------] Global test environment tear-down
[ SKIPSTAT ] 36 tests skipped
[ SKIPSTAT ] TAG='mem_6gb' skip 1 tests
[ SKIPSTAT ] TAG='skip_bigdata' skip 1 tests
[ SKIPSTAT ] TAG='skip_other' skip 34 tests
[==========] 11659 tests from 251 test cases ran. (82892 ms total)
[  PASSED  ] 11659 tests.

@diablodale
Copy link
Copy Markdown
Contributor Author

diablodale commented Jun 22, 2021

The ABI compatibility checker running under linux x64 failed. It is due to the 4 UMat static factory APIs to which I added the usageFlags param. Naturally, the header sets the default so it is API compatible, but at the same time the underlying function signature has widened by one param so it is not ABI compatible. In this scenario, I think this means that recompile and linking to a library would work (as it would have the new header). In constrast, an app that uses (without recompile) a precompiled OpenCV DLL or SO would fail if these 4 APIs were called.

I googled and I did not find an OpenCV doc regarding the topic of ABI compatibility. Is there a doc? If not, would you share OpenCV's approach on this topic? (Might be good to put in Wiki)

Technically, if ABI compatibility is desired while also having these 4 factory updates, I could create sibling APIs. It would double the number of factory APIs. I would simplify when possible, might have the old APIs call the newer wider APIs. Makes the codebase larger and more to maintain -- but that is the cost of maintaining ABI on these...if you want these updates.

- add abi compatible overloads
- add test case
Copy link
Copy Markdown
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Thank you 👍

@opencv-pushbot opencv-pushbot merged commit e9a860d into opencv:master Jun 23, 2021
@alalek alalek mentioned this pull request Oct 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants