- [PP] Addded ability to preprocess inputs into plugin desired format by anton-potapov · Pull Request #857 · openvinotoolkit/openvino

anton-potapov · 2020-06-10T11:40:26Z

[PP] Addded ability to preprocess inputs into plugin
desired format

changed InferRequestInternal:

added _deviceInputs member to store plugin desired perprocessing
targets
added default argument to preProcessingRequired to describe plugin
specific desired preprocessing target
SetBlob and GetBlob to deal with plugin desired preprocessing targets
(_deviceInputs)
added addInputPreProcessingFor helper method to avoid code
duplication

changed TEMPLATE plugin to use new functionality:

removed explicit presicion conversion (to use built-in one of
InferRequestInternal)
_networkInputBlobs to use InferRequestInternal::_deviceInputs

ilyachur · 2020-08-17T05:10:24Z

inference-engine/src/plugin_api/cpp_interfaces/impl/ie_infer_request_internal.hpp

+        for (auto& pp_result : inputs) {
+            auto const& input_name    = pp_result.first;
+            auto & result_blob        = pp_result.second;
+            auto const& ir_input_blob = _inputs[input_name];


What happens if result_blob (inputs[input_name]) and ir_input_blob(_inputs[input_name]) point to the one blob?

it's handled in the device_specific_preproc_req flag

not relevant any more - patch is reworked

ilyachur · 2020-08-17T05:13:11Z

inference-engine/src/preprocessing/ie_preprocess_data.cpp

+    execute(_roiBlob, outBlob, info, serial, batchSize);
+}
+void PreProcessData::execute(const Blob::Ptr& inBlob, Blob::Ptr &outBlob, const PreProcessInfo& info, bool serial, int batchSize) {
+


nit: redundant line

not relevant any more - patch is reworked

vinograd47 · 2020-08-17T10:07:02Z

inference-engine/src/preprocessing/ie_preprocess_data.cpp

+        THROW_IE_EXCEPTION << "Input pre-processing is called with null inBlob";
    }

    batchSize = PreprocEngine::getCorrectBatchSize(batchSize, _roiBlob);


I suppose _roiBlob should be replaced with inBlob everywhere in the function body (including its sub-routines).

not relevant any more - patch is reworked

vinograd47 · 2020-08-17T10:09:04Z

inference-engine/src/plugin_api/cpp_interfaces/impl/ie_infer_request_internal.hpp

+            auto                   pp_it  = _preProcData.find(input_name);
+            const bool preprocDataExists  = (pp_it != _preProcData.end());
+
+            Blob::Ptr roi_blob = (preprocDataExists) ? (*pp_it).second ->getRoiBlob() : Blob::Ptr{nullptr};


Does getRoiBlob always returns non-NULL? Just from its name I would expect that this is an optional crop pre-processing, which might not be enabled by user (for example, only color space conversion preprocessing).

I think yes, if preprocDataExists is true, it means that preporoc engine is created for particular input and setRoiBlob is called in SetBlob

@ilya-lavrenov is correct.

getRoiBlob is a bad name, and now should be something like getUserBlob. I have added a FIXME to rename it (and accompanying one seRoiBlob )

vinograd47 · 2020-08-17T10:09:58Z

inference-engine/src/plugin_api/cpp_interfaces/impl/ie_infer_request_internal.hpp

+            Blob::Ptr roi_blob = (preprocDataExists) ? (*pp_it).second ->getRoiBlob() : Blob::Ptr{nullptr};
+
+            const bool device_specific_preproc_req = (ir_input_blob != result_blob);
+            const bool user_specific_preproc_req   = (roi_blob != nullptr);


Based on the previous comment - I would expect that user_specific_preproc_req == preprocDataExists. And roi_blob is just an optional pre-processing configuration.

roi_blob is not empty, if preprocDataExists is true

not relevant any more - patch is reworked

apankratovantonp · 2020-08-17T12:29:03Z

inference-engine/src/preprocessing/ie_preprocess_data.cpp

    Blob::Ptr getRoiBlob() const override;

    void execute(Blob::Ptr &outBlob, const PreProcessInfo& info, bool serial, int batchSize = -1) override;
+    void execute(const Blob::Ptr& inBlob, Blob::Ptr &outBlob, const PreProcessInfo& info, bool serial, int batchSize = -1) override;


I think it is better to rename outBlob to preprocessedBlobs as inference requests also has some outBlobs.

inBlob to userBlob?

renamed outBlob to preprocessedBlob and _roiBlob to _userBlob. (though renaming of according methods is postponed)

apankratovantonp · 2020-08-17T12:34:41Z

@anton-potapov Please add usage example with explanations into openvino/docs/template_plugin.

ilya-lavrenov · 2020-08-26T14:24:10Z

inference-engine/src/plugin_api/cpp_interfaces/impl/ie_infer_request_internal.hpp

