G-API: onVPL source simple (WIP) by sivanov-work · Pull Request #20469 · opencv/opencv

sivanov-work · 2021-07-28T12:08:32Z

WIP

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

sivanov-work · 2021-07-28T12:13:37Z

modules/gapi/include/opencv2/gapi/streaming/onevpl_builder.hpp

+    void set_arg(const std::string& file_path);
+    void set_arg(const CFGParams& params);
+
+    std::string filePath;


move to Priv

sivanov-work · 2021-07-28T12:14:14Z

modules/gapi/include/opencv2/gapi/streaming/onevpl_builder.hpp

+namespace gapi {
+namespace wip {
+
+using CFGParamName = std::string;


choose new name related to VPL

sivanov-work · 2021-07-28T12:15:46Z

modules/gapi/src/streaming/engine/engine_session.cpp

+EngineSession::~EngineSession()
+{
+    GAPI_LOG_INFO(nullptr, "Close Decode for session: " << session);
+    MFXVideoDECODE_Close(session);


move all decode to DecodeSession

sivanov-work · 2021-07-28T12:16:08Z

modules/gapi/src/streaming/onevpl_builder.cpp

+    std::unique_ptr<VPLSourceImpl> impl(new VPLSourceImpl(filePath, cfg_params));
+    return std::shared_ptr<IStreamSource>(new OneVPLCapture(std::move(impl)));
+#else
+    abort();


pretty exception

sivanov-work · 2021-07-28T12:16:26Z

modules/gapi/src/streaming/onevpl_priv_interface.hpp

+
+    virtual bool pull(cv::gapi::wip::Data& data) = 0;
+    virtual GMetaArg descr_of() const = 0;
+/*


sivanov-work · 2021-07-28T12:19:59Z

modules/gapi/src/streaming/vpl/vpl_legacy_source_engine.cpp

+        [this] (EngineSession& sess) -> ExecutionStatus
+        {
+            LegacyDecodeSession &my_sess = static_cast<LegacyDecodeSession&>(sess);
+            my_sess.last_status = ReadEncodedStream(my_sess.stream, my_sess.source_handle.get());


move to DecodeSession member

sivanov-work · 2021-07-28T12:20:05Z

modules/gapi/src/streaming/vpl/vpl_legacy_source_engine.cpp

+            LegacyDecodeSession &my_sess = static_cast<LegacyDecodeSession&>(sess);
+
+            my_sess.last_status =
+                    MFXVideoDECODE_DecodeFrameAsync(my_sess.session,


move to DecodeSession member

sivanov-work · 2021-07-28T12:22:02Z

modules/gapi/src/streaming/vpl/vpl_legacy_source_engine.hpp

+    void on_frame_ready(LegacyDecodeSession& sess);
+};
+
+class LegacyDecodeSession : public EngineSession {


move into different file

I think it is worth keeping it here.

sivanov-work · 2021-07-28T12:23:09Z

modules/gapi/src/streaming/vpl/vpl_source_impl.cpp

+    }
+
+    mfxBitstream bitstream{};
+    const int BITSTREAM_BUFFER_SIZE = 2000000;


take care about it

sivanov-work · 2021-07-28T12:24:15Z

modules/gapi/src/streaming/vpl/vpl_source_impl.cpp

+GMetaArg VPLSourceImpl::descr_of() const
+{
+    GAPI_Assert(!first_frame.empty());
+    return cv::GMetaArg{cv::descr_of(first_frame)};


to implement

smirnov-alexey · 2021-08-02T13:31:53Z

modules/gapi/CMakeLists.txt

+    ocv_target_compile_definitions(opencv_test_gapi PRIVATE -DHAVE_ONEVPL)
+    ocv_target_link_libraries(opencv_test_gapi PRIVATE VPL::dispatcher)
+  endif()
+  ocv_target_compile_definitions(${the_module} PUBLIC -DHAVE_ONEVPL)


Why is it PUBLIC?

to carry-on dependency to upstream target: example_gapi_*
If you awared that there is very convenient way - please give an example

smirnov-alexey · 2021-08-02T13:32:15Z

modules/gapi/CMakeLists.txt

+  ocv_target_compile_definitions(${the_module} PUBLIC -DHAVE_ONEVPL)
+  ocv_target_link_libraries(${the_module} PUBLIC VPL::dispatcher)
+  if(HAVE_D3D11)
+    message("G-API has DX11")


Please, remove

smirnov-alexey · 2021-08-02T13:35:19Z

modules/gapi/CMakeLists.txt

+  if(HAVE_D3D11)
+    message("G-API has DX11")
+    if(HAVE_OPENCL)
+        ocv_target_include_directories(${the_module} SYSTEM PRIVATE ${OPENCL_INCLUDE_DIRS})


Do you need SYSTEM here?

good question - maybe it's not necessary

smirnov-alexey · 2021-08-02T13:38:48Z

modules/gapi/CMakeLists.txt

 ocv_add_perf_tests()
-ocv_add_samples()
+if(HAVE_ONEVPL)
+    #TODO Why is it? CMake `PUBLIC` dependency must be carried to dependent target of ${the_module}?


I don't think it's a good idea to make it PUBLIC. Actually, I don't see any PUBLIC linkage/definitions in G-API except for standalone mode

if someone uses target_link_libraries(MY_PROJECT gapi) in case of PRIVATE then would get unresolved symbols I believe and additional step find_library(oneVPL) would require. And would required link MY_PROJECT with both gapi and oneVPL. Which looks implicit dependency
In case of PUBLIC only cmake-linkage with gapi is required

smirnov-alexey · 2021-08-02T13:40:29Z

modules/gapi/cmake/detect_onevpl.cmake

@@ -0,0 +1,7 @@
+if(WITH_ONEVPL)
+  find_package(VPL REQUIRED HINTS "${ONEVPLROOT}")
+  message("G-API VPL_FOUND: ${VPL_FOUND}")


Do we need a message here or is it just a debug remnant?

thanks, looks on debug message. i will remove that

smirnov-alexey · 2021-08-02T13:41:38Z

modules/gapi/include/opencv2/gapi/streaming/onevpl_builder.hpp

@@ -0,0 +1,74 @@
+#ifndef OPENCV_GAPI_STREAMING_ONEVPL_PRIV_BUILDER_HPP


Not sure about PRIV

thanks, misprint

smirnov-alexey · 2021-08-02T13:46:20Z

modules/gapi/include/opencv2/gapi/streaming/onevpl_builder.hpp

+private:
+    void set_arg(const std::string& file_path);
+    void set_arg(const CFGParams& params);
+    /*template<typename Param>


Please, remove

… found

sivanov-work · 2021-08-10T15:11:54Z

modules/gapi/cmake/DownloadOneVPL.cmake.in

+  CMAKE_ARGS        -DCMAKE_INSTALL_PREFIX=${ONEVPL_PROJECT_PREFIX}
+  SOURCE_DIR        "${ONEVPL_PROJECT_PREFIX}/src"
+  BINARY_DIR        "${ONEVPL_PROJECT_PREFIX}/build"
+  INSTALL_DIR       ""


use clone & build & install during opencv configure step in case of WITH_ONEVPL is ON and VPL package is not found in predefined places and VPL is not sourced

rgarnov · 2021-08-10T16:02:33Z

modules/gapi/cmake/detect_onevpl.cmake

@@ -0,0 +1,27 @@
+if(WITH_ONEVPL)
+  find_package(VPL QUIET HINTS "${ONEVPLROOT}")


Usually cmake-style locations involve _DIR (like oneVPL_DIR in your case). Is this ONEVPLROOT set if oneVPL installed?

rgarnov · 2021-08-10T16:02:49Z

modules/gapi/CMakeLists.txt

+  endif()
+  ocv_target_compile_definitions(${the_module} PRIVATE -DHAVE_ONEVPL)
+  ocv_target_link_libraries(${the_module} PRIVATE ${VPL_IMPORTED_TARGETS})
+  if(HAVE_D3D11)


Why not if (HAVE_D3D11 AND HAVE_OPENCL)

rgarnov · 2021-08-10T16:05:35Z

modules/gapi/include/opencv2/gapi/streaming/onevpl_cfg_params.hpp

+// of this distribution and at http://opencv.org/license.html.
+//
+// Copyright (C) 2021 Intel Corporation
+


Better to put all onevpl related includes under a separate onevpl folder

rgarnov · 2021-08-10T16:26:24Z

modules/gapi/include/opencv2/gapi/streaming/onevpl_data_provider_interface.hpp

+struct GAPI_EXPORTS IDataProvider {
+    ~IDataProvider() {};
+    virtual size_t provide_data(size_t out_data_bytes_size, void* out_data) = 0;
+    virtual bool empty() const = 0;


Please add a using Ptr = std::shared_ptr<IDataProvider>;

rgarnov · 2021-08-10T16:35:04Z

modules/gapi/include/opencv2/gapi/streaming/onevpl_source.hpp

+GAPI_EXPORTS_W cv::Ptr<IStreamSource> inline make_vpl_src(const std::string& filePath, Args&&... args)
+{
+    return make_src<OneVPLSource>(filePath, std::forward<Args>(args)...);
+}


So, these two functions are needed because make_src cannot choose a correct constructor overload?

I believe it should work

rgarnov · 2021-08-10T16:36:06Z

modules/gapi/src/streaming/engine/base_engine.cpp

+#include "streaming/engine/base_engine.hpp"
+#include "streaming/vpl/vpl_accel_policy.hpp"
+#include "logger.hpp"
+


rgarnov · 2021-08-10T16:37:24Z

modules/gapi/src/streaming/onevpl_source.cpp

+    m_priv(std::move(impl)) {
+}
+
+OneVPLSource::~OneVPLSource() {


https://stackoverflow.com/questions/6012157/is-stdunique-ptrt-required-to-know-the-full-definition-of-t

I'll try to apply syntax later

#*.cpp OneVPLSource::~OneVPLSource() = default

But not sure about different compilators capability

rgarnov · 2021-08-10T16:39:27Z

modules/gapi/src/streaming/vpl/vpl_source_impl.cpp

+
+    // Retrieve the frame information from input stream
+    mfxVideoParam mfxDecParams {};
+    mfxDecParams.mfx.CodecId = decoder.Data.U32;


Please add a comment on that

dmatveev · 2021-08-10T18:17:01Z

modules/gapi/CMakeLists.txt

+        src/streaming/engine/base_engine.cpp
+        src/streaming/engine/engine_session.cpp


Is it related to VPL or it is something general?

If it is something general, then it is worth compile it always (regardless of the VPL).
If it is related to VPL, then please move it to the vpl folder.

dmatveev · 2021-08-10T18:18:12Z

modules/gapi/cmake/DownloadOneVPL.cmake.in

+  SOURCE_DIR        "${ONEVPL_PROJECT_PREFIX}/src"
+  BINARY_DIR        "${ONEVPL_PROJECT_PREFIX}/build"
+  INSTALL_DIR       ""
+  )


Probably it is worth moving this file to the OpenCV's global cmake/ level, where the similar scripts are located.
Probably it is not.

dmatveev · 2021-08-10T18:19:34Z

modules/gapi/include/opencv2/gapi/streaming/onevpl_data_provider_interface.hpp

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


Please move it to streaming/onevpl

dmatveev · 2021-08-10T18:20:45Z

modules/gapi/include/opencv2/gapi/streaming/onevpl_source.hpp

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


not _CAP_

dmatveev · 2021-08-10T18:21:58Z

modules/gapi/include/opencv2/gapi/streaming/onevpl_source.hpp

+GAPI_EXPORTS_W cv::Ptr<IStreamSource> inline make_vpl_src(const std::string& filePath, Args&&... args)
+{
+    return make_src<OneVPLSource>(filePath, std::forward<Args>(args)...);
+}


I believe it should work

dmatveev · 2021-08-10T19:15:43Z

modules/gapi/src/streaming/vpl/vpl_utils.cpp

+    out << "mfxImplDescription.Version: " << static_cast<int>(idesc.Version.Major)
+        << "." << static_cast<int>(idesc.Version.Minor) << std::endl;
+    out << "mfxImplDescription.Impl: " << mfx_impl_to_cstr(idesc.Impl) << std::endl;
+    out << "(*)mfxImplDescription.AccelerationMode: " << mfx_accel_mode_to_cstr(idesc.AccelerationMode) << std::endl;
+    out << "mfxImplDescription.ApiVersion: " << idesc.ApiVersion.Major << "." << idesc.ApiVersion.Minor << std::endl;
+    out << "(*)mfxImplDescription.ApiVersion.Version: " << idesc.ApiVersion.Version << std::endl;
+    out << "mfxImplDescription.ImplName: " << idesc.ImplName << std::endl;
+    out << "mfxImplDescription.License: " << idesc.License << std::endl;
+    out << "mfxImplDescription.Keywords: " << idesc.Keywords << std::endl;
+    out << "mfxImplDescription.VendorID: " << idesc.VendorID << std::endl;
+    out << "mfxImplDescription.VendorImplID: " << idesc.VendorImplID << std::endl;


...and this

dmatveev · 2021-08-10T19:15:52Z

modules/gapi/src/streaming/vpl/vpl_utils.cpp

+    const mfxAccelerationModeDescription &accel = idesc.AccelerationModeDescription;
+    out << "mfxImplDescription.mfxAccelerationMode.Version: " << static_cast<int>(accel.Version.Major)
+        << "." << static_cast<int>(accel.Version.Minor) << std::endl;
+    for (int mode = 0; mode < accel.NumAccelerationModes; mode++) {
+        out << "mfxImplDescription.mfxAccelerationMode.Mode: " << mfx_accel_mode_to_cstr(accel.Mode[mode]) << std::endl;
+    }
+
+    const mfxDeviceDescription &dev = idesc.Dev;
+    out << "mfxImplDescription.mfxDeviceDescription.Version: " << static_cast<int>(dev.Version.Major)
+        << "." << static_cast<int>(dev.Version.Minor) << std::endl;
+    out << "mfxImplDescription.mfxDeviceDescription.DeviceID: " << dev.DeviceID << std::endl;
+    for (int subdevice = 0; subdevice < dev.NumSubDevices; subdevice++) {
+        out << "mfxImplDescription.mfxDeviceDescription.subdevices.Index: " <<     dev.SubDevices[subdevice].Index << std::endl;
+        out << "mfxImplDescription.mfxDeviceDescription.subdevices.SubDeviceID: " <<  dev.SubDevices[subdevice].SubDeviceID << std::endl;
+    }


dmatveev · 2021-08-10T19:15:58Z

modules/gapi/src/streaming/vpl/vpl_utils.cpp

+        auto cid = dec.Codecs[codec].CodecID;
+        out << "(*)mfxImplDescription.mfxDecoderDescription.decoder.CodecID: " << cid;//(cid & 0xff) << "." << (cid >> 8 & 0xff) << "." << (cid >> 16 & 0xff) << "." << (cid >> 24 & 0xff)  << std::endl;
+        out << "mfxImplDescription.mfxDecoderDescription.decoder.MaxcodecLevel: " << dec.Codecs[codec].MaxcodecLevel << std::endl;
+        for (int profile = 0; profile < dec.Codecs[codec].NumProfiles; profile++) {
+            out << "mfxImplDescription.mfxDecoderDescription.decoder.Profiles: "
+                << mfx_codec_type_to_cstr(dec.Codecs[codec].CodecID,
+                                          dec.Codecs[codec].Profiles[profile].Profile) << std::endl;
+            for (int memtype = 0; memtype < dec.Codecs[codec].Profiles[profile].NumMemTypes; memtype++) {
+                out << "mfxImplDescription.mfxDecoderDescription.decoder.Profiles.MemDesc.MemHandleType: "
+                    << mfx_resource_type_to_cstr(dec.Codecs[codec].Profiles[profile].MemDesc[memtype].MemHandleType) << std::endl;
+                out << "mfxImplDescription.mfxDecoderDescription.decoder.Profiles.MemDesc.Width.Min: " 
+                    << dec.Codecs[codec].Profiles[profile].MemDesc[memtype].Width.Min << std::endl;
+                out << "mfxImplDescription.mfxDecoderDescription.decoder.Profiles.MemDesc.Width.Max: "
+                    << dec.Codecs[codec].Profiles[profile].MemDesc[memtype].Width.Max << std::endl;
+                out << "mfxImplDescription.mfxDecoderDescription.decoder.Profiles.MemDesc.Width.Step: "
+                    << dec.Codecs[codec].Profiles[profile].MemDesc[memtype].Width.Step << std::endl;
+                out << "mfxImplDescription.mfxDecoderDescription.decoder.Profiles.MemDesc.Height.Min: "
+                    << dec.Codecs[codec].Profiles[profile].MemDesc[memtype].Height.Min << std::endl;
+                out << "mfxImplDescription.mfxDecoderDescription.decoder.Profiles.MemDesc.Height.Max: "
+                    << dec.Codecs[codec].Profiles[profile].MemDesc[memtype].Height.Max << std::endl;
+                out << "mfxImplDescription.mfxDecoderDescription.decoder.Profiles.MemDesc.Height.Step: "
+                    << dec.Codecs[codec].Profiles[profile].MemDesc[memtype].Height.Step << std::endl;
+            }
+        }


dmatveev · 2021-08-10T19:16:31Z

modules/gapi/src/streaming/vpl/vpl_utils.cpp

+        case MFX_ERR_NONE:
+            return "MFX_ERR_NONE";
+        case MFX_ERR_UNKNOWN:
+            return "MFX_ERR_UNKNOWN";
+        case MFX_ERR_NULL_PTR:
+            return "MFX_ERR_NULL_PTR";
+        case MFX_ERR_UNSUPPORTED:
+            return "MFX_ERR_UNSUPPORTED";
+        case MFX_ERR_MEMORY_ALLOC:
+            return "MFX_ERR_MEMORY_ALLOC";
+        case MFX_ERR_NOT_ENOUGH_BUFFER:
+            return "MFX_ERR_NOT_ENOUGH_BUFFER";
+        case MFX_ERR_INVALID_HANDLE:
+            return "MFX_ERR_INVALID_HANDLE";
+        case MFX_ERR_LOCK_MEMORY:
+            return "MFX_ERR_LOCK_MEMORY";
+        case MFX_ERR_NOT_INITIALIZED:
+            return "MFX_ERR_NOT_INITIALIZED";
+        case MFX_ERR_NOT_FOUND:
+            return "MFX_ERR_NOT_FOUND";
+        case MFX_ERR_MORE_DATA:
+            return "MFX_ERR_MORE_DATA";
+        case MFX_ERR_MORE_SURFACE:
+            return "MFX_ERR_MORE_SURFACE";
+        case MFX_ERR_ABORTED:
+            return "MFX_ERR_ABORTED";
+        case MFX_ERR_DEVICE_LOST:
+            return "MFX_ERR_DEVICE_LOST";
+        case MFX_ERR_INCOMPATIBLE_VIDEO_PARAM:
+            return "MFX_ERR_INCOMPATIBLE_VIDEO_PARAM";
+        case MFX_ERR_INVALID_VIDEO_PARAM:
+            return "MFX_ERR_INVALID_VIDEO_PARAM";
+        case MFX_ERR_UNDEFINED_BEHAVIOR:
+            return "MFX_ERR_UNDEFINED_BEHAVIOR";
+        case MFX_ERR_DEVICE_FAILED:
+            return "MFX_ERR_DEVICE_FAILED";
+        case MFX_ERR_MORE_BITSTREAM:
+            return "MFX_ERR_MORE_BITSTREAM";
+        case MFX_ERR_GPU_HANG:
+            return "MFX_ERR_GPU_HANG";
+        case MFX_ERR_REALLOC_SURFACE:
+            return "MFX_ERR_REALLOC_SURFACE";
+        case MFX_ERR_RESOURCE_MAPPED:
+            return "MFX_ERR_RESOURCE_MAPPED";
+        case MFX_ERR_NOT_IMPLEMENTED:
+            return "MFX_ERR_NOT_IMPLEMENTED";
+        case MFX_WRN_DEVICE_BUSY:
+            return "MFX_WRN_DEVICE_BUSY";
+        case MFX_WRN_VIDEO_PARAM_CHANGED:
+            return "MFX_WRN_VIDEO_PARAM_CHANGED";


and this. BTW, does VPL provides its own means to convert error codes to error messages?

dmatveev · 2021-08-10T19:16:57Z

modules/gapi/src/streaming/vpl/vpl_utils.hpp

+namespace cv {
+namespace gapi {
+namespace wip {


no need to have this namespace here as it is purely internal stuff.

sivanov-work · 2021-08-11T14:28:23Z

modules/gapi/src/streaming/vpl/surface/surface.hpp

+    // it had better to implement RAII
+    size_t obtain_lock();
+    size_t release_lock();


yeah, actually it is MediaFrameAdapter (in ctor: obtain; in dtor: delete)

sivanov-work · 2021-08-11T14:31:14Z

modules/gapi/src/streaming/vpl/surface/surface.hpp

+    // it had better to implement RAII
+    size_t obtain_lock();
+    size_t release_lock();


due to introducing MediaFrameGPUAdapter - it is possible to add Locker here for both adapters.

but effort is increased a little more

sivanov-work · 2021-08-11T14:32:37Z

modules/gapi/src/streaming/vpl/vpl_cpu_accel.cpp

+                std::stringstream ss;
+                ss << "cannot get free surface from pool, size: " << surfaces.size();
+                const std::string& str = ss.str();
+                GAPI_LOG_WARNING(nullptr, str);


sadly no. WARN is top level severity here.
It is possible to introduce ERROR in the next PR and reassess it

sivanov-work · 2021-08-11T14:40:52Z

modules/gapi/samples/onevpl_source.cpp

+    std::string name = line.substr(0, name_endline_pos);
+    std::string value = line.substr(name_endline_pos + 1);
+
+    return cv::gapi::wip::oneVPL_cfg_param::create(name, value);


yes

the most frequently used params can be exposed as standalone classes due to UX:

CodecType : public oneVPL_cfg_param CodecType (enum Type) { ::create("MfxImplemendation.Decoder.CodecId", Type) } }

sivanov-work · 2021-08-11T14:48:11Z

modules/gapi/src/streaming/engine/base_engine.cpp

+namespace gapi {
+namespace wip {
+
+VPLProcessingEngine::VPLProcessingEngine(std::unique_ptr<VPLAccelerationPolicy>&& accel) :


It's a like as "state machine" which implement execution a pipeline, which can represent a decoding(enconding, preporcessing) process
Contains a base logic for executing "stages" and operates with session creation & termination

Specific implementation like VPLLegacyDecodeEngine just feels the pipeline by specific "stages" (lambda or member function) and responsible for actual processing.

VPLLegacyDecodeEngine in comparison with VPLDecodeEngine implement a compilcated pipeline using onVPL the oldest API (CPU KabyLake). But VPLDecodeEngine will provide a lightweight pipeline on new API (soon maybe). Options (legacy or not) applied dynamically based on current VPL API implementation version

Maybe some part of preprocessing can be implemented in another VPLDecodeAndPreprocessEngine in depends on source creation arguments

sivanov-work · 2021-08-11T14:51:38Z

modules/gapi/src/streaming/engine/engine_session.cpp

+namespace wip {
+
+
+EngineSession::EngineSession(mfxSession sess, mfxBitstream&& str) :


it is high level abstraction over MFX session and some other objects related to MFX session.
For example: one MFX session -> one MFX decoder -> one MFX sync -> one file(?) and so on.

Maybe class name is not good enough

sivanov-work · 2021-08-11T14:54:02Z

modules/gapi/src/streaming/engine/engine_session.cpp

+EngineSession::~EngineSession()
+{
+    GAPI_LOG_INFO(nullptr, "Close session: " << session);
+    MFXClose(session);


std:unique_ptr<mfxSession > means mfxSession *
but actually we have mfxSession without pointer. Extra dereferences may confused. I thought about that but it would become more compilated

From this point EngineSession is RAII over mfx session

sivanov-work · 2021-08-11T14:55:17Z

modules/gapi/src/streaming/onevpl_file_data_provider.cpp

+FileDataProvider::FileDataProvider(const std::string& file_path) : 
+    source_handle(fopen(file_path.c_str(), "rb"), &fclose) {
+    if (!source_handle) {
+        throw std::runtime_error("FileDataProvider: cannot open source file: " + file_path);


must throw DataProviderInterfaceErrorCode exception

sivanov-work · 2021-08-11T14:57:46Z

modules/gapi/src/streaming/onevpl_source.cpp

+    m_priv(std::move(impl)) {
+}
+
+OneVPLSource::~OneVPLSource() {


https://stackoverflow.com/questions/6012157/is-stdunique-ptrt-required-to-know-the-full-definition-of-t

I'll try to apply syntax later

#*.cpp OneVPLSource::~OneVPLSource() = default

But not sure about different compilators capability

sivanov-work · 2021-08-11T14:59:20Z

modules/gapi/src/streaming/vpl/vpl_legacy_source_engine.hpp

+    void on_frame_ready(LegacyDecodeSession& sess);
+};
+
+class LegacyDecodeSession : public EngineSession {


sivanov-work · 2021-11-02T13:27:19Z

closed because was merged into independent distinct PRs

sivanov-work added 12 commits July 19, 2021 14:58

Initial commit

230191f

Add oneVPL source initializing

d183851

Clear G-API CMakeLists.txt

55fedf2

Change VPL builder & add DX11

b4cafc9

Implement decode pipeline

92e2ada

Add stub decode

e727c4d

Implement decode using legacy VPL API

67306db

Fix crash on EOF

e151f3d

Implement MediaFrame Adapter

e491e67

Improve logging in base engine

0e60bfa

Improve logging & troubleshooting

06e95d1

Move surface management into vpl_accel, Fix LogLevel

19fc9e1

sivanov-work added the category: g-api / gapi label Jul 28, 2021

sivanov-work requested review from AsyaPronina, TolyaTalamanov, dmatveev, rgarnov and smirnov-alexey July 28, 2021 12:09

sivanov-work marked this pull request as draft July 28, 2021 12:09

sivanov-work commented Jul 28, 2021

View reviewed changes

sivanov-work added 3 commits July 28, 2021 18:30

Add oneVPL source sample (WIP)

26cd4cc

Sample with oneVPL (WIP)

14aa0f5

Add surface reference counting

57e940c

smirnov-alexey reviewed Aug 2, 2021

View reviewed changes

sivanov-work added 11 commits August 6, 2021 10:14

Fix release compilation

ba1f5cb

Remove out oneVPLBuilder

851638c

Rename OneVPLCap ot OneVPLSource

4a90cc1

Introduce IDataProvider interface for VPL source

0950499

Apply IDataProvider in UnitTest

2ee3a3b

Add frame meta generation

eec6b85

Add Copyright

1953880

Fix comment: align header guard macro name with filenames

12294e4

Fix comments: alignmend, move inline utils to cpp

cca8d9b

Add configurable ApiVersion source param

51baf57

Add oneVPL as external cmake project: clone & build if package is not…

b9d0312

… found

sivanov-work marked this pull request as ready for review August 10, 2021 14:52

sivanov-work commented Aug 10, 2021

View reviewed changes

rgarnov requested changes Aug 10, 2021

View reviewed changes

dmatveev reviewed Aug 10, 2021

View reviewed changes

sivanov-work commented Aug 11, 2021

View reviewed changes

sivanov-work mentioned this pull request Aug 24, 2021

G-API: oneVPL (simplification) added surface & frame adapter #20601

Merged

6 tasks

This was referenced Sep 21, 2021

G-API: oneVPL (simplification) added CPU, DX11(fallback CPU) accels & surface pool #20727

Merged

G-API: oneVPL (simplification) Add simple decode pipeline #20739

Merged

sivanov-work mentioned this pull request Oct 19, 2021

G-API: oneVPL - Performance: Add async decode pipeline & add cached pool #20901

Merged

6 tasks

sivanov-work closed this Nov 2, 2021

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

		@@ -0,0 +1,74 @@
		#ifndef OPENCV_GAPI_STREAMING_ONEVPL_PRIV_BUILDER_HPP

		@@ -0,0 +1,27 @@
		if(WITH_ONEVPL)
		find_package(VPL QUIET HINTS "${ONEVPLROOT}")

		src/streaming/engine/base_engine.cpp
		src/streaming/engine/engine_session.cpp

		namespace wip {


		EngineSession::EngineSession(mfxSession sess, mfxBitstream&& str) :

Uh oh!

Conversation

sivanov-work commented Jul 28, 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!

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!

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!

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

sivanov-work commented Jul 28, 2021 •

edited

Loading