From e461031d40889938074e17a66fa173755a06dccd Mon Sep 17 00:00:00 2001 From: Sergey Ivanov Date: Thu, 10 Jun 2021 14:05:46 +0300 Subject: [PATCH] Merge pull request #20184 from sivanov-work:fix_gapi_empty_input G-API: Add standalone fix for graph empty input * Add sandalone fix for graph empty input * Apply some review comments * Fix whitespace * Apply review comment: make Mat check more deeper * Apply some comments * Remove tracer apply exception throwing * Apply comments: move validatio into gproto_priv.hpp * Apply minor text correction * Fix alignment, remove try-catch --- .../include/opencv2/gapi/gtype_traits.hpp | 13 ++++ modules/gapi/include/opencv2/gapi/gtyped.hpp | 1 + modules/gapi/src/api/gproto.cpp | 57 +++++++++++++++++- modules/gapi/src/api/gproto_priv.hpp | 3 + modules/gapi/src/compiler/gcompiler.cpp | 9 ++- modules/gapi/src/compiler/gcompiler.hpp | 2 +- modules/gapi/test/gapi_gcompiled_tests.cpp | 59 +++++++++++++++++++ 7 files changed, 140 insertions(+), 4 deletions(-) diff --git a/modules/gapi/include/opencv2/gapi/gtype_traits.hpp b/modules/gapi/include/opencv2/gapi/gtype_traits.hpp index 2e8dcb1aec..0b11b18485 100644 --- a/modules/gapi/include/opencv2/gapi/gtype_traits.hpp +++ b/modules/gapi/include/opencv2/gapi/gtype_traits.hpp @@ -43,6 +43,19 @@ namespace detail GOPAQUE, // a cv::GOpaqueU (note - exactly GOpaqueU, not GOpaque!) }; + template + constexpr const char* meta_to_string() noexcept; + template<> + constexpr const char* meta_to_string() noexcept { return "GMatDesc"; } + template<> + constexpr const char* meta_to_string() noexcept { return "GScalarDesc"; } + template<> + constexpr const char* meta_to_string() noexcept { return "GArrayDesc"; } + template<> + constexpr const char* meta_to_string() noexcept { return "GOpaqueDesc"; } + template<> + constexpr const char* meta_to_string() noexcept { return "GFrameDesc";} + // Describe G-API types (G-types) with traits. Mostly used by // cv::GArg to store meta information about types passed into // operation arguments. Please note that cv::GComputation is diff --git a/modules/gapi/include/opencv2/gapi/gtyped.hpp b/modules/gapi/include/opencv2/gapi/gtyped.hpp index 6fe52a62e1..27d9777944 100644 --- a/modules/gapi/include/opencv2/gapi/gtyped.hpp +++ b/modules/gapi/include/opencv2/gapi/gtyped.hpp @@ -35,6 +35,7 @@ namespace detail template<> struct ProtoToMeta { using type = cv::GScalarDesc; }; template struct ProtoToMeta > { using type = cv::GArrayDesc; }; template struct ProtoToMeta > { using type = cv::GOpaqueDesc; }; + template<> struct ProtoToMeta { using type = cv::GFrameDesc; }; template using ProtoToMetaT = typename ProtoToMeta::type; //workaround for MSVC 19.0 bug diff --git a/modules/gapi/src/api/gproto.cpp b/modules/gapi/src/api/gproto.cpp index 0c7c6462ee..94234c9b4d 100644 --- a/modules/gapi/src/api/gproto.cpp +++ b/modules/gapi/src/api/gproto.cpp @@ -14,6 +14,7 @@ #include "api/gorigin.hpp" #include "api/gproto_priv.hpp" +#include "logger.hpp" // FIXME: it should be a visitor! // FIXME: Reimplement with traits? @@ -201,6 +202,52 @@ bool cv::can_describe(const GMetaArgs &metas, const GRunArgs &args) }); } +void cv::gimpl::proto::validate_input_meta_arg(const cv::GMetaArg& meta) +{ + switch (meta.index()) + { + case cv::GMetaArg::index_of(): + { + cv::gimpl::proto::validate_input_meta(cv::util::get(meta)); //may throw + break; + } + default: + break; + } +} + +void cv::gimpl::proto::validate_input_meta(const cv::GMatDesc& meta) +{ + if (meta.dims.empty()) + { + if (!(meta.size.height > 0 && meta.size.width > 0)) + { + cv::util::throw_error + (std::logic_error( + "Image format is invalid. Size must contain positive values" + ", got width: " + std::to_string(meta.size.width ) + + (", height: ") + std::to_string(meta.size.height))); + } + + if (!(meta.chan > 0)) + { + cv::util::throw_error + (std::logic_error( + "Image format is invalid. Channel mustn't be negative value, got channel: " + + std::to_string(meta.chan))); + } + } + + if (!(meta.depth >= 0)) + { + cv::util::throw_error + (std::logic_error( + "Image format is invalid. Depth must be positive value, got depth: " + + std::to_string(meta.depth))); + } + // All checks are ok +} + // FIXME: Is it tested for all types? // FIXME: Where does this validation happen?? void cv::validate_input_arg(const GRunArg& arg) @@ -212,13 +259,15 @@ void cv::validate_input_arg(const GRunArg& arg) case GRunArg::index_of(): { const auto desc = cv::descr_of(util::get(arg)); - GAPI_Assert(desc.size.height != 0 && desc.size.width != 0 && "incorrect dimensions of cv::UMat!"); break; + cv::gimpl::proto::validate_input_meta(desc); //may throw + break; } #endif // !defined(GAPI_STANDALONE) case GRunArg::index_of(): { const auto desc = cv::descr_of(util::get(arg)); - GAPI_Assert(desc.size.height != 0 && desc.size.width != 0 && "incorrect dimensions of Mat!"); break; + cv::gimpl::proto::validate_input_meta(desc); //may throw + break; } default: // No extra handling @@ -228,9 +277,13 @@ void cv::validate_input_arg(const GRunArg& arg) void cv::validate_input_args(const GRunArgs& args) { + GAPI_LOG_DEBUG(nullptr, "Total count: " << args.size()); + size_t index = 0; for (const auto& arg : args) { + GAPI_LOG_DEBUG(nullptr, "Process index: " << index); validate_input_arg(arg); + index ++; } } diff --git a/modules/gapi/src/api/gproto_priv.hpp b/modules/gapi/src/api/gproto_priv.hpp index b1d71df881..e42afc3461 100644 --- a/modules/gapi/src/api/gproto_priv.hpp +++ b/modules/gapi/src/api/gproto_priv.hpp @@ -31,6 +31,9 @@ GProtoArg rewrap (const GArg &arg); // FIXME:: GAPI_EXPORTS because of tests only!! GAPI_EXPORTS const void* ptr (const GRunArgP &arg); +void validate_input_meta_arg(const GMetaArg& meta); +void validate_input_meta(const GMatDesc& meta); + } // proto } // gimpl } // cv diff --git a/modules/gapi/src/compiler/gcompiler.cpp b/modules/gapi/src/compiler/gcompiler.cpp index 2f442e40bd..7dd3c80cdf 100644 --- a/modules/gapi/src/compiler/gcompiler.cpp +++ b/modules/gapi/src/compiler/gcompiler.cpp @@ -343,19 +343,26 @@ void cv::gimpl::GCompiler::validateInputMeta() return false; // should never happen }; + GAPI_LOG_DEBUG(nullptr, "Total count: " << m_metas.size()); for (const auto meta_arg_idx : ade::util::indexed(ade::util::zip(m_metas, c_expr.m_ins))) { const auto &meta = std::get<0>(ade::util::value(meta_arg_idx)); const auto &proto = std::get<1>(ade::util::value(meta_arg_idx)); + const auto index = ade::util::index(meta_arg_idx); + GAPI_LOG_DEBUG(nullptr, "Process index: " << index); + + // check types validity if (!meta_matches(meta, proto)) { - const auto index = ade::util::index(meta_arg_idx); util::throw_error(std::logic_error ("GComputation object type / metadata descriptor mismatch " "(argument " + std::to_string(index) + ")")); // FIXME: report what we've got and what we've expected } + + // check value consistency + gimpl::proto::validate_input_meta_arg(meta); //may throw } // All checks are ok } diff --git a/modules/gapi/src/compiler/gcompiler.hpp b/modules/gapi/src/compiler/gcompiler.hpp index 85b05d54db..2712c79394 100644 --- a/modules/gapi/src/compiler/gcompiler.hpp +++ b/modules/gapi/src/compiler/gcompiler.hpp @@ -29,7 +29,7 @@ class GAPI_EXPORTS GCompiler cv::gapi::GKernelPackage m_all_kernels; cv::gapi::GNetPackage m_all_networks; - // Patters built from transformations + // Patterns built from transformations std::vector> m_all_patterns; diff --git a/modules/gapi/test/gapi_gcompiled_tests.cpp b/modules/gapi/test/gapi_gcompiled_tests.cpp index 7951a4c2ff..25d02fb8e9 100644 --- a/modules/gapi/test/gapi_gcompiled_tests.cpp +++ b/modules/gapi/test/gapi_gcompiled_tests.cpp @@ -37,6 +37,31 @@ namespace { } }; + + struct GCompiledValidateMetaEmpty: public ::testing::Test + { + cv::GMat in; + cv::GScalar scale; + cv::GComputation m_ucc; + + G_API_OP(GReturn42, (cv::GMat)>, "org.opencv.test.return_42") + { + static GOpaqueDesc outMeta(cv::GMatDesc /* in */) { return cv::empty_gopaque_desc(); } + }; + + GAPI_OCV_KERNEL(GOCVReturn42, GReturn42) + { + static void run(const cv::Mat &/* in */, int &out) + { + out = 42; + } + }; + + GCompiledValidateMetaEmpty() : m_ucc(cv::GIn(in), + cv::GOut(GReturn42::on(in))) + { + } + }; } // anonymous namespace TEST_F(GCompiledValidateMetaTyped, ValidMeta) @@ -170,4 +195,38 @@ TEST_F(GCompiledValidateMetaUntyped, InvalidMetaNumber) EXPECT_THROW(f(cv::gin(in1, sc), cv::gout(out1, out2)), std::logic_error); } +TEST_F(GCompiledValidateMetaEmpty, InvalidMatMetaCompile) +{ + EXPECT_THROW(m_ucc.compile(cv::empty_gmat_desc(), + cv::empty_scalar_desc()), + std::logic_error); +} + +TEST_F(GCompiledValidateMetaEmpty, InvalidMatMetaApply) +{ + cv::Mat emptyIn; + int out {}; + const auto pkg = cv::gapi::kernels(); + + EXPECT_THROW(m_ucc.apply(cv::gin(emptyIn), cv::gout(out), cv::compile_args(pkg)), + std::logic_error); +} + +TEST_F(GCompiledValidateMetaEmpty, ValidInvalidMatMetasApply) +{ + int out {}; + const auto pkg = cv::gapi::kernels(); + + cv::Mat nonEmptyMat = cv::Mat::eye(cv::Size(64,32), CV_8UC1); + m_ucc.apply(cv::gin(nonEmptyMat), cv::gout(out), cv::compile_args(pkg)); + EXPECT_EQ(out, 42); + + cv::Mat emptyIn; + EXPECT_THROW(m_ucc.apply(cv::gin(emptyIn), cv::gout(out), cv::compile_args(pkg)), + std::logic_error); + + out = 0; + m_ucc.apply(cv::gin(nonEmptyMat), cv::gout(out), cv::compile_args(pkg)); + EXPECT_EQ(out, 42); +} } // namespace opencv_test