[libcamera-devel] [PATCH v2 2/3] libcamera: Drop the LIBCAMERA_D_PTR macro in favour of the _d() function
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Jul 12 10:30:50 CEST 2021
Hi Laurent,
On 11/07/2021 18:03, 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>
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> 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);
>
>
More information about the libcamera-devel
mailing list