Merge pull request #16081 from dmatveev:dm/ocv42_gapi_bugfixes

G-API: Fix various issues for 4.2 release

* G-API: Fix issues reported by Coverity

- Fixed: passing values by value instead of passing by reference

* G-API: Fix redundant std::move()'s in return statements

Fixes #15903

* G-API: Added a smarter handling of Stop messages in the pipeline

- This should fix the "expected 100, got 99 frames" problem
- Fixes #15882

* G-API: Pass enum instead of GKernelPackage in Streaming test parameters

- Likely fixes #15836

* G-API: Address review issues in new bugfix comments

modules/gapi/src/backends/fluid/gfluidbuffer.cpp

@@ -347,12 +347,12 @@ std::unique_ptr<fluid::BufferStorage> createStorage(int capacity, int desc_width
         std::unique_ptr<fluid::BufferStorageWithBorder> storage(new BufferStorageWithBorder);
         storage->init(type, border_size, border.value());
         storage->create(capacity, desc_width, type);
-        return std::move(storage);
+        return storage;
     }
 
     std::unique_ptr<BufferStorageWithoutBorder> storage(new BufferStorageWithoutBorder);
     storage->create(capacity, desc_width, type);
-    return std::move(storage);
+    return storage;
 }
 
 std::unique_ptr<BufferStorage> createStorage(const cv::gapi::own::Mat& data, cv::gapi::own::Rect roi);
@@ -360,7 +360,7 @@ std::unique_ptr<BufferStorage> createStorage(const cv::gapi::own::Mat& data, cv:
 {
     std::unique_ptr<BufferStorageWithoutBorder> storage(new BufferStorageWithoutBorder);
     storage->attach(data, roi);
-    return std::move(storage);
+    return storage;
 }
 } // namespace
 } // namespace fluid

modules/gapi/src/compiler/gmodel.cpp

@@ -118,7 +118,7 @@ void GModel::linkOut(Graph &g, ade::NodeHandle opH, ade::NodeHandle objH, std::s
     op.outs[out_port] = RcDesc{gm.rc, gm.shape, {}};
 }
 
