[libcamera-devel] [PATCH v2 2/3] libcamera: Drop the LIBCAMERA_D_PTR macro in favour of the _d() function
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Jul 12 10:35:33 CEST 2021
Hi Jacopo,
On Mon, Jul 12, 2021 at 10:18:04AM +0200, Jacopo Mondi wrote:
> On Sun, Jul 11, 2021 at 08:03:58PM +0300, Laurent Pinchart wrote:
> > Now that all Extensible classes expose a _d() function that performs
> > appropriate casts, the LIBCAMERA_D_PTR brings no real additional value.
> > Replace it with direct calls to the _d() function.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> Ah well, I should have read this before answering to the previous
> version.
>
> If deemed better by the majority
That's what I'm not sure about :-) What's your opinion, do you like it
better or not ?
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
>
> > ---
> > include/libcamera/base/class.h | 4 ---
> > src/android/camera_buffer.h | 15 ++++-------
> > src/android/camera_hal_config.cpp | 3 +--
> > src/libcamera/base/class.cpp | 17 +++---------
> > src/libcamera/camera.cpp | 44 ++++++++++++-------------------
> > src/libcamera/camera_manager.cpp | 16 +++++------
> > 6 files changed, 34 insertions(+), 65 deletions(-)
> >
> > diff --git a/include/libcamera/base/class.h b/include/libcamera/base/class.h
> > index 8212c3d4a5ae..c2e1d3534ed1 100644
> > --- a/include/libcamera/base/class.h
> > +++ b/include/libcamera/base/class.h
> > @@ -49,16 +49,12 @@ public: \
> > friend class klass; \
> > using Public = klass;
> >
> > -#define LIBCAMERA_D_PTR() \
> > - _d();
> > -
> > #define LIBCAMERA_O_PTR() \
> > _o<Public>();
> >
> > #else
> > #define LIBCAMERA_DECLARE_PRIVATE()
> > #define LIBCAMERA_DECLARE_PUBLIC(klass)
> > -#define LIBCAMERA_D_PTR()
> > #define LIBCAMERA_O_PTR()
> > #endif
> >
> > diff --git a/src/android/camera_buffer.h b/src/android/camera_buffer.h
> > index 2617ff6b11a1..21373fa25bf8 100644
> > --- a/src/android/camera_buffer.h
> > +++ b/src/android/camera_buffer.h
> > @@ -40,27 +40,22 @@ CameraBuffer::~CameraBuffer() \
> > } \
> > bool CameraBuffer::isValid() const \
> > { \
> > - const Private *const d = LIBCAMERA_D_PTR(); \
> > - return d->isValid(); \
> > + return _d()->isValid(); \
> > } \
> > unsigned int CameraBuffer::numPlanes() const \
> > { \
> > - const Private *const d = LIBCAMERA_D_PTR(); \
> > - return d->numPlanes(); \
> > + return _d()->numPlanes(); \
> > } \
> > Span<const uint8_t> CameraBuffer::plane(unsigned int plane) const \
> > { \
> > - const Private *const d = LIBCAMERA_D_PTR(); \
> > - return const_cast<Private *>(d)->plane(plane); \
> > + return const_cast<Private *>(_d())->plane(plane); \
> > } \
> > Span<uint8_t> CameraBuffer::plane(unsigned int plane) \
> > { \
> > - Private *const d = LIBCAMERA_D_PTR(); \
> > - return d->plane(plane); \
> > + return _d()->plane(plane); \
> > } \
> > size_t CameraBuffer::jpegBufferSize(size_t maxJpegBufferSize) const \
> > { \
> > - const Private *const d = LIBCAMERA_D_PTR(); \
> > - return d->jpegBufferSize(maxJpegBufferSize); \
> > + return _d()->jpegBufferSize(maxJpegBufferSize); \
> > }
> > #endif /* __ANDROID_CAMERA_BUFFER_H__ */
> > diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp
> > index d84de4fd6f90..833cf4ba98a9 100644
> > --- a/src/android/camera_hal_config.cpp
> > +++ b/src/android/camera_hal_config.cpp
> > @@ -375,8 +375,7 @@ int CameraHalConfig::parseConfigurationFile()
> >
> > exists_ = true;
> >
> > - Private *const d = LIBCAMERA_D_PTR();
> > - int ret = d->parseConfigFile(fh, &cameras_);
> > + int ret = _d()->parseConfigFile(fh, &cameras_);
> > fclose(fh);
> > if (ret)
> > return -EINVAL;
> > diff --git a/src/libcamera/base/class.cpp b/src/libcamera/base/class.cpp
> > index 165beafc243d..26b4967726d6 100644
> > --- a/src/libcamera/base/class.cpp
> > +++ b/src/libcamera/base/class.cpp
> > @@ -94,23 +94,12 @@ namespace libcamera {
> > * name passed as the \a klass parameter.
> > */
> >
> > -/**
> > - * \def LIBCAMERA_D_PTR()
> > - * \brief Retrieve the private data pointer
> > - *
> > - * This macro can be used in any member function of a class that inherits,
> > - * directly or indirectly, from the Extensible class, to create a local
> > - * variable named 'd' that points to the class' private data instance.
> > - */
> > -
> > /**
> > * \def LIBCAMERA_O_PTR()
> > * \brief Retrieve the public instance corresponding to the private data
> > *
> > - * This macro is the counterpart of LIBCAMERA_D_PTR() for private data classes.
> > - * It can be used in any member function of the private data class to create a
> > - * local variable named 'o' that points to the public class instance
> > - * corresponding to the private data.
> > + * This macro is used in any member function of the private data class to access
> > + * the public class instance corresponding to the private data.
> > */
> >
> > /**
> > @@ -148,6 +137,8 @@ namespace libcamera {
> > * class need to be qualified with appropriate access specifiers. The
> > * PublicClass and Private classes always have full access to each other's
> > * protected and private members.
> > + *
> > + * The PublicClass exposes its Private data pointer through the _d() function.
> > */
> >
> > /**
> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > index 29f2d91d05d3..c8858e71d105 100644
> > --- a/src/libcamera/camera.cpp
> > +++ b/src/libcamera/camera.cpp
> > @@ -604,8 +604,7 @@ std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe,
> > */
> > const std::string &Camera::id() const
> > {
> > - const Private *const d = LIBCAMERA_D_PTR();
> > - return d->id_;
> > + return _d()->id_;
> > }
> >
> > /**
> > @@ -655,18 +654,16 @@ Camera::~Camera()
> > */
> > void Camera::disconnect()
> > {
> > - Private *const d = LIBCAMERA_D_PTR();
> > -
> > LOG(Camera, Debug) << "Disconnecting camera " << id();
> >
> > - d->disconnect();
> > + _d()->disconnect();
> > disconnected.emit(this);
> > }
> >
> > int Camera::exportFrameBuffers(Stream *stream,
> > std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > {
> > - Private *const d = LIBCAMERA_D_PTR();
> > + Private *const d = _d();
> >
> > int ret = d->isAccessAllowed(Private::CameraConfigured);
> > if (ret < 0)
> > @@ -709,7 +706,7 @@ int Camera::exportFrameBuffers(Stream *stream,
> > */
> > int Camera::acquire()
> > {
> > - Private *const d = LIBCAMERA_D_PTR();
> > + Private *const d = _d();
> >
> > /*
> > * No manual locking is required as PipelineHandler::lock() is
> > @@ -746,7 +743,7 @@ int Camera::acquire()
> > */
> > int Camera::release()
> > {
> > - Private *const d = LIBCAMERA_D_PTR();
> > + Private *const d = _d();
> >
> > int ret = d->isAccessAllowed(Private::CameraAvailable,
> > Private::CameraConfigured, true);
> > @@ -772,8 +769,7 @@ int Camera::release()
> > */
> > const ControlInfoMap &Camera::controls() const
> > {
> > - const Private *const d = LIBCAMERA_D_PTR();
> > - return d->pipe_->controls(this);
> > + return _d()->pipe_->controls(this);
> > }
> >
> > /**
> > @@ -786,8 +782,7 @@ const ControlInfoMap &Camera::controls() const
> > */
> > const ControlList &Camera::properties() const
> > {
> > - const Private *const d = LIBCAMERA_D_PTR();
> > - return d->pipe_->properties(this);
> > + return _d()->pipe_->properties(this);
> > }
> >
> > /**
> > @@ -803,8 +798,7 @@ const ControlList &Camera::properties() const
> > */
> > const std::set<Stream *> &Camera::streams() const
> > {
> > - const Private *const d = LIBCAMERA_D_PTR();
> > - return d->streams_;
> > + return _d()->streams_;
> > }
> >
> > /**
> > @@ -825,7 +819,7 @@ const std::set<Stream *> &Camera::streams() const
> > */
> > std::unique_ptr<CameraConfiguration> Camera::generateConfiguration(const StreamRoles &roles)
> > {
> > - Private *const d = LIBCAMERA_D_PTR();
> > + Private *const d = _d();
> >
> > int ret = d->isAccessAllowed(Private::CameraAvailable,
> > Private::CameraRunning);
> > @@ -886,7 +880,7 @@ std::unique_ptr<CameraConfiguration> Camera::generateConfiguration(const StreamR
> > */
> > int Camera::configure(CameraConfiguration *config)
> > {
> > - Private *const d = LIBCAMERA_D_PTR();
> > + Private *const d = _d();
> >
> > int ret = d->isAccessAllowed(Private::CameraAcquired,
> > Private::CameraConfigured);
> > @@ -958,10 +952,8 @@ int Camera::configure(CameraConfiguration *config)
> > */
> > std::unique_ptr<Request> Camera::createRequest(uint64_t cookie)
> > {
> > - Private *const d = LIBCAMERA_D_PTR();
> > -
> > - int ret = d->isAccessAllowed(Private::CameraConfigured,
> > - Private::CameraRunning);
> > + int ret = _d()->isAccessAllowed(Private::CameraConfigured,
> > + Private::CameraRunning);
> > if (ret < 0)
> > return nullptr;
> >
> > @@ -992,7 +984,7 @@ std::unique_ptr<Request> Camera::createRequest(uint64_t cookie)
> > */
> > int Camera::queueRequest(Request *request)
> > {
> > - Private *const d = LIBCAMERA_D_PTR();
> > + Private *const d = _d();
> >
> > int ret = d->isAccessAllowed(Private::CameraRunning);
> > if (ret < 0)
> > @@ -1044,7 +1036,7 @@ int Camera::queueRequest(Request *request)
> > */
> > int Camera::start(const ControlList *controls)
> > {
> > - Private *const d = LIBCAMERA_D_PTR();
> > + Private *const d = _d();
> >
> > int ret = d->isAccessAllowed(Private::CameraConfigured);
> > if (ret < 0)
> > @@ -1079,7 +1071,7 @@ int Camera::start(const ControlList *controls)
> > */
> > int Camera::stop()
> > {
> > - Private *const d = LIBCAMERA_D_PTR();
> > + Private *const d = _d();
> >
> > /*
> > * \todo Make calling stop() when not in 'Running' part of the state
> > @@ -1115,11 +1107,9 @@ int Camera::stop()
> > */
> > void Camera::requestComplete(Request *request)
> > {
> > - Private *const d = LIBCAMERA_D_PTR();
> > -
> > /* Disconnected cameras are still able to complete requests. */
> > - if (d->isAccessAllowed(Private::CameraStopping, Private::CameraRunning,
> > - true))
> > + if (_d()->isAccessAllowed(Private::CameraStopping, Private::CameraRunning,
> > + true))
> > LOG(Camera, Fatal) << "Trying to complete a request when stopped";
> >
> > requestCompleted.emit(request);
> > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> > index fc3bd88c737b..1c79308ad4b5 100644
> > --- a/src/libcamera/camera_manager.cpp
> > +++ b/src/libcamera/camera_manager.cpp
> > @@ -291,11 +291,9 @@ CameraManager::~CameraManager()
> > */
> > int CameraManager::start()
> > {
> > - Private *const d = LIBCAMERA_D_PTR();
> > -
> > LOG(Camera, Info) << "libcamera " << version_;
> >
> > - int ret = d->start();
> > + int ret = _d()->start();
> > if (ret)
> > LOG(Camera, Error) << "Failed to start camera manager: "
> > << strerror(-ret);
> > @@ -315,7 +313,7 @@ int CameraManager::start()
> > */
> > void CameraManager::stop()
> > {
> > - Private *const d = LIBCAMERA_D_PTR();
> > + Private *const d = _d();
> > d->exit();
> > d->wait();
> > }
> > @@ -333,7 +331,7 @@ void CameraManager::stop()
> > */
> > std::vector<std::shared_ptr<Camera>> CameraManager::cameras() const
> > {
> > - const Private *const d = LIBCAMERA_D_PTR();
> > + const Private *const d = _d();
> >
> > MutexLocker locker(d->mutex_);
> >
> > @@ -353,7 +351,7 @@ std::vector<std::shared_ptr<Camera>> CameraManager::cameras() const
> > */
> > std::shared_ptr<Camera> CameraManager::get(const std::string &id)
> > {
> > - Private *const d = LIBCAMERA_D_PTR();
> > + Private *const d = _d();
> >
> > MutexLocker locker(d->mutex_);
> >
> > @@ -383,7 +381,7 @@ std::shared_ptr<Camera> CameraManager::get(const std::string &id)
> > */
> > std::shared_ptr<Camera> CameraManager::get(dev_t devnum)
> > {
> > - Private *const d = LIBCAMERA_D_PTR();
> > + Private *const d = _d();
> >
> > MutexLocker locker(d->mutex_);
> >
> > @@ -439,7 +437,7 @@ std::shared_ptr<Camera> CameraManager::get(dev_t devnum)
> > void CameraManager::addCamera(std::shared_ptr<Camera> camera,
> > const std::vector<dev_t> &devnums)
> > {
> > - Private *const d = LIBCAMERA_D_PTR();
> > + Private *const d = _d();
> >
> > ASSERT(Thread::current() == d);
> >
> > @@ -459,7 +457,7 @@ void CameraManager::addCamera(std::shared_ptr<Camera> camera,
> > */
> > void CameraManager::removeCamera(std::shared_ptr<Camera> camera)
> > {
> > - Private *const d = LIBCAMERA_D_PTR();
> > + Private *const d = _d();
> >
> > ASSERT(Thread::current() == d);
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
> >
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list