G-API: VPL Source turn on Linux CPU version by SergeyIvanov87 · Pull Request #21883 · opencv/opencv

SergeyIvanov87 · 2022-04-18T08:38:24Z

Fix compilation VPL Streaming Source for Linux
Add stub for VAAPI acceleration which uses accel for decode but produces MediaFrame in CPU memory
Fix UT of existing functionalities

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

Build Configuration

Xforce_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

buildworker:Custom=linux-1
build_gapi_standalone:Custom=ade-0.1.1f
build_image:Custom=onevpl-2021.6.0
test_opencl:Custom=OFF

Xbuild_image:Custom=ubuntu-openvino-2021.3.0:20.04
Xbuild_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

build_image:Custom Win=gapi-onevpl-2021.6.0
buildworker:Custom Win=windows-3
build_contrib:Custom Win=OFF

dmatveev

I see only two new files were added, assuming extending tests for new backend I'd expect 2-3 more files modified, why it is 35 files long then? :)

dmatveev · 2022-04-18T09:23:47Z

modules/gapi/CMakeLists.txt

    src/backends/fluid/gfluidimgproc_func.dispatch.cpp
    src/backends/fluid/gfluidcore.cpp
-	  src/backends/fluid/gfluidcore_func.dispatch.cpp
+      src/backends/fluid/gfluidcore_func.dispatch.cpp


dmatveev · 2022-04-18T09:24:58Z

modules/gapi/CMakeLists.txt

+  if(UNIX)
+    find_package(PkgConfig)
+    if(PkgConfig_FOUND)
+      pkg_check_modules(PKG_LIBVA libva>=1.2 libva-drm>=1.2)
+      if(PKG_LIBVA_FOUND)
+        set(CMAKE_THREAD_PREFER_PTHREAD TRUE)
+        set(THREADS_PREFER_PTHREAD_FLAG TRUE)
+        find_package(Threads REQUIRED)
+      else()
+        message(FATAL_ERROR "libva not found: building HAVE_GAPI_ONEVPL without libVA support is impossible on UNIX systems")
+      endif()
+    else()
+      message(FATAL_ERROR "PkgConfig not found: building HAVE_GAPI_ONEVPL without libVA support is impossible on UNIX systems")
+    endif()
+    ocv_target_link_libraries(${the_module} PRIVATE ${PKG_LIBVA_LIBRARIES} ${PKG_THREAD_LIBRARIES})
+  endif()
 endif()


Probably this whole thing should be done one-two levels above. Maybe make VA lookup OpenCV-global? cc: @alalek

it activates in composition with -DWITH_GAPI_ONEVPL and should be removed (or changed) when plugin-based atrchitecture applied. That's because I did not spend much time for working this out. But any other suggestions are appreciated

dmatveev · 2022-04-18T09:25:31Z

modules/gapi/include/opencv2/gapi/gstreaming.hpp

 #include <opencv2/gapi/util/optional.hpp>
 #include <opencv2/gapi/garg.hpp>
 #include <opencv2/gapi/streaming/source.hpp>
+#include <opencv2/gapi/gcommon.hpp>


Is this really required by this particular header?

i'll try to check it

dmatveev · 2022-04-18T09:26:18Z