-std::vector<ade::NodeHandle> GModel::orderedInputs(ConstGraph &g, ade::NodeHandle nh)
+std::vector<ade::NodeHandle> GModel::orderedInputs(const ConstGraph &g, ade::NodeHandle nh)
 {
     std::vector<ade::NodeHandle> sorted_in_nhs(nh->inEdges().size());
     for (const auto& in_eh : nh->inEdges())
@@ -130,7 +130,7 @@ std::vector<ade::NodeHandle> GModel::orderedInputs(ConstGraph &g, ade::NodeHandl
     return sorted_in_nhs;
 }
 
-std::vector<ade::NodeHandle> GModel::orderedOutputs(ConstGraph &g, ade::NodeHandle nh)
+std::vector<ade::NodeHandle> GModel::orderedOutputs(const ConstGraph &g, ade::NodeHandle nh)
 {
     std::vector<ade::NodeHandle> sorted_out_nhs(nh->outEdges().size());
     for (const auto& out_eh : nh->outEdges())
@@ -227,7 +227,7 @@ void GModel::redirectWriter(Graph &g, ade::NodeHandle from, ade::NodeHandle to)
     linkOut(g, op, to, output.port);
 }
 
-GMetaArgs GModel::collectInputMeta(GModel::ConstGraph cg, ade::NodeHandle node)
+GMetaArgs GModel::collectInputMeta(const GModel::ConstGraph &cg, ade::NodeHandle node)
 {
     GAPI_Assert(cg.metadata(node).get<NodeType>().t == NodeType::OP);
     GMetaArgs in_meta_args(cg.metadata(node).get<Op>().args.size());
@@ -254,7 +254,7 @@ ade::EdgeHandle GModel::getInEdgeByPort(const GModel::ConstGraph& cg,
     return *edge;
 }
 
-GMetaArgs GModel::collectOutputMeta(GModel::ConstGraph cg, ade::NodeHandle node)
+GMetaArgs GModel::collectOutputMeta(const GModel::ConstGraph &cg, ade::NodeHandle node)
 {
     GAPI_Assert(cg.metadata(node).get<NodeType>().t == NodeType::OP);
     GMetaArgs out_meta_args(cg.metadata(node).get<Op>().outs.size());

modules/gapi/src/compiler/gmodel.hpp

@@ -260,15 +260,15 @@ namespace GModel
     GAPI_EXPORTS void redirectReaders(Graph &g, ade::NodeHandle from, ade::NodeHandle to);
     GAPI_EXPORTS void redirectWriter (Graph &g, ade::NodeHandle from, ade::NodeHandle to);
 
-    GAPI_EXPORTS std::vector<ade::NodeHandle> orderedInputs (ConstGraph &g, ade::NodeHandle nh);
-    GAPI_EXPORTS std::vector<ade::NodeHandle> orderedOutputs(ConstGraph &g, ade::NodeHandle nh);
+    GAPI_EXPORTS std::vector<ade::NodeHandle> orderedInputs (const ConstGraph &g, ade::NodeHandle nh);
+    GAPI_EXPORTS std::vector<ade::NodeHandle> orderedOutputs(const ConstGraph &g, ade::NodeHandle nh);
 
     // Returns input meta array for given op node
     // Array is sparse, as metadata for non-gapi input objects is empty
     // TODO:
     // Cover with tests!!
-    GAPI_EXPORTS GMetaArgs collectInputMeta(GModel::ConstGraph cg, ade::NodeHandle node);
-    GAPI_EXPORTS GMetaArgs collectOutputMeta(GModel::ConstGraph cg, ade::NodeHandle node);
+    GAPI_EXPORTS GMetaArgs collectInputMeta(const GModel::ConstGraph &cg, ade::NodeHandle node);
+    GAPI_EXPORTS GMetaArgs collectOutputMeta(const GModel::ConstGraph &cg, ade::NodeHandle node);
 
     GAPI_EXPORTS ade::EdgeHandle getInEdgeByPort(const GModel::ConstGraph& cg, const ade::NodeHandle& nh, std::size_t in_port);
 

modules/gapi/src/executor/gstreamingexecutor.hpp

@@ -32,7 +32,13 @@ namespace gimpl {
 
 namespace stream {
 struct Start {};
-struct Stop {};
+struct Stop {
+    enum class Kind {
+        HARD, // a hard-stop: end-of-pipeline reached or stop() called
+        CNST, // a soft-stop emitted for/by constant sources (see QueueReader)
+    } kind = Kind::HARD;
+    cv::GRunArg cdata; // const data for CNST stop
+};
 
 using Cmd = cv::util::variant
     < cv::util::monostate
@@ -91,7 +97,7 @@ protected:
         cv::GMetaArgs       out_metas;
         ade::NodeHandle     nh;
 
-        std::vector<GRunArg> in_constants;
+        cv::GRunArgs in_constants;
 
         std::shared_ptr<GIslandExecutable> isl_exec;
     };
@@ -104,6 +110,8 @@ protected:
     };
     std::vector<DataDesc> m_slots;
 
+    cv::GRunArgs m_const_vals;
+
     // Order in these vectors follows the GComputaion's protocol
     std::vector<ade::NodeHandle> m_emitters;
     std::vector<ade::NodeHandle> m_sinks;

modules/gapi/test/internal/gapi_int_pattern_matching_test.cpp

@@ -47,12 +47,12 @@ void initGModel(ade::Graph& gr,
     gm.metadata().set(p);
 }
 
-bool isConsumedBy(cv::gimpl::GModel::Graph gm, ade::NodeHandle data_nh, ade::NodeHandle op_nh) {
+bool isConsumedBy(const cv::gimpl::GModel::ConstGraph &gm, ade::NodeHandle data_nh, ade::NodeHandle op_nh) {
     auto oi = cv::gimpl::GModel::orderedInputs(gm, op_nh);
     return std::find(oi.begin(), oi.end(), data_nh) != oi.end();
 }
 
-std::string opName(cv::gimpl::GModel::Graph gm, ade::NodeHandle op_nh) {
+std::string opName(const cv::gimpl::GModel::ConstGraph &gm, ade::NodeHandle op_nh) {
     return gm.metadata(op_nh).get<cv::gimpl::Op>().k.name;
 }
 

modules/gapi/test/streaming/gapi_streaming_tests.cpp

@@ -39,47 +39,65 @@ void initTestDataPath()
 #endif // WINRT
 }
 
-cv::gapi::GKernelPackage OCV_KERNELS()
+enum class KernelPackage: int
 {
-    static cv::gapi::GKernelPackage pkg =
-        cv::gapi::combine(cv::gapi::core::cpu::kernels(),
-                          cv::gapi::imgproc::cpu::kernels());
-    return pkg;
-}
-
-cv::gapi::GKernelPackage OCV_FLUID_KERNELS()
-{
-    static cv::gapi::GKernelPackage pkg =
-        cv::gapi::combine(OCV_KERNELS(),
-                          cv::gapi::core::fluid::kernels());
-    return pkg;
-}
-
-#if 0
-// FIXME: OpenCL backend seem to work fine with Streaming
-// however the results are not very bit exact with CPU
-// It may be a problem but may be just implementation innacuracy.
-// Need to customize the comparison function in tests where OpenCL
-// is involved.
-cv::gapi::GKernelPackage OCL_KERNELS()
-{
-    static cv::gapi::GKernelPackage pkg =
-        cv::gapi::combine(cv::gapi::core::ocl::kernels(),
-                          cv::gapi::imgproc::ocl::kernels());
-    return pkg;
-}
-
-cv::gapi::GKernelPackage OCL_FLUID_KERNELS()
+    OCV,
+    OCV_FLUID,
+    OCL,
+    OCL_FLUID,
+};
+std::ostream& operator<< (std::ostream &os, const KernelPackage &e)
 {
-    static cv::gapi::GKernelPackage pkg =
-        cv::gapi::combine(OCL_KERNELS(),
-                          cv::gapi::core::fluid::kernels());
-    return pkg;
+    switch (e)
+    {
+#define _C(X) case KernelPackage::X: os << #X; break
+        _C(OCV);
+        _C(OCV_FLUID);
+        _C(OCL);
+        _C(OCL_FLUID);
+#undef _C
+    default: GAPI_Assert(false);
+    }
+    return os;
 }
-#endif // 0
 
-struct GAPI_Streaming: public ::testing::TestWithParam<cv::gapi::GKernelPackage> {
+struct GAPI_Streaming: public ::testing::TestWithParam<KernelPackage> {
     GAPI_Streaming() { initTestDataPath(); }
+
+    cv::gapi::GKernelPackage getKernelPackage()
+    {
+        using namespace cv::gapi;
+        switch (GetParam())
+        {
+        case KernelPackage::OCV:
+            return cv::gapi::combine(core::cpu::kernels(),
+                                     imgproc::cpu::kernels());
+            break;
+
+        case KernelPackage::OCV_FLUID:
+            return cv::gapi::combine(core::cpu::kernels(),
+                                     imgproc::cpu::kernels(),
+                                     core::fluid::kernels());
+            break;
+
+        // FIXME: OpenCL backend seem to work fine with Streaming
+        // however the results are not very bit exact with CPU
+        // It may be a problem but may be just implementation innacuracy.
+        // Need to customize the comparison function in tests where OpenCL
+        // is involved.
+        case KernelPackage::OCL:
+            return cv::gapi::combine(core::ocl::kernels(),
+                                     imgproc::ocl::kernels());
+            break;
+
+        case KernelPackage::OCL_FLUID:
+            return cv::gapi::combine(core::ocl::kernels(),
+                                     imgproc::ocl::kernels(),
+                                     core::fluid::kernels());
+            break;
+        }
+        throw std::logic_error("Unknown package");
+    }
 };
 
 } // anonymous namespace
@@ -122,7 +140,7 @@ TEST_P(GAPI_Streaming, SmokeTest_ConstInput_GMat)
 
     // Compilation & testing
     auto ccomp = c.compileStreaming(cv::descr_of(in_mat),
-                                    cv::compile_args(cv::gapi::use_only{GetParam()}));
+                                    cv::compile_args(cv::gapi::use_only{getKernelPackage()}));
     EXPECT_TRUE(ccomp);
     EXPECT_FALSE(ccomp.running());
 
@@ -167,7 +185,7 @@ TEST_P(GAPI_Streaming, SmokeTest_VideoInput_GMat)
 
     // Compilation & testing
     auto ccomp = c.compileStreaming(cv::GMatDesc{CV_8U,3,cv::Size{768,576}},
-                                    cv::compile_args(cv::gapi::use_only{GetParam()}));
+                                    cv::compile_args(cv::gapi::use_only{getKernelPackage()}));
     EXPECT_TRUE(ccomp);
     EXPECT_FALSE(ccomp.running());
 
@@ -213,7 +231,7 @@ TEST_P(GAPI_Streaming, Regression_CompileTimeScalar)
     cv::GComputation c(cv::GIn(in), cv::GOut(tmp, tmp + 1));
 
     auto ccomp = c.compileStreaming(cv::GMatDesc{CV_8U,3,cv::Size{768,512}},
-                                    cv::compile_args(cv::gapi::use_only{GetParam()}));
+                                    cv::compile_args(cv::gapi::use_only{getKernelPackage()}));
 
     cv::Mat in_mat = cv::imread(findDataFile("cv/edgefilter/kodim23.png"));
     cv::Mat out_mat1, out_mat2;
