[libcamera-devel] [PATCH 10/14] libcamera: bound_method: Propagate method return value
Niklas Söderlund
niklas.soderlund at ragnatech.se
Tue Jan 7 20:57:06 CET 2020
Hi Laurent,
Thanks for your work.
On 2020-01-04 07:09:43 +0200, Laurent Pinchart wrote:
> Propagate the return value of the bound method all the way to the caller
> of activate(). The value is stored in the arguments pack for indirect
> invocation.
>
> As C++ doesn't allow instantiating a variable of type void, we need to
> specialize the template class BoundMethodPack for methods returning
> void. This in turn requires template specialization for the
> BoundMethodArgs class in order to store the return value in the pack,
> and for the BoundMemberMethod class to extract the return value from the
> pack.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
I think the penny dropped for me at the end after reading it 10+ times
;-)
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> include/libcamera/bound_method.h | 99 +++++++++++++++++++++++++++-----
> include/libcamera/object.h | 6 +-
> src/libcamera/bound_method.cpp | 23 ++++++--
> src/libcamera/object.cpp | 6 +-
> 4 files changed, 113 insertions(+), 21 deletions(-)
>
> diff --git a/include/libcamera/bound_method.h b/include/libcamera/bound_method.h
> index a74d2c503cdb..0a91f44a2054 100644
> --- a/include/libcamera/bound_method.h
> +++ b/include/libcamera/bound_method.h
> @@ -28,7 +28,7 @@ public:
> virtual ~BoundMethodPackBase() {}
> };
>
> -template<typename... Args>
> +template<typename R, typename... Args>
> class BoundMethodPack : public BoundMethodPackBase
> {
> public:
> @@ -37,6 +37,19 @@ public:
> {
> }
>
> + std::tuple<typename std::remove_reference<Args>::type...> args_;
> + R ret_;
> +};
> +
> +template<typename... Args>
> +class BoundMethodPack<void, Args...> : public BoundMethodPackBase
> +{
> +public:
> + BoundMethodPack(const Args &... args)
> + : args_(args...)
> + {
> + }
> +
> std::tuple<typename std::remove_reference<Args>::type...> args_;
> };
>
> @@ -77,7 +90,7 @@ protected:
> };
> #endif
>
> - void activatePack(std::shared_ptr<BoundMethodPackBase> pack,
> + bool activatePack(std::shared_ptr<BoundMethodPackBase> pack,
> bool deleteMethod);
>
> void *obj_;
> @@ -91,7 +104,34 @@ template<typename R, typename... Args>
> class BoundMethodArgs : public BoundMethodBase
> {
> public:
> - using PackType = BoundMethodPack<Args...>;
> + using PackType = BoundMethodPack<R, Args...>;
> +
> +private:
> + template<int... S>
> + void invokePack(BoundMethodPackBase *pack, BoundMethodBase::sequence<S...>)
> + {
> + PackType *args = static_cast<PackType *>(pack);
> + args->ret_ = invoke(std::get<S>(args->args_)...);
> + }
> +
> +public:
> + BoundMethodArgs(void *obj, Object *object, ConnectionType type)
> + : BoundMethodBase(obj, object, type) {}
> +
> + void invokePack(BoundMethodPackBase *pack) override
> + {
> + invokePack(pack, typename BoundMethodBase::generator<sizeof...(Args)>::type());
> + }
> +
> + virtual R activate(Args... args, bool deleteMethod = false) = 0;
> + virtual R invoke(Args... args) = 0;
> +};
> +
> +template<typename... Args>
> +class BoundMethodArgs<void, Args...> : public BoundMethodBase
> +{
> +public:
> + using PackType = BoundMethodPack<void, Args...>;
>
> private:
> template<int... S>
> @@ -129,15 +169,45 @@ public:
>
> bool match(R (T::*func)(Args...)) const { return func == func_; }
>
> + R activate(Args... args, bool deleteMethod = false) override
> + {
> + if (!this->object_)
> + return (static_cast<T *>(this->obj_)->*func_)(args...);
> +
> + auto pack = std::make_shared<PackType>(args...);
> + bool sync = BoundMethodBase::activatePack(pack, deleteMethod);
> + return sync ? pack->ret_ : R();
> + }
> +
> + R invoke(Args... args) override
> + {
> + return (static_cast<T *>(this->obj_)->*func_)(args...);
> + }
> +
> +private:
> + R (T::*func_)(Args...);
> +};
> +
> +template<typename T, typename... Args>
> +class BoundMemberMethod<T, void, Args...> : public BoundMethodArgs<void, Args...>
> +{
> +public:
> + using PackType = typename BoundMethodArgs<void *, Args...>::PackType;
> +
> + BoundMemberMethod(T *obj, Object *object, void (T::*func)(Args...),
> + ConnectionType type = ConnectionTypeAuto)
> + : BoundMethodArgs<void, Args...>(obj, object, type), func_(func)
> + {
> + }
> +
> + bool match(void (T::*func)(Args...)) const { return func == func_; }
> +
> void activate(Args... args, bool deleteMethod = false) override
> {
> - if (!this->object_) {
> - (static_cast<T *>(this->obj_)->*func_)(args...);
> - return;
> - }
> + if (!this->object_)
> + return (static_cast<T *>(this->obj_)->*func_)(args...);
>
> - std::shared_ptr<BoundMethodPackBase> pack =
> - std::make_shared<typename BoundMemberMethod<T, R, Args...>::PackType>(args...);
> + auto pack = std::make_shared<PackType>(args...);
> BoundMethodBase::activatePack(pack, deleteMethod);
> }
>
> @@ -147,7 +217,7 @@ public:
> }
>
> private:
> - R (T::*func_)(Args...);
> + void (T::*func_)(Args...);
> };
>
> template<typename R, typename... Args>
> @@ -162,12 +232,15 @@ public:
>
> bool match(R (*func)(Args...)) const { return func == func_; }
>
> - void activate(Args... args, bool deleteMethod = false) override
> + R activate(Args... args, bool deleteMethod = false) override
> {
> - (*func_)(args...);
> + return (*func_)(args...);
> }
>
> - void invoke(Args...) override {}
> + R invoke(Args...) override
> + {
> + return R();
> + }
>
> private:
> R (*func_)(Args...);
> diff --git a/include/libcamera/object.h b/include/libcamera/object.h
> index 586c2cf6fc70..04aa18394d55 100644
> --- a/include/libcamera/object.h
> +++ b/include/libcamera/object.h
> @@ -31,12 +31,12 @@ public:
>
> template<typename T, typename R, typename... FuncArgs, typename... Args,
> typename std::enable_if<std::is_base_of<Object, T>::value>::type * = nullptr>
> - void invokeMethod(R (T::*func)(FuncArgs...), ConnectionType type,
> - Args... args)
> + R invokeMethod(R (T::*func)(FuncArgs...), ConnectionType type,
> + Args... args)
> {
> T *obj = static_cast<T *>(this);
> auto *method = new BoundMemberMethod<T, R, FuncArgs...>(obj, this, func, type);
> - method->activate(args..., true);
> + return method->activate(args..., true);
> }
>
> Thread *thread() const { return thread_; }
> diff --git a/src/libcamera/bound_method.cpp b/src/libcamera/bound_method.cpp
> index 82339b7e9c95..8e95c7eec92c 100644
> --- a/src/libcamera/bound_method.cpp
> +++ b/src/libcamera/bound_method.cpp
> @@ -48,7 +48,22 @@ namespace libcamera {
> * deadlock will occur.
> */
>
> -void BoundMethodBase::activatePack(std::shared_ptr<BoundMethodPackBase> pack,
> +/**
> + * \brief Invoke the bound method with packed arguments
> + * \param[in] pack Packed arguments
> + * \param[in] deleteMethod True to delete \a this bound method instance when
> + * method invocation completes
> + *
> + * The bound method stores its return value, if any, in the arguments \a pack.
> + * For direct and blocking invocations, this is performed synchronously, and
> + * the return value contained in the pack may be used. For queued invocations,
> + * the return value is stored at an undefined point of time and shall thus not
> + * be used by the caller.
> + *
> + * \return True if the return value contained in the \a pack may be used by the
> + * caller, false otherwise
> + */
> +bool BoundMethodBase::activatePack(std::shared_ptr<BoundMethodPackBase> pack,
> bool deleteMethod)
> {
> ConnectionType type = connectionType_;
> @@ -65,13 +80,13 @@ void BoundMethodBase::activatePack(std::shared_ptr<BoundMethodPackBase> pack,
> invokePack(pack.get());
> if (deleteMethod)
> delete this;
> - break;
> + return true;
>
> case ConnectionTypeQueued: {
> std::unique_ptr<Message> msg =
> utils::make_unique<InvokeMessage>(this, pack, nullptr, deleteMethod);
> object_->postMessage(std::move(msg));
> - break;
> + return false;
> }
>
> case ConnectionTypeBlocking: {
> @@ -82,7 +97,7 @@ void BoundMethodBase::activatePack(std::shared_ptr<BoundMethodPackBase> pack,
> object_->postMessage(std::move(msg));
>
> semaphore.acquire();
> - break;
> + return true;
> }
> }
> }
> diff --git a/src/libcamera/object.cpp b/src/libcamera/object.cpp
> index e76faf48b8ed..21aad5652b38 100644
> --- a/src/libcamera/object.cpp
> +++ b/src/libcamera/object.cpp
> @@ -143,7 +143,7 @@ void Object::message(Message *msg)
> }
>
> /**
> - * \fn void Object::invokeMethod()
> + * \fn R Object::invokeMethod()
> * \brief Invoke a method asynchronously on an Object instance
> * \param[in] func The object method to invoke
> * \param[in] type Connection type for method invocation
> @@ -156,6 +156,10 @@ void Object::message(Message *msg)
> * Arguments \a args passed by value or reference are copied, while pointers
> * are passed untouched. The caller shall ensure that any pointer argument
> * remains valid until the method is invoked.
> + *
> + * \return For connection types ConnectionTypeDirect and
> + * ConnectionTypeBlocking, return the return value of the invoked method. For
> + * connection type ConnectionTypeQueued, return a default-constructed R value.
> */
>
> /**
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list