Message ID | 20210301150111.61791-7-jacopo@jmondi.org |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
Hi Jacopo, Thank you for the patch. On Mon, Mar 01, 2021 at 04:01:07PM +0100, Jacopo Mondi wrote: > In order to prepare to support more memory backends, make the > CameraBuffer class implement the PIMPL (pointer-to-implementation) > pattern by inheriting from the libcamera::Extensible class. > > Temporary maintain libcamera::MappedBuffer as the CameraBuffer base > class to maintain compatibility of the CameraStream::process() interface > that requires a MappedBuffer * as second argument and will be converted > to use a CameraBuffer in the next patch. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/android/camera_buffer.h | 14 ++++- > src/android/mm/generic_camera_buffer.cpp | 74 +++++++++++++++++++++++- > 2 files changed, 86 insertions(+), 2 deletions(-) > > diff --git a/src/android/camera_buffer.h b/src/android/camera_buffer.h > index 0590cd84652b..ca4f4527c690 100644 > --- a/src/android/camera_buffer.h > +++ b/src/android/camera_buffer.h > @@ -9,13 +9,25 @@ > > #include <hardware/camera3.h> > > +#include <libcamera/class.h> > #include <libcamera/internal/buffer.h> > +#include <libcamera/span.h> > > -class CameraBuffer : public libcamera::MappedBuffer > +class CameraBuffer final : public libcamera::Extensible, > + public libcamera::MappedBuffer > { > + LIBCAMERA_DECLARE_PRIVATE(CameraBuffer) > + > public: > CameraBuffer(buffer_handle_t camera3Buffer, int flags); > ~CameraBuffer(); > + > + bool isValid() const; > + > + unsigned int numPlanes() const; > + > + libcamera::Span<const uint8_t> plane(unsigned int plane) const; > + libcamera::Span<uint8_t> plane(unsigned int plane); > }; > > #endif /* __ANDROID_CAMERA_BUFFER_H__ */ > diff --git a/src/android/mm/generic_camera_buffer.cpp b/src/android/mm/generic_camera_buffer.cpp > index d1407f9cddf4..eedf16b76542 100644 > --- a/src/android/mm/generic_camera_buffer.cpp > +++ b/src/android/mm/generic_camera_buffer.cpp > @@ -13,7 +13,25 @@ using namespace libcamera; > > LOG_DECLARE_CATEGORY(HAL) > > -CameraBuffer::CameraBuffer(buffer_handle_t camera3Buffer, int flags) > +class CameraBuffer::Private : public Extensible::Private, > + public libcamera::MappedBuffer > +{ > + LIBCAMERA_DECLARE_PUBLIC(CameraBuffer) > + > +public: > + Private(CameraBuffer *cameraBuffer, > + buffer_handle_t camera3Buffer, int flags); > + ~Private(); > + > + unsigned int numPlanes() const; > + > + Span<const uint8_t> plane(unsigned int plane) const; > + Span<uint8_t> plane(unsigned int plane); > +}; > + > +CameraBuffer::Private::Private(CameraBuffer *cameraBuffer, > + buffer_handle_t camera3Buffer, int flags) > + : Extensible::Private(cameraBuffer) > { > maps_.reserve(camera3Buffer->numFds); > error_ = 0; > @@ -42,6 +60,60 @@ CameraBuffer::CameraBuffer(buffer_handle_t camera3Buffer, int flags) > } > } > > +CameraBuffer::Private::~Private() > +{ > +} > + > +unsigned int CameraBuffer::Private::numPlanes() const > +{ > + return maps_.size(); > +} > + > +Span<const uint8_t> CameraBuffer::Private::plane(unsigned int plane) const > +{ > + if (plane >= maps_.size()) > + return {}; > + > + return maps_[plane]; > +} > + > +Span<uint8_t> CameraBuffer::Private::plane(unsigned int plane) > +{ > + if (plane >= maps_.size()) > + return {}; > + > + return maps_[plane]; > +} > + > +CameraBuffer::CameraBuffer(buffer_handle_t camera3Buffer, int flags) > + : Extensible(new Private(this, camera3Buffer, flags)) > +{ > +} > + > CameraBuffer::~CameraBuffer() > { > } > + > +bool CameraBuffer::isValid() const > +{ > + const Private *const d = LIBCAMERA_D_PTR(); > + return d->isValid(); > +} > + > +unsigned int CameraBuffer::numPlanes() const > +{ > + const Private *const d = LIBCAMERA_D_PTR(); > + return d->numPlanes(); > +} > + > +Span<const uint8_t> CameraBuffer::plane(unsigned int plane) const > +{ > + const Private *const d = LIBCAMERA_D_PTR(); > + return d->plane(plane); How about implementing this as return const_cast<Private *>(d)->plane(plane); to avoid two plane() function in the private class ? You could then drop the const version of Private::plane() (it would be possible to do it the other way around, but conceptually, keeping the non-const version is better I think as we return a pointer to non-const memory). Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > +} > + > +Span<uint8_t> CameraBuffer::plane(unsigned int plane) > +{ > + Private *const d = LIBCAMERA_D_PTR(); > + return d->plane(plane); > +}
diff --git a/src/android/camera_buffer.h b/src/android/camera_buffer.h index 0590cd84652b..ca4f4527c690 100644 --- a/src/android/camera_buffer.h +++ b/src/android/camera_buffer.h @@ -9,13 +9,25 @@ #include <hardware/camera3.h> +#include <libcamera/class.h> #include <libcamera/internal/buffer.h> +#include <libcamera/span.h> -class CameraBuffer : public libcamera::MappedBuffer +class CameraBuffer final : public libcamera::Extensible, + public libcamera::MappedBuffer { + LIBCAMERA_DECLARE_PRIVATE(CameraBuffer) + public: CameraBuffer(buffer_handle_t camera3Buffer, int flags); ~CameraBuffer(); + + bool isValid() const; + + unsigned int numPlanes() const; + + libcamera::Span<const uint8_t> plane(unsigned int plane) const; + libcamera::Span<uint8_t> plane(unsigned int plane); }; #endif /* __ANDROID_CAMERA_BUFFER_H__ */ diff --git a/src/android/mm/generic_camera_buffer.cpp b/src/android/mm/generic_camera_buffer.cpp index d1407f9cddf4..eedf16b76542 100644 --- a/src/android/mm/generic_camera_buffer.cpp +++ b/src/android/mm/generic_camera_buffer.cpp @@ -13,7 +13,25 @@ using namespace libcamera; LOG_DECLARE_CATEGORY(HAL) -CameraBuffer::CameraBuffer(buffer_handle_t camera3Buffer, int flags) +class CameraBuffer::Private : public Extensible::Private, + public libcamera::MappedBuffer +{ + LIBCAMERA_DECLARE_PUBLIC(CameraBuffer) + +public: + Private(CameraBuffer *cameraBuffer, + buffer_handle_t camera3Buffer, int flags); + ~Private(); + + unsigned int numPlanes() const; + + Span<const uint8_t> plane(unsigned int plane) const; + Span<uint8_t> plane(unsigned int plane); +}; + +CameraBuffer::Private::Private(CameraBuffer *cameraBuffer, + buffer_handle_t camera3Buffer, int flags) + : Extensible::Private(cameraBuffer) { maps_.reserve(camera3Buffer->numFds); error_ = 0; @@ -42,6 +60,60 @@ CameraBuffer::CameraBuffer(buffer_handle_t camera3Buffer, int flags) } } +CameraBuffer::Private::~Private() +{ +} + +unsigned int CameraBuffer::Private::numPlanes() const +{ + return maps_.size(); +} + +Span<const uint8_t> CameraBuffer::Private::plane(unsigned int plane) const +{ + if (plane >= maps_.size()) + return {}; + + return maps_[plane]; +} + +Span<uint8_t> CameraBuffer::Private::plane(unsigned int plane) +{ + if (plane >= maps_.size()) + return {}; + + return maps_[plane]; +} + +CameraBuffer::CameraBuffer(buffer_handle_t camera3Buffer, int flags) + : Extensible(new Private(this, camera3Buffer, flags)) +{ +} + CameraBuffer::~CameraBuffer() { } + +bool CameraBuffer::isValid() const +{ + const Private *const d = LIBCAMERA_D_PTR(); + return d->isValid(); +} + +unsigned int CameraBuffer::numPlanes() const +{ + const Private *const d = LIBCAMERA_D_PTR(); + return d->numPlanes(); +} + +Span<const uint8_t> CameraBuffer::plane(unsigned int plane) const +{ + const Private *const d = LIBCAMERA_D_PTR(); + return d->plane(plane); +} + +Span<uint8_t> CameraBuffer::plane(unsigned int plane) +{ + Private *const d = LIBCAMERA_D_PTR(); + return d->plane(plane); +}
In order to prepare to support more memory backends, make the CameraBuffer class implement the PIMPL (pointer-to-implementation) pattern by inheriting from the libcamera::Extensible class. Temporary maintain libcamera::MappedBuffer as the CameraBuffer base class to maintain compatibility of the CameraStream::process() interface that requires a MappedBuffer * as second argument and will be converted to use a CameraBuffer in the next patch. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/android/camera_buffer.h | 14 ++++- src/android/mm/generic_camera_buffer.cpp | 74 +++++++++++++++++++++++- 2 files changed, 86 insertions(+), 2 deletions(-)