@@ -236,7 +254,7 @@ TEST_P(GAPI_Streaming, SmokeTest_StartRestart)
     cv::GComputation c(cv::GIn(in), cv::GOut(cv::gapi::copy(in), out));
 
     auto ccomp = c.compileStreaming(cv::GMatDesc{CV_8U,3,cv::Size{768,576}},
-                                    cv::compile_args(cv::gapi::use_only{GetParam()}));
+                                    cv::compile_args(cv::gapi::use_only{getKernelPackage()}));
     EXPECT_TRUE(ccomp);
     EXPECT_FALSE(ccomp.running());
 
@@ -273,7 +291,7 @@ TEST_P(GAPI_Streaming, SmokeTest_VideoConstSource_NoHang)
         cv::GMat in;
         return cv::GComputation(in, cv::gapi::copy(in));
     }).compileStreaming(cv::GMatDesc{CV_8U,3,cv::Size{768,576}},
-                        cv::compile_args(cv::gapi::use_only{GetParam()}));
+                        cv::compile_args(cv::gapi::use_only{getKernelPackage()}));
 
     refc.setSource(gapi::wip::make_src<cv::gapi::wip::GCaptureSource>(findDataFile("cv/video/768x576.avi")));
     refc.start();
@@ -290,7 +308,7 @@ TEST_P(GAPI_Streaming, SmokeTest_VideoConstSource_NoHang)
     auto testc = cv::GComputation(cv::GIn(in, in2), cv::GOut(out))
         .compileStreaming(cv::GMatDesc{CV_8U,3,cv::Size{256,256}},
                           cv::GMatDesc{CV_8U,3,cv::Size{768,576}},
-                          cv::compile_args(cv::gapi::use_only{GetParam()}));
+                          cv::compile_args(cv::gapi::use_only{getKernelPackage()}));
 
     cv::Mat in_const = cv::Mat::eye(cv::Size(256,256), CV_8UC3);
     testc.setSource(cv::gin(in_const,
@@ -311,7 +329,7 @@ TEST_P(GAPI_Streaming, SmokeTest_AutoMeta)
     cv::GMat out = blr - in;
 
     auto testc = cv::GComputation(cv::GIn(in, in2), cv::GOut(out))
-        .compileStreaming(cv::compile_args(cv::gapi::use_only{GetParam()}));
+        .compileStreaming(cv::compile_args(cv::gapi::use_only{getKernelPackage()}));
 
     cv::Mat in_const = cv::Mat::eye(cv::Size(256,256), CV_8UC3);
     cv::Mat tmp;
@@ -345,7 +363,7 @@ TEST_P(GAPI_Streaming, SmokeTest_AutoMeta_2xConstMat)
     cv::GMat out = blr - in;
 
     auto testc = cv::GComputation(cv::GIn(in, in2), cv::GOut(out))
-        .compileStreaming(cv::compile_args(cv::gapi::use_only{GetParam()}));
+        .compileStreaming(cv::compile_args(cv::gapi::use_only{getKernelPackage()}));
 
     cv::Mat in_const = cv::Mat::eye(cv::Size(256,256), CV_8UC3);
     cv::Mat tmp;
@@ -376,7 +394,7 @@ TEST_P(GAPI_Streaming, SmokeTest_AutoMeta_VideoScalar)
     cv::GMat out_m = in_m * in_s;
 
     auto testc = cv::GComputation(cv::GIn(in_m, in_s), cv::GOut(out_m))
-        .compileStreaming(cv::compile_args(cv::gapi::use_only{GetParam()}));
+        .compileStreaming(cv::compile_args(cv::gapi::use_only{getKernelPackage()}));
 
     cv::Mat tmp;
     // Test with one video source and scalar
@@ -399,10 +417,10 @@ TEST_P(GAPI_Streaming, SmokeTest_AutoMeta_VideoScalar)
 }
 
 INSTANTIATE_TEST_CASE_P(TestStreaming, GAPI_Streaming,
-                        Values(  OCV_KERNELS()
-                                 //, OCL_KERNELS() // FIXME: Fails bit-exactness check, maybe relax it?
-                               , OCV_FLUID_KERNELS()
-                                 //, OCL_FLUID_KERNELS() // FIXME: Fails bit-exactness check, maybe relax it?
+                        Values(  KernelPackage::OCV
+                             //, KernelPackage::OCL // FIXME: Fails bit-exactness check, maybe relax it?
+                               , KernelPackage::OCV_FLUID
+                             //, KernelPackage::OCL // FIXME: Fails bit-exactness check, maybe relax it?
                                ));
 
 namespace TypesTest