[libcamera-devel] [PATCH 4/4] libcamera: camera: Inherit from Extensible
Niklas Söderlund
niklas.soderlund at ragnatech.se
Mon Sep 21 11:53:58 CEST 2020
Hi Laurent,
Thanks for your work.
On 2020-09-21 06:10:57 +0300, Laurent Pinchart wrote:
> Use the d-pointer infrastructure offered by the Extensible class to
> replace the custom implementation.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> include/libcamera/camera.h | 9 +--
> src/libcamera/camera.cpp | 123 ++++++++++++++++++++++---------------
> 2 files changed, 80 insertions(+), 52 deletions(-)
>
> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> index 272c12c3c473..77a4402cc766 100644
> --- a/include/libcamera/camera.h
> +++ b/include/libcamera/camera.h
> @@ -13,6 +13,7 @@
> #include <string>
>
> #include <libcamera/controls.h>
> +#include <libcamera/extensible.h>
> #include <libcamera/object.h>
> #include <libcamera/request.h>
> #include <libcamera/signal.h>
> @@ -67,8 +68,11 @@ protected:
> std::vector<StreamConfiguration> config_;
> };
>
> -class Camera final : public Object, public std::enable_shared_from_this<Camera>
> +class Camera final : public Object, public std::enable_shared_from_this<Camera>,
> + public Extensible
> {
> + LIBCAMERA_DECLARE_PRIVATE(Camera)
> +
> public:
> static std::shared_ptr<Camera> create(PipelineHandler *pipe,
> const std::string &id,
> @@ -104,9 +108,6 @@ private:
> const std::set<Stream *> &streams);
> ~Camera();
>
> - class Private;
> - std::unique_ptr<Private> p_;
> -
> friend class PipelineHandler;
> void disconnect();
> void requestComplete(Request *request);
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index ae16a64a3b44..4141d63e606c 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -256,8 +256,10 @@ std::size_t CameraConfiguration::size() const
> * \brief The vector of stream configurations
> */
>
> -class Camera::Private
> +class Camera::Private : public Extensible::Private
> {
> + LIBCAMERA_DECLARE_PUBLIC(Camera)
> +
> public:
> enum State {
> CameraAvailable,
> @@ -266,7 +268,7 @@ public:
> CameraRunning,
> };
>
> - Private(PipelineHandler *pipe, const std::string &id,
> + Private(Camera *camera, PipelineHandler *pipe, const std::string &id,
> const std::set<Stream *> &streams);
> ~Private();
>
> @@ -287,10 +289,11 @@ private:
> std::atomic<State> state_;
> };
>
> -Camera::Private::Private(PipelineHandler *pipe, const std::string &id,
> +Camera::Private::Private(Camera *camera, PipelineHandler *pipe,
> + const std::string &id,
> const std::set<Stream *> &streams)
> - : pipe_(pipe->shared_from_this()), id_(id), streams_(streams),
> - disconnected_(false), state_(CameraAvailable)
> + : Extensible::Private(camera), pipe_(pipe->shared_from_this()), id_(id),
> + streams_(streams), disconnected_(false), state_(CameraAvailable)
> {
> }
>
> @@ -505,7 +508,8 @@ std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe,
> */
> const std::string &Camera::id() const
> {
> - return p_->id_;
> + LIBCAMERA_D_PTR(Camera);
> + return d->id_;
> }
>
> /**
> @@ -533,7 +537,7 @@ const std::string &Camera::id() const
>
> Camera::Camera(PipelineHandler *pipe, const std::string &id,
> const std::set<Stream *> &streams)
> - : p_(new Private(pipe, id, streams))
> + : Extensible(new Private(this, pipe, id, streams))
> {
> }
>
> @@ -555,28 +559,32 @@ Camera::~Camera()
> */
> void Camera::disconnect()
> {
> + LIBCAMERA_D_PTR(Camera);
> +
> LOG(Camera, Debug) << "Disconnecting camera " << id();
>
> - p_->disconnect();
> + d->disconnect();
> disconnected.emit(this);
> }
>
> int Camera::exportFrameBuffers(Stream *stream,
> std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> {
> - int ret = p_->isAccessAllowed(Private::CameraConfigured);
> + LIBCAMERA_D_PTR(Camera);
> +
> + int ret = d->isAccessAllowed(Private::CameraConfigured);
> if (ret < 0)
> return ret;
>
> if (streams().find(stream) == streams().end())
> return -EINVAL;
>
> - if (p_->activeStreams_.find(stream) == p_->activeStreams_.end())
> + if (d->activeStreams_.find(stream) == d->activeStreams_.end())
> return -EINVAL;
>
> - return p_->pipe_->invokeMethod(&PipelineHandler::exportFrameBuffers,
> - ConnectionTypeBlocking, this, stream,
> - buffers);
> + return d->pipe_->invokeMethod(&PipelineHandler::exportFrameBuffers,
> + ConnectionTypeBlocking, this, stream,
> + buffers);
> }
>
> /**
> @@ -605,21 +613,23 @@ int Camera::exportFrameBuffers(Stream *stream,
> */
> int Camera::acquire()
> {
> + LIBCAMERA_D_PTR(Camera);
> +
> /*
> * No manual locking is required as PipelineHandler::lock() is
> * thread-safe.
> */
> - int ret = p_->isAccessAllowed(Private::CameraAvailable);
> + int ret = d->isAccessAllowed(Private::CameraAvailable);
> if (ret < 0)
> return ret == -EACCES ? -EBUSY : ret;
>
> - if (!p_->pipe_->lock()) {
> + if (!d->pipe_->lock()) {
> LOG(Camera, Info)
> << "Pipeline handler in use by another process";
> return -EBUSY;
> }
>
> - p_->setState(Private::CameraAcquired);
> + d->setState(Private::CameraAcquired);
>
> return 0;
> }
> @@ -640,14 +650,16 @@ int Camera::acquire()
> */
> int Camera::release()
> {
> - int ret = p_->isAccessAllowed(Private::CameraAvailable,
> - Private::CameraConfigured, true);
> + LIBCAMERA_D_PTR(Camera);
> +
> + int ret = d->isAccessAllowed(Private::CameraAvailable,
> + Private::CameraConfigured, true);
> if (ret < 0)
> return ret == -EACCES ? -EBUSY : ret;
>
> - p_->pipe_->unlock();
> + d->pipe_->unlock();
>
> - p_->setState(Private::CameraAvailable);
> + d->setState(Private::CameraAvailable);
>
> return 0;
> }
> @@ -664,7 +676,8 @@ int Camera::release()
> */
> const ControlInfoMap &Camera::controls() const
> {
> - return p_->pipe_->controls(this);
> + LIBCAMERA_D_PTR(Camera);
> + return d->pipe_->controls(this);
> }
>
> /**
> @@ -677,7 +690,8 @@ const ControlInfoMap &Camera::controls() const
> */
> const ControlList &Camera::properties() const
> {
> - return p_->pipe_->properties(this);
> + LIBCAMERA_D_PTR(Camera);
> + return d->pipe_->properties(this);
> }
>
> /**
> @@ -693,7 +707,8 @@ const ControlList &Camera::properties() const
> */
> const std::set<Stream *> &Camera::streams() const
> {
> - return p_->streams_;
> + LIBCAMERA_D_PTR(Camera);
> + return d->streams_;
> }
>
> /**
> @@ -714,15 +729,17 @@ const std::set<Stream *> &Camera::streams() const
> */
> std::unique_ptr<CameraConfiguration> Camera::generateConfiguration(const StreamRoles &roles)
> {
> - int ret = p_->isAccessAllowed(Private::CameraAvailable,
> - Private::CameraRunning);
> + LIBCAMERA_D_PTR(Camera);
> +
> + int ret = d->isAccessAllowed(Private::CameraAvailable,
> + Private::CameraRunning);
> if (ret < 0)
> return nullptr;
>
> if (roles.size() > streams().size())
> return nullptr;
>
> - CameraConfiguration *config = p_->pipe_->generateConfiguration(this, roles);
> + CameraConfiguration *config = d->pipe_->generateConfiguration(this, roles);
> if (!config) {
> LOG(Camera, Debug)
> << "Pipeline handler failed to generate configuration";
> @@ -773,8 +790,10 @@ std::unique_ptr<CameraConfiguration> Camera::generateConfiguration(const StreamR
> */
> int Camera::configure(CameraConfiguration *config)
> {
> - int ret = p_->isAccessAllowed(Private::CameraAcquired,
> - Private::CameraConfigured);
> + LIBCAMERA_D_PTR(Camera);
> +
> + int ret = d->isAccessAllowed(Private::CameraAcquired,
> + Private::CameraConfigured);
> if (ret < 0)
> return ret;
>
> @@ -796,26 +815,26 @@ int Camera::configure(CameraConfiguration *config)
>
> LOG(Camera, Info) << msg.str();
>
> - ret = p_->pipe_->invokeMethod(&PipelineHandler::configure,
> - ConnectionTypeBlocking, this, config);
> + ret = d->pipe_->invokeMethod(&PipelineHandler::configure,
> + ConnectionTypeBlocking, this, config);
> if (ret)
> return ret;
>
> - p_->activeStreams_.clear();
> + d->activeStreams_.clear();
> for (const StreamConfiguration &cfg : *config) {
> Stream *stream = cfg.stream();
> if (!stream) {
> LOG(Camera, Fatal)
> << "Pipeline handler failed to update stream configuration";
> - p_->activeStreams_.clear();
> + d->activeStreams_.clear();
> return -EINVAL;
> }
>
> stream->configuration_ = cfg;
> - p_->activeStreams_.insert(stream);
> + d->activeStreams_.insert(stream);
> }
>
> - p_->setState(Private::CameraConfigured);
> + d->setState(Private::CameraConfigured);
>
> return 0;
> }
> @@ -842,8 +861,10 @@ int Camera::configure(CameraConfiguration *config)
> */
> Request *Camera::createRequest(uint64_t cookie)
> {
> - int ret = p_->isAccessAllowed(Private::CameraConfigured,
> - Private::CameraRunning);
> + LIBCAMERA_D_PTR(Camera);
> +
> + int ret = d->isAccessAllowed(Private::CameraConfigured,
> + Private::CameraRunning);
> if (ret < 0)
> return nullptr;
>
> @@ -877,7 +898,9 @@ Request *Camera::createRequest(uint64_t cookie)
> */
> int Camera::queueRequest(Request *request)
> {
> - int ret = p_->isAccessAllowed(Private::CameraRunning);
> + LIBCAMERA_D_PTR(Camera);
> +
> + int ret = d->isAccessAllowed(Private::CameraRunning);
> if (ret < 0)
> return ret;
>
> @@ -895,14 +918,14 @@ int Camera::queueRequest(Request *request)
> for (auto const &it : request->buffers()) {
> const Stream *stream = it.first;
>
> - if (p_->activeStreams_.find(stream) == p_->activeStreams_.end()) {
> + if (d->activeStreams_.find(stream) == d->activeStreams_.end()) {
> LOG(Camera, Error) << "Invalid request";
> return -EINVAL;
> }
> }
>
> - return p_->pipe_->invokeMethod(&PipelineHandler::queueRequest,
> - ConnectionTypeQueued, this, request);
> + return d->pipe_->invokeMethod(&PipelineHandler::queueRequest,
> + ConnectionTypeQueued, this, request);
> }
>
> /**
> @@ -923,18 +946,20 @@ int Camera::queueRequest(Request *request)
> */
> int Camera::start()
> {
> - int ret = p_->isAccessAllowed(Private::CameraConfigured);
> + LIBCAMERA_D_PTR(Camera);
> +
> + int ret = d->isAccessAllowed(Private::CameraConfigured);
> if (ret < 0)
> return ret;
>
> LOG(Camera, Debug) << "Starting capture";
>
> - ret = p_->pipe_->invokeMethod(&PipelineHandler::start,
> - ConnectionTypeBlocking, this);
> + ret = d->pipe_->invokeMethod(&PipelineHandler::start,
> + ConnectionTypeBlocking, this);
> if (ret)
> return ret;
>
> - p_->setState(Private::CameraRunning);
> + d->setState(Private::CameraRunning);
>
> return 0;
> }
> @@ -955,16 +980,18 @@ int Camera::start()
> */
> int Camera::stop()
> {
> - int ret = p_->isAccessAllowed(Private::CameraRunning);
> + LIBCAMERA_D_PTR(Camera);
> +
> + int ret = d->isAccessAllowed(Private::CameraRunning);
> if (ret < 0)
> return ret;
>
> LOG(Camera, Debug) << "Stopping capture";
>
> - p_->setState(Private::CameraConfigured);
> + d->setState(Private::CameraConfigured);
>
> - p_->pipe_->invokeMethod(&PipelineHandler::stop, ConnectionTypeBlocking,
> - this);
> + d->pipe_->invokeMethod(&PipelineHandler::stop, ConnectionTypeBlocking,
> + this);
>
> return 0;
> }
> --
> 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