CARVIEW |
Navigation Menu
-
-
Notifications
You must be signed in to change notification settings - Fork 56.2k
G-API: oneVPL DX11 inference #21232
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: oneVPL DX11 inference #21232
Conversation
a20815b
to
10d558a
Compare
one more commit is expected to fix CustomMac CI in a way to fix comparison between InferenceEngines types |
@@ -510,15 +510,51 @@ inline IE::Blob::Ptr extractRemoteBlob(IECallContext& ctx, std::size_t i) { | |||
using ParamType = std::pair<InferenceEngine::TensorDesc, |
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 for this changes but:
auto ie_core = cv::gimpl::ie::wrap::getCore();
- Is unused variable?
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.
you right, it seems is unused
if (ctx.uu.params.device_id.find("GPU") != std::string::npos) { | ||
GAPI_LOG_DEBUG(nullptr, "Extract remote blob for GPU DX11 device_id: " << | ||
ctx.uu.params.device_id); | ||
// despite of layout, blob dimensions always follow in N,C,H,W order |
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.
Will only 4D blob be supported?
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 depends from openvino and code taken from openvino code make_shared_nv12_blob
// TODO NV12 surface supported only | ||
if (static_cast<InferenceEngine::ColorFormat>(frame_blob_params->second["COLOR_FORMAT"]) == | ||
InferenceEngine::ColorFormat::NV12) { | ||
InferenceEngine::Blob::Ptr y_blob = |
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.
Can be used InferenceEngine::NV12Blob
for this case?
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 has huge class hierarchy, but actually final blob must be one of descendant of ClBlob
which represents remote GPU memory. Maybe NV12Blob is present in class inheritance in the middle chain, i'm not sure
if (static_cast<InferenceEngine::ColorFormat>(frame_blob_params->second["COLOR_FORMAT"]) == | ||
InferenceEngine::ColorFormat::NV12) { | ||
InferenceEngine::Blob::Ptr y_blob = | ||
std::dynamic_pointer_cast<InferenceEngine::Blob>( |
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.
Can this be done with some make_..._blob functions? For interest.
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 it is: 524 - 550 lines related to make_shared_nv12_blob
API function and to declare that function we need to include specific OV include file with this make* API but this header file has a required dependency from OpenCL -HPP package which is not a part of openCV and should be not according to license type. So all make_* functions from OV simply unwrapped from its definition and copied here
*(ctx->uu.params.input_names.begin()), | ||
IE::make_shared_blob(this_blob, toIE(this_roi)), | ||
*ctx); | ||
setROIBlob(req, |
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, the function should be used for InferList and InferList2.
Can GPU ROI support appear in future?
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 mean - place such changes SetBlob -> SetROIBlob in other place? could you point me out, please?
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.
make_shared_blob for ROIs + SetBlob in InferList and InferList2. Is it necessary align this with SetRoiBlob? Now it is called in one place.
IE::Blob::Ptr roi_blob = IE::make_shared_blob(this_blob, toIE(rc)); |
this_blob = IE::make_shared_blob(blob_0, toIE(vec[list_idx])); |
const cv::Rect &roi, | ||
const IECallContext& ctx) { | ||
if (ctx.uu.params.device_id.find("GPU") != std::string::npos) { | ||
GAPI_LOG_DEBUG(nullptr, "Skip ROI blob creation for device_id: " << |
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.
This can confuse. M b it should be assert.
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.
definitely no: GPU blob doesn't support ROI but need to continue with SeBlob further
Let's apply limitation that ROI for GPU (right before OpenVINO 2.0 adaptation) must be done on G-API / GSource side manually
modules/gapi/src/streaming/onevpl/engine/decode/decode_engine_legacy.hpp
Outdated
Show resolved
Hide resolved
modules/gapi/src/streaming/onevpl/engine/decode/decode_session.hpp
Outdated
Show resolved
Hide resolved
modules/gapi/src/streaming/onevpl/engine/decode/decode_engine_legacy.cpp
Outdated
Show resolved
Hide resolved
modules/gapi/src/streaming/onevpl/engine/decode/decode_engine_legacy.cpp
Outdated
Show resolved
Hide resolved
modules/gapi/src/streaming/onevpl/engine/transcode/transcode_session.cpp
Show resolved
Hide resolved
modules/gapi/src/streaming/onevpl/engine/transcode/transcode_engine_legacy.cpp
Show resolved
Hide resolved
modules/gapi/src/streaming/onevpl/engine/transcode/transcode_engine_legacy.cpp
Show resolved
Hide resolved
modules/gapi/src/streaming/onevpl/engine/transcode/transcode_engine_legacy.cpp
Show resolved
Hide resolved
modules/gapi/src/streaming/onevpl/engine/transcode/transcode_engine_legacy.cpp
Outdated
Show resolved
Hide resolved
modules/gapi/src/streaming/onevpl/engine/transcode/transcode_engine_legacy.cpp
Outdated
Show resolved
Hide resolved
modules/gapi/src/streaming/onevpl/engine/transcode/transcode_engine_legacy.cpp
Show resolved
Hide resolved
modules/gapi/src/streaming/onevpl/engine/transcode/transcode_engine_legacy.cpp
Show resolved
Hide resolved
modules/gapi/src/streaming/onevpl/engine/transcode/transcode_engine_legacy.cpp
Outdated
Show resolved
Hide resolved
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.
Some unanswered questions.
static_cast<uint64_t>(value), false); | ||
} | ||
|
||
|
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.
Extra empty line.
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.
fixed
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.
Approved with comments
" For exploring list of supported transformations please find out " | ||
" vpp_* related stuff in" | ||
" gapi/include/opencv2/gapi/streaming/onevpl/cfg_params.hpp\n" | ||
" Pay attention that to obtain expected result In this case VPP " |
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 \n
here
" Please consider param \"source_preproc_enable=1\" and specify " | ||
" appropriated media frame transformation using oneVPL::VPP primitives" |
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 \n
here as well
" For exploring list of supported transformations please find out " | ||
" vpp_* related stuff in" |
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.
wow here too
ParamType* frame_blob_params = cv::util::any_cast<ParamType>(&any_blob_params); | ||
if (frame_blob_params == nullptr) { | ||
GAPI_Assert(false && "Incorrect type of blobParams: " | ||
"expected std::pair<InferenceEngine::TensorDesc," | ||
"InferenceEngine::ParamMap>"); | ||
} |
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 there should be bad_any_cast
already so no need to use *
return ctx.uu.rctx->CreateBlob(blob_params->first, | ||
blob_params->second); | ||
if (ctx.uu.params.device_id.find("GPU") != std::string::npos) { | ||
GAPI_LOG_DEBUG(nullptr, "Extract remote blob for GPU DX11 device_id: " << |
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 see the whole context but happens on Linux? GPU is perfectly valid there, but there's no 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.
no MediaFrame Adapter for it - DX11 only
There is a check with assert whilst we create Source that available device type
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.
Some remaining comments can be answered and addressed in the next PR
it seems before merge need to rebase with latter nv12 changes in giebackend |
WTF??? |
… due to OV issue
362b597
to
8251811
Compare
G-API: oneVPL DX11 inference * Draft GPU infer * Fix incorrect subresource_id for array of textures * Fix for TheOneSurface in different Frames * Turn on VPP param configuration * Add cropIn params * Remove infer sync sample * Remove comments * Remove DX11AllocResource extra init * Add condition for NV12 processing in giebackend * Add VPP frames pool param configurable * -M Remove extra WARN & INFOs, Fix custom MAC * Remove global vars from example, Fix some comments, Disable blobParam due to OV issue * Conflict resolving * Revert back pointer cast for cv::any
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