Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

G-API OV backend requires cv::MediaFrame #24938

Conversation

DariaMityagina
Copy link
Contributor

@DariaMityagina DariaMityagina commented Jan 29, 2024

Pull Request Readiness Checklist

Background_subtraction demo G-API issue. Update:

Porting to API20 resulted in an error (both for CPU and NPU):

[ERROR] OpenCV(4.9.0-dev) /home/runner/work/open_model_zoo/open_model_zoo/cache/opencv/modules/gapi/src/backends/ov/govbackend.cpp:813: error: (-215: assertion not done ) cv::util::holds_alternative<cv::GMatDesc>(input_meta) in function 'cfgPreProcessing'

Adding cv::MediaFrame support to govbackend resulted in the following (tested with CPU):
image

TODO

  • As part of the review process this comment was addressed which make it impossible to run the demo. I will bring those changes back in a separate PR [support PartialShape]

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 another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • There is a reference to the original bug report and related work
  • 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

@DariaMityagina
Copy link
Contributor Author

Need to investigate this issue in one of the test cases - WIP:
https://github.com/openvinotoolkit/open_model_zoo/actions/runs/7700178002/job/20983409804?pr=3903

[ ERROR ] OpenCV(4.9.0-dev) /home/runner/work/open_model_zoo/open_model_zoo/cache/opencv-with-fix/opencv/modules/core/src/matrix_wrap.cpp:1667: error: (-215:Assertion failed) !fixedSize() in function 'release'

@TolyaTalamanov
Copy link
Contributor

TolyaTalamanov commented Jan 30, 2024

Need to investigate this issue in one of the test cases - WIP: https://github.com/openvinotoolkit/open_model_zoo/actions/runs/7700178002/job/20983409804?pr=3903

[ ERROR ] OpenCV(4.9.0-dev) /home/runner/work/open_model_zoo/open_model_zoo/cache/opencv-with-fix/opencv/modules/core/src/matrix_wrap.cpp:1667: error: (-215:Assertion failed) !fixedSize() in function 'release'

Will be glad to help if it hasn't been resolved yet

@DariaMityagina
Copy link
Contributor Author

DariaMityagina commented Jan 31, 2024

Need to investigate this issue in one of the test cases - WIP: https://github.com/openvinotoolkit/open_model_zoo/actions/runs/7700178002/job/20983409804?pr=3903

[ ERROR ] OpenCV(4.9.0-dev) /home/runner/work/open_model_zoo/open_model_zoo/cache/opencv-with-fix/opencv/modules/core/src/matrix_wrap.cpp:1667: error: (-215:Assertion failed) !fixedSize() in function 'release'

Will be glad to help if it hasn't been resolved yet

Thanks a lot!
I think I found the problem, I'm working on a fix now.

@DariaMityagina DariaMityagina force-pushed the icv/dm/add-media-frame-support-to-govbackend branch 3 times, most recently from 8e87508 to 633ccf9 Compare February 2, 2024 10:24
@DariaMityagina DariaMityagina marked this pull request as ready for review February 2, 2024 10:35
@DariaMityagina
Copy link
Contributor Author

Opened this PR for review.