modules/gapi/include/opencv2/gapi/streaming/onevpl/accel_types.hpp

 enum class AccelType: uint8_t {
    HOST,
    DX11,
+    VA_API,


or VAAPI? That's how it's called at 01.org - https://01.org/linuxmedia/vaapi

you right, i'll rename it

dmatveev · 2022-04-18T09:27:32Z

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

@@ -155,8 +156,12 @@ VPLCPUAccelerationPolicy::create_surface_pool(size_t pool_size, size_t surface_s
    GAPI_LOG_DEBUG(nullptr, "page size: " << page_size_bytes << ", preallocated_raw_bytes: " << preallocated_raw_bytes);
    preallocated_pool_memory_ptr = _aligned_malloc(preallocated_raw_bytes, page_size_bytes);


Just wondering why it wasn't RAII-d

it "semi" RAII-d because it is being wrapped into std::shared_ptr with custom deleter (free/aligned_free) in 174 line below

dmatveev · 2022-04-18T09:27:55Z

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

 #else
-        GAPI_Assert(false && "Not implemented for systems differ than \"_WIN32\". "
-                             "Please feel free to set it up under OPENCV contribution policy");
+        free(ptr);


..at least this could have been automated then

it's deleter definition for RAII-d shared_ptr, which constructed from raw memory pointer
So, RAII was applied in result :)

dmatveev · 2022-04-18T09:29:35Z

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

+    device_fd(-1) {
+#if defined(HAVE_VA) || defined(HAVE_VA_INTEL)
+    // TODO Move it out in device selector
+    device_fd = open("/dev/dri/renderD128", O_RDWR);


it will be moved into device selector in next PRs
at current step RAII for device_fd is VPLVAAPIAcceleration policy by itself

SergeyIvanov87 · 2022-04-18T10:30:07Z

@dmatveev

I see only two new files were added, assuming extending tests for new backend I'd expect 2-3 more files modified, why it is 35 files long then? :)

It seems gcc is more strict compiler than MSVC version. Currently, i used GCC 11.2 which complaints about much stuff, for example as different function declararion & definitions (for non-static) and integers implicit conversions and some others

ghost · 2022-04-19T11:48:53Z

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

+
+cv::MediaFrame::AdapterPtr VPLVAAPIAccelerationPolicy::create_frame_adapter(pool_key_t,
+                                                                          const FrameConstructorArgs &) {
+    GAPI_Assert(false && "VPLVAAPIAccelerationPolicy unavailable in current configuration");


Not sure if it's a good idea, but given the amount of code here, maybe it's worth to put this unsupported asserts in the default implementation of these methods in the base class?

Maybe...
But current version gives additional hint about descendant name whithout additional method name() exposing.
Also i do not want to pollute pure interface

ghost · 2022-04-19T11:49:28Z

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

+// 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) 2021 Intel Corporation


I'm not sure about whole comments because it implemented as free-initiative

SergeyIvanov87 · 2022-04-22T07:45:46Z

@alalek Could you merge it please? It seems only minor commets are elapsed

SergeyIvanov87 · 2022-05-04T19:06:57Z

@alalek @asmorkalov should we merge it?

SergeyIvanov87 · 2022-05-12T06:52:46Z

@alalek License headers inserted in newly added files, please take a look

G-API: VPL Source turn on Linux CPU version * Turn on linux compilation * Apply comments * Change new files headline * Add license header

SergeyIvanov87 force-pushed the gapi_vpl_linux branch from 92d56f3 to 80184b2 Compare April 18, 2022 08:43

asmorkalov added the category: g-api / gapi label Apr 18, 2022

SergeyIvanov87 mentioned this pull request Apr 18, 2022

G-API: VPL Source turn on Linux CPU version #21790

Closed

6 tasks

dmatveev self-assigned this Apr 18, 2022

dmatveev added this to the 4.6.0 milestone Apr 18, 2022

dmatveev approved these changes Apr 18, 2022

View reviewed changes

ghost reviewed Apr 19, 2022

View reviewed changes

ghost approved these changes Apr 22, 2022

View reviewed changes

SergeyIvanov87 added 4 commits May 10, 2022 16:16

Turn on linux compilation

17e46c8

Apply comments

cdf0157

Change new files headline

23c63db

Add license header

ffe3297

SergeyIvanov87 force-pushed the gapi_vpl_linux branch from 63b7f49 to ffe3297 Compare May 10, 2022 13:17

opencv-pushbot merged commit eff5605 into opencv:4.x May 12, 2022

alalek mentioned this pull request Aug 21, 2022

(5.x) Merge 4.x #22408

Merged

		@@ -155,8 +156,12 @@ VPLCPUAccelerationPolicy::create_surface_pool(size_t pool_size, size_t surface_s
		GAPI_LOG_DEBUG(nullptr, "page size: " << page_size_bytes << ", preallocated_raw_bytes: " << preallocated_raw_bytes);
		preallocated_pool_memory_ptr = _aligned_malloc(preallocated_raw_bytes, page_size_bytes);

Uh oh!

Conversation

SergeyIvanov87 commented Apr 18, 2022 • edited by opencv-pushbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Readiness Checklist

Build Configuration

Uh oh!

dmatveev left a comment

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!

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!

SergeyIvanov87 commented Apr 18, 2022

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!

SergeyIvanov87 commented Apr 22, 2022

Uh oh!

SergeyIvanov87 commented May 4, 2022

Uh oh!

SergeyIvanov87 commented May 12, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

SergeyIvanov87 commented Apr 18, 2022 •

edited by opencv-pushbot

Loading