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: onVPL source simple (WIP) #20469
Conversation
void set_arg(const std::string& file_path); | ||
void set_arg(const CFGParams& params); | ||
|
||
std::string filePath; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move to Priv
namespace gapi { | ||
namespace wip { | ||
|
||
using CFGParamName = std::string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
choose new name related to VPL
EngineSession::~EngineSession() | ||
{ | ||
GAPI_LOG_INFO(nullptr, "Close Decode for session: " << session); | ||
MFXVideoDECODE_Close(session); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move all decode to DecodeSession
std::unique_ptr<VPLSourceImpl> impl(new VPLSourceImpl(filePath, cfg_params)); | ||
return std::shared_ptr<IStreamSource>(new OneVPLCapture(std::move(impl))); | ||
#else | ||
abort(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pretty exception
|
||
virtual bool pull(cv::gapi::wip::Data& data) = 0; | ||
virtual GMetaArg descr_of() const = 0; | ||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove
[this] (EngineSession& sess) -> ExecutionStatus | ||
{ | ||
LegacyDecodeSession &my_sess = static_cast<LegacyDecodeSession&>(sess); | ||
my_sess.last_status = ReadEncodedStream(my_sess.stream, my_sess.source_handle.get()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move to DecodeSession member
LegacyDecodeSession &my_sess = static_cast<LegacyDecodeSession&>(sess); | ||
|
||
my_sess.last_status = | ||
MFXVideoDECODE_DecodeFrameAsync(my_sess.session, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move to DecodeSession member
void on_frame_ready(LegacyDecodeSession& sess); | ||
}; | ||
|
||
class LegacyDecodeSession : public EngineSession { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move into different file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is worth keeping it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
} | ||
|
||
mfxBitstream bitstream{}; | ||
const int BITSTREAM_BUFFER_SIZE = 2000000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
take care about it
GMetaArg VPLSourceImpl::descr_of() const | ||
{ | ||
GAPI_Assert(!first_frame.empty()); | ||
return cv::GMetaArg{cv::descr_of(first_frame)}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to implement
modules/gapi/CMakeLists.txt
Outdated
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) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is it PUBLIC
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to carry-on dependency to upstream target: example_gapi_*
If you awared that there is very convenient way - please give an example
modules/gapi/CMakeLists.txt
Outdated
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") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
if(HAVE_D3D11) | ||
message("G-API has DX11") | ||
if(HAVE_OPENCL) | ||
ocv_target_include_directories(${the_module} SYSTEM PRIVATE ${OPENCL_INCLUDE_DIRS}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need SYSTEM
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good question - maybe it's not necessary
modules/gapi/CMakeLists.txt
Outdated
@@ -250,4 +277,10 @@ if(HAVE_ONNX) | |||
endif() | |||
|
|||
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}? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
@@ -0,0 +1,7 @@ | |||
if(WITH_ONEVPL) | |||
find_package(VPL REQUIRED HINTS "${ONEVPLROOT}") | |||
message("G-API VPL_FOUND: ${VPL_FOUND}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a message here or is it just a debug remnant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, looks on debug message. i will remove that
@@ -0,0 +1,74 @@ | |||
#ifndef OPENCV_GAPI_STREAMING_ONEVPL_PRIV_BUILDER_HPP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about PRIV
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, misprint
private: | ||
void set_arg(const std::string& file_path); | ||
void set_arg(const CFGParams& params); | ||
/*template<typename Param> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, remove
CMAKE_ARGS -DCMAKE_INSTALL_PREFIX=${ONEVPL_PROJECT_PREFIX} | ||
SOURCE_DIR "${ONEVPL_PROJECT_PREFIX}/src" | ||
BINARY_DIR "${ONEVPL_PROJECT_PREFIX}/build" | ||
INSTALL_DIR "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
@@ -0,0 +1,27 @@ | |||
if(WITH_ONEVPL) | |||
find_package(VPL QUIET HINTS "${ONEVPLROOT}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually cmake-style locations involve _DIR
(like oneVPL_DIR in your case). Is this ONEVPLROOT set if oneVPL installed?
endif() | ||
ocv_target_compile_definitions(${the_module} PRIVATE -DHAVE_ONEVPL) | ||
ocv_target_link_libraries(${the_module} PRIVATE ${VPL_IMPORTED_TARGETS}) | ||
if(HAVE_D3D11) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not if (HAVE_D3D11 AND HAVE_OPENCL)
// of this distribution and at http://opencv.org/license.html. | ||
// | ||
// Copyright (C) 2021 Intel Corporation | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to put all onevpl related includes under a separate onevpl
folder
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; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a using Ptr = std::shared_ptr<IDataProvider>;
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)...); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, these two functions are needed because make_src
cannot choose a correct constructor overload?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it should work
#include "streaming/engine/base_engine.hpp" | ||
#include "streaming/vpl/vpl_accel_policy.hpp" | ||
#include "logger.hpp" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
m_priv(std::move(impl)) { | ||
} | ||
|
||
OneVPLSource::~OneVPLSource() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
= default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll try to apply syntax later
#*.cpp
OneVPLSource::~OneVPLSource() = default
But not sure about different compilators capability
|
||
// Retrieve the frame information from input stream | ||
mfxVideoParam mfxDecParams {}; | ||
mfxDecParams.mfx.CodecId = decoder.Data.U32; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a comment on that
src/streaming/engine/base_engine.cpp | ||
src/streaming/engine/engine_session.cpp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
SOURCE_DIR "${ONEVPL_PROJECT_PREFIX}/src" | ||
BINARY_DIR "${ONEVPL_PROJECT_PREFIX}/build" | ||
INSTALL_DIR "" | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably it is worth moving this file to the OpenCV's global cmake/
level, where the similar scripts are located.
Probably it is not.
@@ -0,0 +1,38 @@ | |||
// This file is part of OpenCV project. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move it to streaming/onevpl
// Copyright (C) 2021 Intel Corporation | ||
|
||
#ifndef OPENCV_GAPI_STREAMING_ONEVPL_CAP_HPP | ||
#define OPENCV_GAPI_STREAMING_ONEVPL_CAP_HPP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not _CAP_
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)...); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it should work
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; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...and this
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; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and this
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; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and this
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"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and this. BTW, does VPL provides its own means to convert error codes to error messages?
namespace cv { | ||
namespace gapi { | ||
namespace wip { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to have this namespace here as it is purely internal stuff.
// it had better to implement RAII | ||
size_t obtain_lock(); | ||
size_t release_lock(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, actually it is MediaFrameAdapter (in ctor: obtain; in dtor: delete)
// it had better to implement RAII | ||
size_t obtain_lock(); | ||
size_t release_lock(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
due to introducing MediaFrameGPUAdapter - it is possible to add Locker here for both adapters.
but effort is increased a little more
std::stringstream ss; | ||
ss << "cannot get free surface from pool, size: " << surfaces.size(); | ||
const std::string& str = ss.str(); | ||
GAPI_LOG_WARNING(nullptr, str); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sadly no. WARN is top level severity here.
It is possible to introduce ERROR in the next PR and reassess it
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); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
}
}
namespace gapi { | ||
namespace wip { | ||
|
||
VPLProcessingEngine::VPLProcessingEngine(std::unique_ptr<VPLAccelerationPolicy>&& accel) : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
namespace wip { | ||
|
||
|
||
EngineSession::EngineSession(mfxSession sess, mfxBitstream&& str) : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
EngineSession::~EngineSession() | ||
{ | ||
GAPI_LOG_INFO(nullptr, "Close session: " << session); | ||
MFXClose(session); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
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); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
must throw DataProviderInterfaceErrorCode exception
m_priv(std::move(impl)) { | ||
} | ||
|
||
OneVPLSource::~OneVPLSource() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll try to apply syntax later
#*.cpp
OneVPLSource::~OneVPLSource() = default
But not sure about different compilators capability
void on_frame_ready(LegacyDecodeSession& sess); | ||
}; | ||
|
||
class LegacyDecodeSession : public EngineSession { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
closed because was merged into independent distinct PRs |
WIP
Pull Request Readiness Checklist
See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request
Patch to opencv_extra has the same branch name.
Build configuration