This issue is going to be addressed in a follow-up ticket - 111291.
Since it only affected instance-segmentation-security-0091 case in OMZ demos check, I only enabled passing instance-segmentation-person-0007 case for now (please see openvinotoolkit/open_model_zoo#3903).

@DariaMityagina
Copy link
Contributor Author

@TolyaTalamanov could you please review this PR?

@DariaMityagina DariaMityagina changed the title G-API OV backend requires cv::MediaFrame G-API OV backend requires cv::MediaFrame - part 1 Feb 2, 2024
@DariaMityagina DariaMityagina force-pushed the icv/dm/add-media-frame-support-to-govbackend branch 3 times, most recently from 34960ec to 58cb0ff Compare February 2, 2024 21:07
// Assert
validate();
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

opencv_test_gapi --gtest_filter=*MediaFrameTestAgeGenderOV* 
CTEST_FULL_OUTPUT
OpenCV version: 4.9.0-dev
OpenCV VCS version: 4.9.0-106-gbde2400e29-dirty
Build type: Release
Compiler: /usr/bin/c++  (ver 9.4.0)
Parallel framework: pthreads (nthreads=6)
CPU features: SSE SSE2 SSE3 *SSE4.1 *SSE4.2 *FP16 *AVX *AVX2 *AVX512-SKX?
Intel(R) IPP version: ippIP AVX2 (l9) 2021.10.0 (-) Sep 18 2023
Intel(R) IPP features code: 0x8000
OpenCL is disabled
TEST: Skip tests with tags: 'mem_6gb', 'verylong'
Note: Google Test filter = *MediaFrameTestAgeGenderOV*
[==========] Running 2 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 2 tests from MediaFrameTestAgeGenderOV
[ RUN      ] MediaFrameTestAgeGenderOV.InferMediaInputBGR
[       OK ] MediaFrameTestAgeGenderOV.InferMediaInputBGR (118 ms)
[ RUN      ] MediaFrameTestAgeGenderOV.InferROIGenericMediaInputBGR
[       OK ] MediaFrameTestAgeGenderOV.InferROIGenericMediaInputBGR (72 ms)
[----------] 2 tests from MediaFrameTestAgeGenderOV (190 ms total)

[----------] Global test environment tear-down
[==========] 2 tests from 1 test case ran. (190 ms total)
[  PASSED  ] 2 tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool! Can we also cover NV12 test cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks! 24e5ceb

@DariaMityagina DariaMityagina changed the title G-API OV backend requires cv::MediaFrame - part 1 G-API OV backend requires cv::MediaFrame Feb 12, 2024
@DariaMityagina
Copy link
Contributor Author

@TolyaTalamanov could you please review this PR?

@asmorkalov asmorkalov added this to the 4.10.0 milestone Feb 12, 2024
@DariaMityagina DariaMityagina force-pushed the icv/dm/add-media-frame-support-to-govbackend branch 2 times, most recently from e58497b to 17cbb67 Compare February 13, 2024 10:17
modules/gapi/src/backends/ov/govbackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ov/govbackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ov/govbackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ov/govbackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ov/govbackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ov/govbackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ov/govbackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ov/govbackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ov/govbackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ov/govbackend.cpp Outdated Show resolved Hide resolved
@DariaMityagina
Copy link
Contributor Author

@asmorkalov hi! Could you please advise how to run all pre-commit checks, not just the default check?

@asmorkalov
Copy link
Contributor

@DariaMityagina Github actions are triggered automatically for OpenCV organization members only. I approved built for you.

@DariaMityagina
Copy link
Contributor Author

@DariaMityagina Github actions are triggered automatically for OpenCV organization members only. I approved built for you.

Great, thank you!

modules/gapi/src/backends/ov/govbackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ov/govbackend.cpp Show resolved Hide resolved
modules/gapi/src/backends/ov/govbackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ov/govbackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ov/govbackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ov/govbackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ov/govbackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ov/govbackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ov/govbackend.cpp Outdated Show resolved Hide resolved
// Assert
validate();
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool! Can we also cover NV12 test cases?

@DariaMityagina
Copy link
Contributor Author

@DariaMityagina @TolyaTalamanov Friendly reminder.

Hello! Will get back to this PR soon. Thanks!

@DariaMityagina DariaMityagina force-pushed the icv/dm/add-media-frame-support-to-govbackend branch from 833bd89 to b1a8203 Compare April 15, 2024 14:53
@DariaMityagina DariaMityagina force-pushed the icv/dm/add-media-frame-support-to-govbackend branch from b1a8203 to 7d58666 Compare April 15, 2024 16:53
@DariaMityagina
Copy link
Contributor Author

@DariaMityagina Github actions are triggered automatically for OpenCV organization members only. I approved built for you.

@asmorkalov hi! Could you please trigger the checks again?

@DariaMityagina
Copy link
Contributor Author

@TolyaTalamanov hi! Could you please take a look at the PR?

@DariaMityagina
Copy link
Contributor Author

@asmorkalov hi! Could you please trigger the checks again? I fixed tests failure.

@DariaMityagina
Copy link
Contributor Author

@asmorkalov hi! Could you please trigger the checks again?

modules/gapi/src/backends/ov/govbackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ov/govbackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ov/govbackend.cpp Show resolved Hide resolved
modules/gapi/src/backends/ov/govbackend.cpp Show resolved Hide resolved
modules/gapi/src/backends/ov/govbackend.cpp Outdated Show resolved Hide resolved
modules/gapi/src/backends/ov/govbackend.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@TolyaTalamanov TolyaTalamanov left a comment

Choose a reason for hiding this comment

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

Great, thank you so much 🔥

LGTM 👍

@DariaMityagina
Copy link
Contributor Author

DariaMityagina commented Apr 23, 2024

@asmorkalov hi! Can this PR be merged? Could you please take a look?

Copy link
Contributor

@dmatveev dmatveev left a comment

Choose a reason for hiding this comment

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

Fantastic work, thanks @DariaMityagina !

@asmorkalov asmorkalov merged commit ebea657 into opencv:4.x Apr 24, 2024
27 of 28 checks passed
@DariaMityagina
Copy link
Contributor Author

@asmorkalov thanks!

asmorkalov added a commit to asmorkalov/opencv that referenced this pull request Apr 24, 2024
asmorkalov added a commit to asmorkalov/opencv that referenced this pull request Apr 25, 2024
asmorkalov added a commit that referenced this pull request Apr 25, 2024
klatism pushed a commit to klatism/opencv that referenced this pull request May 17, 2024
…frame-support-to-govbackend

G-API OV backend requires cv::MediaFrame opencv#24938

### Pull Request Readiness Checklist

**Background_subtraction demo G-API issue. Update:**

Porting to API20 resulted in an error (both for CPU and NPU):
```
[ERROR] OpenCV(4.9.0-dev) /home/runner/work/open_model_zoo/open_model_zoo/cache/opencv/modules/gapi/src/backends/ov/govbackend.cpp:813: error: (-215: assertion not done ) cv::util::holds_alternative<cv::GMatDesc>(input_meta) in function 'cfgPreProcessing'
```

Adding cv::MediaFrame support to govbackend resulted in the following (tested with CPU):
<img width="941" alt="image" src="https://github.com/opencv/opencv/assets/52502732/3a003d61-bda7-4b1e-9117-3410cda1ba32">

### TODO

- [ ] **As part of the review process [this comment](opencv#24938 (comment)) was addressed which make it impossible to run the demo. I will bring those changes back in a separate PR [support `PartialShape`]**

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

- [x] I agree to contribute to the project under Apache 2 License.
- [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
- [x] The PR is proposed to the proper branch
- [x] There is a reference to the original bug report and related work
- [ ] 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
klatism pushed a commit to klatism/opencv that referenced this pull request May 17, 2024
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.

None yet

4 participants