Message ID | 20200921031057.3564-5-laurent.pinchart@ideasonboard.com |
---|---|
State | Accepted |
Headers | show |
Series |
|
Related | show |
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@ideasonboard.com> Reviewed-by: Niklas Söderlund <niklas.soderlund@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@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
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; }
Use the d-pointer infrastructure offered by the Extensible class to replace the custom implementation. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- include/libcamera/camera.h | 9 +-- src/libcamera/camera.cpp | 123 ++++++++++++++++++++++--------------- 2 files changed, 80 insertions(+), 52 deletions(-)