+                if (device_specific_preproc_req && !preprocDataExists) {
+                    pp_it = _preProcData.emplace_hint(_preProcData.end(), input_name, CreatePreprocDataHelper());
+                }
+                auto& pp = (*pp_it).second;


(*pp_it).second -> pp_it->second

not relevant any more - patch is reworked

ilya-lavrenov · 2020-08-26T14:35:21Z

inference-engine/src/preprocessing/ie_preprocess_data.cpp

@@ -779,6 +779,7 @@ class PreProcessData : public IPreProcessData {
    Blob::Ptr getRoiBlob() const override;

    void execute(Blob::Ptr &outBlob, const PreProcessInfo& info, bool serial, int batchSize = -1) override;


do we need a method with a single blob argument?

not relevant any more - patch is reworked

ilya-lavrenov · 2020-08-26T14:43:55Z

inference-engine/src/plugin_api/cpp_interfaces/impl/ie_infer_request_internal.hpp

+                }
+                auto& pp = (*pp_it).second;
+
+                auto src_blob = (roi_blob) ? roi_blob : ir_input_blob;


src_blob -> user_blob?

not relevant any more - patch is reworked

ilya-lavrenov · 2020-08-26T14:47:25Z

inference-engine/src/plugin_api/cpp_interfaces/impl/ie_infer_request_internal.hpp

