G-API: oneVPL (simplification) added surface & frame adapter by sivanov-work · Pull Request #20601 · opencv/opencv

sivanov-work · 2021-08-24T15:54:14Z

this PR is the single one in series of #20469

Introduce VPL Surface & CPU MediaFrame Adapter

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

Build configuration

force_builders=Custom,Custom Win,Custom Mac
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

Xbuild_image:Custom=centos:7
Xbuildworker:Custom=linux-1
build_gapi_standalone:Custom=ade-0.1.1f

build_image:Custom=ubuntu-openvino-2021.3.0:20.04
build_image:Custom Win=openvino-2021.2.0
build_image:Custom Mac=openvino-2021.2.0

test_modules:Custom=gapi,python2,python3,java
test_modules:Custom Win=gapi,python2,python3,java
test_modules:Custom Mac=gapi,python2,python3,java

buildworker:Custom=linux-1
test_opencl:Custom=OFF
test_bigdata:Custom=1
test_filter:Custom=*

modules/gapi/CMakeLists.txt

dmatveev · 2021-08-24T16:56:45Z

modules/gapi/src/streaming/onevpl/accelerators/accel_policy_interface.hpp

@@ -0,0 +1,59 @@
+// This file is part of OpenCV project.


accelerators may be a way too long directory name

May I put on it here before till final PRs? there are lot of changes will come

dmatveev · 2021-08-24T16:57:11Z

modules/gapi/src/streaming/onevpl/accelerators/accel_policy_interface.hpp

+// Copyright (C) 2021 Intel Corporation
+
+#ifndef GAPI_STREAMING_ONEVPL_ACCELERATORS_ACCEL_POLICY_INTERFACE_HPP
+#define GAPI_STREAMING_ONEVPL_ACCELERATORS_ACCEL_POLICY_INTERFACE_HPP


ACCELERATORS_ACCEL

I believe it can be simplified to just ACCEL. No need to repeat one

as @smirnov-alexey mentioned before - it should take a form path/file_name if i understood correctly
to do not conflict with potential similar file names on uper-level

Looks like some other headers are simplified to just file_name, so it can be applied here. I think only public header should follow path/file_name guideline. Sorry, if I was mistaken

I faced with the situation sometimes:

sources ---component_1 ------file_name.hpp ---component_2 -------<the same file_name>.hpp

in this case full path is an option, that's because i have been using path/filename solution.
If it is not a big problem, let's put it as is :) ?

modules/gapi/src/streaming/onevpl/accelerators/surface/cpu_frame_adapter.cpp

modules/gapi/src/streaming/onevpl/accelerators/surface/surface.cpp

modules/gapi/src/streaming/onevpl/accelerators/surface/surface.hpp

modules/gapi/test/streaming/gapi_streaming_vpl_core_test.cpp

smirnov-alexey · 2021-09-14T05:23:20Z

modules/gapi/src/streaming/onevpl/accelerators/surface/cpu_frame_adapter.cpp

+    const Surface::data_t& data = parent_surface_ptr->get_data();
+    const Surface::info_t& info = parent_surface_ptr->get_info();
+    using stride_t = typename cv::MediaFrame::View::Strides::value_type;
+    GAPI_Assert(data.Pitch >= 0 && "Pitch is less than 0");


Just to double-check: what is Pitch's type?

good question
it's U16 type. in this case assert is redundant

smirnov-alexey · 2021-09-14T05:24:53Z

modules/gapi/src/streaming/onevpl/accelerators/surface/cpu_frame_adapter.cpp

+
+    stride_t pitch = static_cast<stride_t>(data.Pitch);
+    switch(info.FourCC) {
+        case MFX_FOURCC_I420:


There is an assert in meta() on that format. Does it really work?

access and meta looked as independent methods and could be invoked in different order, so check required both of them.

BUT, because surface is locked in ctor and must be not changed until dtor it is possible to move out this checking from meta and access and put it up into ctor - to keep invariant safe after MediaFrame construction.

i write down such proposal into Jira item to apply it in the last PRs (in series)

smirnov-alexey · 2021-09-14T05:35:40Z

modules/gapi/test/streaming/gapi_streaming_vpl_core_test.cpp

+// It is subject to the license terms in the LICENSE file found in the top-level directory
+// of this distribution and at http://opencv.org/license.html.
+//
+// Copyright (C) 2019-2020 Intel Corporation


smirnov-alexey · 2021-09-14T05:37:10Z

modules/gapi/test/streaming/gapi_streaming_vpl_core_test.cpp

+
+namespace opencv_test
+{
+namespace


Why do you need an additional namespace here?

I adhered existing tests style

dmatveev

LGTM, please go ahead with the merge

G-API: oneVPL (simplification) added surface & frame adapter * added surface & frame adapter * Add FrameAdapter unut tests * Fix compilation after rebase * Fix compilation tests * Apply some review comments * Fix compile warning * Revert back CV_Func usage * Apply some comments

sivanov-work added 4 commits August 24, 2021 18:39

added surface & frame adapter

85be327

Add FrameAdapter unut tests

79e7aa8

Fix compilation after rebase

2bcdaae

Fix compilation tests

267e82b

sivanov-work added category: g-api / gapi feature labels Aug 24, 2021

sivanov-work requested review from TolyaTalamanov, dmatveev, rgarnov and smirnov-alexey August 24, 2021 15:55

dmatveev self-assigned this Aug 24, 2021

dmatveev added this to the 4.5.4 milestone Aug 24, 2021

dmatveev reviewed Aug 24, 2021

View reviewed changes

sivanov-work added 3 commits August 25, 2021 10:17

Apply some review comments

c93ebbf

Fix compile warning

41c3cd2

Revert back CV_Func usage

5688871

smirnov-alexey reviewed Sep 14, 2021

View reviewed changes

Apply some comments

4d3b22e

dmatveev approved these changes Sep 20, 2021

View reviewed changes

alalek merged commit ba8f9d8 into opencv:master Sep 20, 2021

alalek mentioned this pull request Oct 15, 2021

(5.x) Merge 4.x #20886

Merged

sivanov-work deleted the surf_bk_test branch March 5, 2022 05:49

Uh oh!

Conversation

sivanov-work commented Aug 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Readiness Checklist

Build configuration

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dmatveev left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sivanov-work commented Aug 24, 2021 •

edited

Loading