[G-API]: Performance tests for BackgroundSubtractor by OrestChura · Pull Request #19542 · opencv/opencv

OrestChura · 2021-02-16T12:24:18Z

Performance tests for BackgroundSubtractor stateful operation added
Accuracy test misprint fixed

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 reference to 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

force_builders=Custom
build_gapi_standalone:Linux x64=ade-0.1.1f
build_gapi_standalone:Win64=ade-0.1.1f
build_gapi_standalone:Mac=ade-0.1.1f
build_gapi_standalone:Linux x64 Debug=ade-0.1.1f

build_image:Custom=centos:7
buildworker:Custom=linux-1
build_gapi_standalone:Custom=ade-0.1.1f

xbuild_image:Custom=ubuntu-openvino-2021.1.0:20.04
xbuild_image:Custom Win=openvino-2021.1.0
xbuild_image:Custom Mac=openvino-2021.1.0

test_modules:Custom=gapi
test_modules:Custom Win=gapi
test_modules:Custom Mac=gapi

# disabled due high memory usage: test_opencl:Custom=ON
test_opencl:Custom=OFF
test_bigdata:Custom=1
test_filter:Custom=*

OrestChura · 2021-02-17T08:17:09Z

@anna-khakimova please review

mpashchenkov

First wave.

modules/gapi/perf/cpu/gapi_video_perf_tests_cpu.cpp

modules/gapi/perf/common/gapi_video_perf_tests_inl.hpp

alalek

Please take a look on existed OpenCV performance tests for this functionality first.

alalek · 2021-02-19T07:48:57Z

modules/gapi/perf/common/gapi_video_perf_tests_inl.hpp

+    // Processing `testNumFrames` amount of frames without measurements:
+    while (frames++ <= testNumFrames && cap.grab())
+    {
+        cap.retrieve(frame);
+        c.apply(frame, gapiForeground);
+        pOCVBackSub->apply(frame, ocvForeground, learningRate);
+        EXPECT_TRUE(cmpF(gapiForeground, ocvForeground));
+    }


Accuracy checks must be removed from the performance tests. Use accuracy tests for that.

"OCV" code must gone from all perf tests.

Yes, a lot of comparison was excess; comparison was removed from the cycle.
However, by design of G-API performance tests, it is necessary to be sure the kernel is working properly while measuring the performance.
It's really not my call to change it in this PR.
I left a single comparison for the validation.

alalek · 2021-02-19T07:53:21Z

modules/gapi/perf/common/gapi_video_perf_tests_inl.hpp

+    // ...and then measuring the G-API subtractor performance on the single frame
+    TEST_CYCLE()
+    {
+        c.apply(frame, gapiForeground);


Is c stateless?

if yes, the just 1 frame is not enough (code measures "initialization" code which doesn't make sense)

if no, then it is just wrong. Performance samples must measure the same code.

You are right, thanks! The measuring was refactored and now a performance of the kernel on a sequence of the same n frames (with resetting the state between iterations) is measured.

anna-khakimova · 2021-02-25T09:00:50Z

Looks good for me.

modules/gapi/perf/common/gapi_video_perf_tests_inl.hpp

mpashchenkov · 2021-02-26T14:06:57Z

modules/gapi/perf/common/gapi_video_perf_tests_inl.hpp

+            frames.push_back(frame);
+        }
+    }
+    testNumFrames = frames.size();


Another change of value, it is curiously. testNumFrames = warm_up_frames + perf_test_work_frames, is it?

This line is to avoid termination in case the video has less frames than it was required. So at this point testNumFrames is the real number of frames read

This variable independent of test parameter. What if read() returns false (some problem) and we receive part of frames? Is it correct behavior for this case?

Yes, exactly, this line handles the case when, for example, a video is too short and there are not enough frames. If so, I decided that the expected behaviour would be to process the obtainable amount of frames.

In the case when everything is all right the variable wouldn't change

I hope "everything isn't right" case doesn't relate to this test and this step is just in case.

What if frames.size() is zero?

Yes, absolutely, this step is just in case.

If size is 0.. it's extremely unlikely, as video should have no frames despite the fact it have been read successfully.
But if it actually happened, nothing would be processed, so output Mats would remain empty. And then validation either would or would not pass. Which is not the expected behaviour

I'll add assert to have speakable error

Assertion check must be here.

We can't have "flexible" scope for performance sampling. Test code must be fixed instead if error occurs.

I see. Added termination in case frames.size() is not testNumFrames after reading instead of re-assignment

modules/gapi/perf/cpu/gapi_video_perf_tests_cpu.cpp

modules/gapi/perf/common/gapi_video_perf_tests_inl.hpp

OrestChura · 2021-03-03T13:29:55Z

@alalek Are the Jenkins builds important to be green for merging?
If yes, could you please retry them? They seem just to not have been able to fetch OpenCV for some reasons last time

alalek · 2021-03-03T13:32:47Z

Jenkins builders are optional for now (there are several infrastructure issues not related to any patch)

OrestChura · 2021-03-04T08:58:52Z

@alalek please check if this is mergeable

[G-API]: Performance tests for BackgroundSubtractor * Perf.Tests for BackgroundSubtractor kernel * Fix CI * Addressing comments * Addressing a comment * Test cycle and validation changes * Addressing comment * Added assert

Perf.Tests for BackgroundSubtractor kernel

f9a47c6

OrestChura added test category: g-api / gapi labels Feb 16, 2021

OrestChura requested a review from mpashchenkov February 16, 2021 12:24

Fix CI

5dbdaad

mpashchenkov reviewed Feb 17, 2021

View reviewed changes

OrestChura added 2 commits February 18, 2021 12:05

Addressing comments

4c972ea

Addressing a comment

861108f

mpashchenkov approved these changes Feb 19, 2021

View reviewed changes

alalek requested changes Feb 19, 2021

View reviewed changes

Test cycle and validation changes

ca31258

OrestChura requested a review from mpashchenkov February 24, 2021 21:05

mpashchenkov reviewed Feb 26, 2021

View reviewed changes

OrestChura added 2 commits March 1, 2021 17:50

Addressing comment

75bf812

Added assert

003425d

OrestChura assigned mpashchenkov Mar 2, 2021

OrestChura requested a review from alalek March 9, 2021 13:51

alalek merged commit c1a57a1 into opencv:master Mar 10, 2021

alalek mentioned this pull request Apr 9, 2021

(5.x) Merge 4.x #19885

Merged

Uh oh!

Conversation

OrestChura commented Feb 16, 2021 • edited by alalek Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Readiness Checklist

Uh oh!

OrestChura commented Feb 17, 2021

Uh oh!

mpashchenkov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alalek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OrestChura Feb 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anna-khakimova commented Feb 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mpashchenkov Feb 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OrestChura Mar 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OrestChura Mar 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

OrestChura commented Mar 3, 2021

Uh oh!

alalek commented Mar 3, 2021

Uh oh!

OrestChura commented Mar 4, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

OrestChura commented Feb 16, 2021 •

edited by alalek

Loading

OrestChura Feb 23, 2021 •

edited

Loading

anna-khakimova commented Feb 25, 2021 •

edited

Loading

mpashchenkov Feb 26, 2021 •

edited

Loading

OrestChura Mar 2, 2021 •

edited

Loading

OrestChura Mar 2, 2021 •

edited

Loading