@@ -228,13 +228,29 @@ class InferRequestInternal : virtual public IInferRequestInternal {
     * @param serial Whether to use multiple threads to execute the step
     */
    void execDataPreprocessing(InferenceEngine::BlobMap& inputs, bool serial = false) {


why inputs? I suppose they are outputs?

renamed to preprocessedBlobs

ilya-lavrenov · 2020-08-26T14:48:30Z

inference-engine/src/plugin_api/cpp_interfaces/impl/ie_infer_request_internal.hpp

+
+            Blob::Ptr roi_blob = (preprocDataExists) ? (*pp_it).second ->getRoiBlob() : Blob::Ptr{nullptr};
+
+            const bool device_specific_preproc_req = (ir_input_blob != result_blob);


I want to see the usage of this function when device_specific_preproc_req is true, because currently all plugins call this function with _inputs and device_specific_preproc_req is always false

not relevant any more - patch is reworked

ilya-lavrenov · 2020-09-29T20:42:19Z

@anton-potapov when are we going to proceed with this PR?

anton-potapov · 2020-12-01T14:42:54Z

@anton-potapov Please add usage example with explanations into openvino/docs/template_plugin.

added

anton-potapov · 2020-12-01T14:57:54Z

@ilya-lavrenov ping

inference-engine/src/plugin_api/cpp_interfaces/impl/ie_infer_request_internal.hpp

inference-engine/src/preprocessing/ie_preprocess_data.hpp

ilya-lavrenov · 2020-12-09T11:27:05Z

@ilyachur @vinograd47 please, have a look as well.

ilyachur

In general LGTM.
But I have a question. Do we have any plugins (excludes the template plugin) which use it?
Because I have found when added the tests for pre-processing that majority of plugins use the own code for pre-processing.

anton-potapov · 2020-12-09T11:41:13Z

... Do we have any plugins (excludes the template plugin) which use it?

Not yet.

Because I have found when added the tests for pre-processing that majority of plugins use the own code for pre-processing.

This is correct, test were added in advance to make sure the behaviour is not broken during the refactoring.

vinograd47

Looks OK to me

desired format changed InferRequestInternal: - added _deviceInputs member to store plugin desired perprocessing targets - added default argument to preProcessingRequired to describe plugin specific desired preprocessing target - SetBlob and GetBlob to deal with plugin desired preprocessing targets (_deviceInputs) - added addInputPreProcessingFor helper method to avoid code duplication changed TEMPLATE plugin to use new functionality: - removed explicit presicion conversion (to use built-in one of InferRequestInternal) - _networkInputBlobs to use InferRequestInternal::_deviceInputs

…#857) desired format changed InferRequestInternal: - added _deviceInputs member to store plugin desired perprocessing targets - added default argument to preProcessingRequired to describe plugin specific desired preprocessing target - SetBlob and GetBlob to deal with plugin desired preprocessing targets (_deviceInputs) - added addInputPreProcessingFor helper method to avoid code duplication changed TEMPLATE plugin to use new functionality: - removed explicit presicion conversion (to use built-in one of InferRequestInternal) - _networkInputBlobs to use InferRequestInternal::_deviceInputs

anton-potapov requested a review from a team June 10, 2020 11:40

anton-potapov force-pushed the preproc_streamlining branch 2 times, most recently from fe8d75c to be11fd8 Compare June 11, 2020 08:11

ilya-lavrenov added the category: preprocessing (deprecated) Inference Engine Preprocessing library label Jul 21, 2020

anton-potapov force-pushed the preproc_streamlining branch from be11fd8 to 7e5b7eb Compare July 24, 2020 09:47

openvino-pushbot added the category: inference OpenVINO Runtime library - Inference label Aug 11, 2020

ilya-lavrenov self-assigned this Aug 15, 2020

ilya-lavrenov requested review from apankratovantonp, ilyachur and vinograd47 August 15, 2020 11:55

ilyachur reviewed Aug 17, 2020

View reviewed changes

ilyachur self-assigned this Aug 17, 2020

vinograd47 reviewed Aug 17, 2020

View reviewed changes

apankratovantonp reviewed Aug 17, 2020

View reviewed changes

ilya-lavrenov reviewed Aug 26, 2020

View reviewed changes

anton-potapov mentioned this pull request Sep 1, 2020

[PP GAPI] Addded tests to cover exisiting precision conversions done by some plugins #1976

Merged

anton-potapov mentioned this pull request Sep 16, 2020

[PP GAPI] Extended preprocessing graph to support precision conversions #2290

Merged

anton-potapov force-pushed the preproc_streamlining branch from 7e5b7eb to 02139a7 Compare November 9, 2020 09:26

anton-potapov requested a review from a team as a code owner November 9, 2020 09:26

anton-potapov requested a review from a team November 9, 2020 09:26

anton-potapov force-pushed the preproc_streamlining branch from 02139a7 to 72dc849 Compare November 12, 2020 12:15

anton-potapov force-pushed the preproc_streamlining branch 2 times, most recently from c9cff58 to e8946d2 Compare November 24, 2020 09:38

anton-potapov force-pushed the preproc_streamlining branch 2 times, most recently from d62ba8e to b63d411 Compare December 1, 2020 14:31

anton-potapov changed the title ~~- [PrepProcessing] Addded ability to preprocess inputs into plugin~~ - [PrepProcessing] Addded ability to preprocess inputs into plugin desired format Dec 1, 2020

anton-potapov force-pushed the preproc_streamlining branch from b63d411 to 12e96c2 Compare December 1, 2020 14:52

anton-potapov force-pushed the preproc_streamlining branch 2 times, most recently from d90f60b to f8ca497 Compare December 7, 2020 08:52

ilya-lavrenov reviewed Dec 8, 2020

View reviewed changes

ilya-lavrenov added this to the 2021.3 milestone Dec 8, 2020

anton-potapov force-pushed the preproc_streamlining branch 2 times, most recently from 929a08f to 1223f07 Compare December 9, 2020 10:37

ilya-lavrenov approved these changes Dec 9, 2020

View reviewed changes

ilyachur approved these changes Dec 9, 2020

View reviewed changes

vinograd47 approved these changes Dec 9, 2020

View reviewed changes

anton-potapov force-pushed the preproc_streamlining branch from 1223f07 to 3b97794 Compare December 10, 2020 19:40

anton-potapov changed the title ~~- [PrepProcessing] Addded ability to preprocess inputs into plugin desired format~~ - [PP] Addded ability to preprocess inputs into plugin desired format Dec 10, 2020

anton-potapov force-pushed the preproc_streamlining branch 2 times, most recently from 3fcc2e1 to 71f443f Compare December 11, 2020 09:28

anton-potapov force-pushed the preproc_streamlining branch from 71f443f to b974c2c Compare December 11, 2020 12:50

ilya-lavrenov approved these changes Dec 11, 2020

View reviewed changes

ilya-lavrenov merged commit 2495eaf into openvinotoolkit:master Dec 11, 2020

nikita-kud mentioned this pull request Feb 2, 2021

[IE Myriad] Fix for myriadInferTests_nightly.NCHW_Input test #4141

Merged

2 tasks

		@@ -779,6 +779,7 @@ class PreProcessData : public IPreProcessData {
		Blob::Ptr getRoiBlob() const override;

		void execute(Blob::Ptr &outBlob, const PreProcessInfo& info, bool serial, int batchSize = -1) override;


		Blob::Ptr roi_blob = (preprocDataExists) ? (*pp_it).second ->getRoiBlob() : Blob::Ptr{nullptr};

		const bool device_specific_preproc_req = (ir_input_blob != result_blob);

Conversation

anton-potapov commented Jun 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

anton-potapov Dec 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

anton-potapov Dec 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

apankratovantonp commented Aug 17, 2020

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!

ilya-lavrenov commented Sep 29, 2020

Uh oh!

anton-potapov commented Dec 1, 2020

Uh oh!

anton-potapov commented Dec 1, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ilya-lavrenov commented Dec 9, 2020

Uh oh!

ilyachur left a comment

Choose a reason for hiding this comment

anton-potapov commented Jun 10, 2020 •

edited

Loading

anton-potapov Dec 1, 2020 •

edited

Loading

anton-potapov Dec 1, 2020 •

edited

Loading

anton-potapov commented Dec 9, 2020 •

edited

Loading