| Message ID | 20210711170359.300-3-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi 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@ideasonboard.com> Ah well, I should have read this before answering to the previous version. If deemed better by the majority Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > --- > 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 >
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@ideasonboard.com> Reviewed-by: Kieran Bingham <kieran.bingham@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); > >
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@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@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 > >
Hi Laurent, On Mon, Jul 12, 2021 at 11:35:33AM +0300, Laurent Pinchart wrote: > 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@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 ? > Not really, but Kieran has a convincing argument about two ways to achieve the same being confusing, so my tag stands :) > > Reviewed-by: Jacopo Mondi <jacopo@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
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);
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@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(-)