Skip to content

Commit

Permalink
Fix problem with handleNewStream functionality
Browse files Browse the repository at this point in the history
  • Loading branch information
@TolyaTalamanov
TolyaTalamanov committed Sep 6, 2022
1 parent bf54a37 commit 7955469
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 98 deletions.
14 changes: 4 additions & 10 deletions modules/gapi/include/opencv2/gapi/python/python.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,7 @@ struct GPythonContext
const cv::GMetaArgs &in_metas;
const cv::GTypesInfo &out_info;

bool m_isStateful;

GArg m_state;
cv::optional<cv::GArg> m_state;
};

using Impl = std::function<cv::GRunArgs(const GPythonContext&)>;
Expand All @@ -46,13 +44,9 @@ class GAPI_EXPORTS GPythonKernel
GPythonKernel() = default;
GPythonKernel(Impl run, Setup setup);

cv::GRunArgs operator()(const GPythonContext& ctx);

bool m_isStateful = false;
Setup m_setup = nullptr;

private:
Impl m_run;
Impl run;
Setup setup = nullptr;
bool is_stateful = false;
};

class GAPI_EXPORTS GPythonFunctor : public cv::gapi::GFunctor
Expand Down
120 changes: 51 additions & 69 deletions modules/gapi/misc/python/pyopencv_gapi.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,8 @@ static cv::GRunArgs run_py_kernel(cv::detail::PyObjectHolder kernel,
// NB: Doesn't increase reference counter (false),
// because PyObject already have ownership.
// In case exception decrement reference counter.
cv::detail::PyObjectHolder args(PyTuple_New(ctx.m_isStateful ? ins.size() + 1: ins.size()), false);
cv::detail::PyObjectHolder args(
PyTuple_New(ctx.m_state.has_value() ? ins.size() + 1 : ins.size()), false);
for (size_t i = 0; i < ins.size(); ++i)
{
// NB: If meta is monostate then object isn't associated with G-TYPE.
Expand Down Expand Up @@ -691,9 +692,9 @@ static cv::GRunArgs run_py_kernel(cv::detail::PyObjectHolder kernel,
++in_idx;
}

if (ctx.m_isStateful)
if (ctx.m_state.has_value())
{
PyTuple_SetItem(args.get(), ins.size(), pyopencv_from(ctx.m_state));
PyTuple_SetItem(args.get(), ins.size(), pyopencv_from(ctx.m_state.value()));
}

// NB: Doesn't increase reference counter (false).
Expand Down Expand Up @@ -742,8 +743,45 @@ static cv::GRunArgs run_py_kernel(cv::detail::PyObjectHolder kernel,
return outs;
}

static cv::GArg setup_py(cv::detail::PyObjectHolder out_meta, const cv::GMetaArgs& meta,
const cv::GArgs& gargs)
static void unpackMetasToTuple(const cv::GMetaArgs& meta,
const cv::GArgs& gargs,
cv::detail::PyObjectHolder& tuple)
{
size_t idx = 0;
for (auto&& m : meta)
{
switch (m.index())
{
case cv::GMetaArg::index_of<cv::GMatDesc>():
PyTuple_SetItem(tuple.get(), idx, pyopencv_from(cv::util::get<cv::GMatDesc>(m)));
break;
case cv::GMetaArg::index_of<cv::GScalarDesc>():
PyTuple_SetItem(tuple.get(), idx,
pyopencv_from(cv::util::get<cv::GScalarDesc>(m)));
break;
case cv::GMetaArg::index_of<cv::GArrayDesc>():
PyTuple_SetItem(tuple.get(), idx,
pyopencv_from(cv::util::get<cv::GArrayDesc>(m)));
break;
case cv::GMetaArg::index_of<cv::GOpaqueDesc>():
PyTuple_SetItem(tuple.get(), idx,
pyopencv_from(cv::util::get<cv::GOpaqueDesc>(m)));
break;
case cv::GMetaArg::index_of<cv::util::monostate>():
PyTuple_SetItem(tuple.get(), idx, pyopencv_from(gargs[idx]));
break;
case cv::GMetaArg::index_of<cv::GFrameDesc>():
util::throw_error(
std::logic_error("GFrame isn't supported for custom operation"));
break;
}
++idx;
}
}

static cv::GArg setup_py(cv::detail::PyObjectHolder setup,
const cv::GMetaArgs& meta,
const cv::GArgs& gargs)
{
PyGILState_STATE gstate;
gstate = PyGILState_Ensure();
Expand All @@ -756,42 +794,11 @@ static cv::GArg setup_py(cv::detail::PyObjectHolder out_meta, const cv::GMetaArg
// because PyObject already have ownership.
// In case exception decrement reference counter.
cv::detail::PyObjectHolder args(PyTuple_New(meta.size()), false);
size_t idx = 0;
for (auto&& m : meta)
{
switch (m.index())
{
case cv::GMetaArg::index_of<cv::GMatDesc>():
PyTuple_SetItem(args.get(), idx, pyopencv_from(cv::util::get<cv::GMatDesc>(m)));
break;
case cv::GMetaArg::index_of<cv::GScalarDesc>():
PyTuple_SetItem(args.get(), idx,
pyopencv_from(cv::util::get<cv::GScalarDesc>(m)));
break;
case cv::GMetaArg::index_of<cv::GArrayDesc>():
PyTuple_SetItem(args.get(), idx,
pyopencv_from(cv::util::get<cv::GArrayDesc>(m)));
break;
case cv::GMetaArg::index_of<cv::GOpaqueDesc>():
PyTuple_SetItem(args.get(), idx,
pyopencv_from(cv::util::get<cv::GOpaqueDesc>(m)));
break;
case cv::GMetaArg::index_of<cv::util::monostate>():
PyTuple_SetItem(args.get(), idx, pyopencv_from(gargs[idx]));
break;
case cv::GMetaArg::index_of<cv::GFrameDesc>():
util::throw_error(
std::logic_error("GFrame isn't supported for custom operation"));
break;
}
++idx;
}

// PyTuple_SetItem(args.get(), idx, pyopencv_from(gmarg));

// NB: Doesn't increase reference counter (false).
// In case PyObject_CallObject return NULL, do nothing in destructor.
cv::detail::PyObjectHolder result(PyObject_CallObject(out_meta.get(), args.get()), false);
unpackMetasToTuple(meta, gargs, args);
// NB: Take an onwership because this state is "Python" type so it will be wrapped as-is
// into cv::GArg and stored in GPythonBackend. Object without ownership can't
// be dealocated outside this function.
cv::detail::PyObjectHolder result(PyObject_CallObject(setup.get(), args.get()), true);

if (PyErr_Occurred())
{
Expand Down Expand Up @@ -841,8 +848,8 @@ static cv::GMetaArgs get_meta_args(PyObject* tuple)
}

static GMetaArgs run_py_meta(cv::detail::PyObjectHolder out_meta,
const cv::GMetaArgs &meta,
const cv::GArgs &gargs)
const cv::GMetaArgs &meta,
const cv::GArgs &gargs)
{
PyGILState_STATE gstate;
gstate = PyGILState_Ensure();
Expand All @@ -854,32 +861,7 @@ static GMetaArgs run_py_meta(cv::detail::PyObjectHolder out_meta,
// because PyObject already have ownership.
// In case exception decrement reference counter.
cv::detail::PyObjectHolder args(PyTuple_New(meta.size()), false);
size_t idx = 0;
for (auto&& m : meta)
{
switch (m.index())
{
case cv::GMetaArg::index_of<cv::GMatDesc>():
PyTuple_SetItem(args.get(), idx, pyopencv_from(cv::util::get<cv::GMatDesc>(m)));
break;
case cv::GMetaArg::index_of<cv::GScalarDesc>():
PyTuple_SetItem(args.get(), idx, pyopencv_from(cv::util::get<cv::GScalarDesc>(m)));
break;
case cv::GMetaArg::index_of<cv::GArrayDesc>():
PyTuple_SetItem(args.get(), idx, pyopencv_from(cv::util::get<cv::GArrayDesc>(m)));
break;
case cv::GMetaArg::index_of<cv::GOpaqueDesc>():
PyTuple_SetItem(args.get(), idx, pyopencv_from(cv::util::get<cv::GOpaqueDesc>(m)));
break;
case cv::GMetaArg::index_of<cv::util::monostate>():
PyTuple_SetItem(args.get(), idx, pyopencv_from(gargs[idx]));
break;
case cv::GMetaArg::index_of<cv::GFrameDesc>():
util::throw_error(std::logic_error("GFrame isn't supported for custom operation"));
break;
}
++idx;
}
unpackMetasToTuple(meta, gargs, args);
// NB: Doesn't increase reference counter (false).
// In case PyObject_CallObject return NULL, do nothing in destructor.
cv::detail::PyObjectHolder result(
Expand Down
46 changes: 46 additions & 0 deletions modules/gapi/misc/python/test/test_gapi_stateful_kernel.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,52 @@ def test_stateful_kernel_multiple_instances(self):
self.assertEqual(ref, acc1)


def test_stateful_throw_setup(self):
@cv.gapi.kernel(GStatefulCounter)
class GThrowStatefulCounterImpl:
"""Implementation for GStatefulCounter operation
that throw exception in setup method"""

@staticmethod
def setup(desc):
raise Exception('Throw from setup method')

@staticmethod
def run(value, state):
raise Exception('Unreachable')

g_in = cv.GOpaque.Int()
g_out = GStatefulCounter.on(g_in)
comp = cv.GComputation(cv.GIn(g_in), cv.GOut(g_out))
pkg = cv.gapi.kernels(GThrowStatefulCounterImpl)

with self.assertRaises(Exception): comp.apply(cv.gin(42),
args=cv.gapi.compile_args(pkg))


def test_stateful_reset(self):
g_in = cv.GOpaque.Int()
g_out = GStatefulCounter.on(g_in)
comp = cv.GComputation(cv.GIn(g_in), cv.GOut(g_out))
pkg = cv.gapi.kernels(GStatefulCounterImpl)

cc = comp.compileStreaming(args=cv.gapi.compile_args(pkg))

cc.setSource(cv.gin(1))
cc.start()
for i in range(1, 10):
_, actual = cc.pull()
self.assertEqual(i, actual)
cc.stop()

cc.setSource(cv.gin(2))
cc.start()
for i in range(2, 10, 2):
_, actual = cc.pull()
self.assertEqual(i, actual)
cc.stop()


except unittest.SkipTest as e:

message = str(e)
Expand Down
31 changes: 12 additions & 19 deletions modules/gapi/src/backends/python/gpythonbackend.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,10 @@
#include "api/gbackend_priv.hpp"
#include "backends/common/gbackend.hpp"

cv::gapi::python::GPythonKernel::GPythonKernel(cv::gapi::python::Impl run,
cv::gapi::python::Setup setup)
: m_run(run), m_setup(setup)
cv::gapi::python::GPythonKernel::GPythonKernel(cv::gapi::python::Impl runf,
cv::gapi::python::Setup setupf)
: run(runf), setup(setupf), is_stateful(setup != nullptr)
{
m_isStateful = (setup != nullptr);
}

cv::GRunArgs cv::gapi::python::GPythonKernel::operator()(const cv::gapi::python::GPythonContext& ctx)
{
return m_run(ctx);
}

cv::gapi::python::GPythonFunctor::GPythonFunctor(const char* id,
Expand Down Expand Up @@ -162,11 +156,11 @@ static void writeBack(cv::GRunArg& arg, cv::GRunArgP& out)

void GPythonExecutable::handleNewStream()
{
if (!m_kernel.m_isStateful)
if (!m_kernel.is_stateful)
return;

m_node_state = m_kernel.m_setup(cv::gimpl::GModel::collectInputMeta(m_gm, m_op),
m_gm.metadata(m_op).get<cv::gimpl::Op>().args);
m_node_state = m_kernel.setup(cv::gimpl::GModel::collectInputMeta(m_gm, m_op),
m_gm.metadata(m_op).get<cv::gimpl::Op>().args);
}

void GPythonExecutable::run(std::vector<InObj> &&input_objs,
Expand All @@ -181,16 +175,15 @@ void GPythonExecutable::run(std::vector<InObj> &&input_objs,
std::back_inserter(inputs),
std::bind(&packArg, std::ref(m_res), _1));

cv::gapi::python::GPythonContext ctx{inputs, m_in_metas, m_out_info, false};
cv::gapi::python::GPythonContext ctx{inputs, m_in_metas, m_out_info, /*state*/{}};

// For stateful kernel add state to its execution context
if (m_kernel.m_isStateful)
// NB: For stateful kernel add state to its execution context
if (m_kernel.is_stateful)
{
ctx.m_state = m_node_state;
ctx.m_isStateful = true;
ctx.m_state = cv::optional<cv::GArg>(m_node_state);
}

auto outs = m_kernel(ctx);
auto outs = m_kernel.run(ctx);

for (auto&& it : ade::util::zip(outs, output_objs))
{
Expand Down Expand Up @@ -249,7 +242,7 @@ GPythonExecutable::GPythonExecutable(const ade::Graph& g,
m_kernel = cag.metadata(m_op).get<PythonUnit>().kernel;

// If kernel is stateful then prepare storage for its state.
if (m_kernel.m_isStateful)
if (m_kernel.is_stateful)
{
m_node_state = cv::GArg{ };
}
Expand Down

0 comments on commit 7955469

Please sign in to comment.