[libcamera-devel] [RFC PATCH 3/3] android: camera_buffer: Add stride/offset/size function
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Aug 23 23:56:35 CEST 2021
Hi Hiro,
Thank you for the patch.
On Mon, Aug 23, 2021 at 09:43:21PM +0900, Hirokazu Honda wrote:
> This adds getter functions of stride, offset and size to CameraBuffer
> interface.
>
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> ---
> src/android/camera_buffer.h | 16 +++++
> src/android/mm/cros_camera_buffer.cpp | 19 ++++++
> src/android/mm/generic_camera_buffer.cpp | 78 ++++++++++++++++++------
> 3 files changed, 94 insertions(+), 19 deletions(-)
>
> diff --git a/src/android/camera_buffer.h b/src/android/camera_buffer.h
> index 87df2570..226a8f5c 100644
> --- a/src/android/camera_buffer.h
> +++ b/src/android/camera_buffer.h
> @@ -31,6 +31,10 @@ public:
> libcamera::Span<const uint8_t> plane(unsigned int plane) const;
> libcamera::Span<uint8_t> plane(unsigned int plane);
>
> + unsigned int stride(unsigned int plane) const;
> + unsigned int offset(unsigned int plane) const;
> + unsigned int size(unsigned int plane) const;
> +
> size_t jpegBufferSize(size_t maxJpegBufferSize) const;
> };
>
> @@ -62,6 +66,18 @@ Span<uint8_t> CameraBuffer::plane(unsigned int plane) \
> { \
> return _d()->plane(plane); \
> } \
> +unsigned int CameraBuffer::stride(unsigned int plane) const \
> +{ \
> + return _d()->stride(plane); \
> +} \
> +unsigned int CameraBuffer::offset(unsigned int plane) const \
> +{ \
> + return _d()->offset(plane); \
> +} \
> +unsigned int CameraBuffer::size(unsigned int plane) const \
> +{ \
> + return _d()->size(plane); \
> +} \
> size_t CameraBuffer::jpegBufferSize(size_t maxJpegBufferSize) const \
> { \
> return _d()->jpegBufferSize(maxJpegBufferSize); \
> diff --git a/src/android/mm/cros_camera_buffer.cpp b/src/android/mm/cros_camera_buffer.cpp
> index 85ef6480..4ba24f79 100644
> --- a/src/android/mm/cros_camera_buffer.cpp
> +++ b/src/android/mm/cros_camera_buffer.cpp
> @@ -31,6 +31,10 @@ public:
>
> Span<uint8_t> plane(unsigned int plane);
>
> + unsigned int stride(unsigned int plane) const;
> + unsigned int offset(unsigned int plane) const;
> + unsigned int size(unsigned int plane) const;
> +
> size_t jpegBufferSize(size_t maxJpegBufferSize) const;
>
> private:
> @@ -112,6 +116,21 @@ Span<uint8_t> CameraBuffer::Private::plane(unsigned int plane)
> bufferManager_->GetPlaneSize(handle_, plane) };
> }
>
> +unsigned int CameraBuffer::Private::stride(unsigned int plane) const
> +{
> + return cros::CameraBufferManager::GetPlaneStride(handle_, plane);
> +}
> +
> +unsigned int CameraBuffer::Private::offset(unsigned int plane) const
> +{
> + return cros::CameraBufferManager::GetPlaneOffset(handle_, plane);
> +}
> +
> +unsigned int CameraBuffer::Private::size(unsigned int plane) const
> +{
> + return cros::CameraBufferManager::GetPlaneSize(handle_, plane);
> +}
> +
> size_t CameraBuffer::Private::jpegBufferSize([[maybe_unused]] size_t maxJpegBufferSize) const
> {
> return bufferManager_->GetPlaneSize(handle_, 0);
> diff --git a/src/android/mm/generic_camera_buffer.cpp b/src/android/mm/generic_camera_buffer.cpp
> index 6c1e4611..b296b3e3 100644
> --- a/src/android/mm/generic_camera_buffer.cpp
> +++ b/src/android/mm/generic_camera_buffer.cpp
> @@ -34,15 +34,24 @@ public:
>
> Span<uint8_t> plane(unsigned int plane);
>
> + unsigned int stride(unsigned int plane) const;
> + unsigned int offset(unsigned int plane) const;
> + unsigned int size(unsigned int plane) const;
> +
> size_t jpegBufferSize(size_t maxJpegBufferSize) const;
>
> private:
> + struct PlaneInfo {
> + unsigned int stride;
> + unsigned int offset;
> + unsigned int size;
> + };
> +
> bool Map();
>
> int fd_;
> int flags_;
> - libcamera::Size size_;
> - libcamera::PixelFormatInfo info_;
> + std::vector<PlaneInfo> planeInfo_;
> /* \todo remove planes_ is added to MappedBuffer. */
> std::vector<Span<uint8_t>> planes_;
> };
> @@ -51,7 +60,7 @@ CameraBuffer::Private::Private([[maybe_unused]] CameraBuffer *cameraBuffer,
> buffer_handle_t camera3Buffer,
> libcamera::PixelFormat pixelFormat,
> const libcamera::Size &size, int flags)
> - : fd_(-1), flags_(flags), size_(size)
> + : fd_(-1), flags_(flags)
> {
> error_ = 0;
>
> @@ -68,13 +77,29 @@ CameraBuffer::Private::Private([[maybe_unused]] CameraBuffer *cameraBuffer,
> return;
> }
>
> - info_ = libcamera::PixelFormatInfo::info(pixelFormat);
> - if (!info_.isValid()) {
> + const auto &info = libcamera::PixelFormatInfo::info(pixelFormat);
> + if (!info.isValid()) {
> error_ = EINVAL;
> LOG(HAL, Error) << "Invalid pixel format: "
> << pixelFormat.toString();
> return;
> }
> +
> + const unsigned int numPlanes = info.numPlanes();
> + planeInfo_.resize(numPlanes);
> + unsigned int offset = 0;
> + for (unsigned int i = 0; i < numPlanes; ++i) {
> + const unsigned int vertSubSample = info.planes[i].verticalSubSampling;
> + const unsigned int stride = info.stride(size.width, i, 1u);
> + const unsigned int planeSize =
> + stride * ((size.height + vertSubSample - 1) / vertSubSample);
> +
> + planeInfo_[i].stride = stride;
> + planeInfo_[i].offset = offset;
> + planeInfo_[i].size = planeSize;
> +
> + offset += planeSize;
> + }
Ah, I see this code moving back to the constructor, as I mentioned in
the review of 2/3 :-) If it doesn't make the patches too ugly, it would
be nice to keep it in the constructor in 2/3 instead of moving it back.
> }
>
> CameraBuffer::Private::~Private()
> @@ -83,7 +108,7 @@ CameraBuffer::Private::~Private()
>
> unsigned int CameraBuffer::Private::numPlanes() const
> {
> - return info_.numPlanes();
> + return planeInfo_.size();
> }
>
> Span<uint8_t> CameraBuffer::Private::plane(unsigned int plane)
> @@ -97,6 +122,30 @@ Span<uint8_t> CameraBuffer::Private::plane(unsigned int plane)
> return planes_[plane];
> }
>
> +unsigned int CameraBuffer::Private::stride(unsigned int plane) const
> +{
> + if (plane >= planeInfo_.size())
> + return 0;
> +
> + return planeInfo_[plane].stride;
> +}
> +
> +unsigned int CameraBuffer::Private::offset(unsigned int plane) const
> +{
> + if (plane >= planeInfo_.size())
> + return 0;
> +
> + return planeInfo_[plane].offset;
> +}
> +
> +unsigned int CameraBuffer::Private::size(unsigned int plane) const
> +{
> + if (plane >= planeInfo_.size())
> + return 0;
> +
> + return planeInfo_[plane].size;
> +}
> +
> size_t CameraBuffer::Private::jpegBufferSize(size_t maxJpegBufferSize) const
> {
> if (maps_.empty()) {
> @@ -129,19 +178,10 @@ bool CameraBuffer::Private::Map()
> }
> maps_.emplace_back(static_cast<uint8_t *>(address), bufferLength);
>
> - const unsigned int numPlanes = info_.numPlanes();
> - planes_.resize(numPlanes);
> - unsigned int offset = 0;
> - for (unsigned int i = 0; i < numPlanes; ++i) {
> - const unsigned int vertSubSample = info_.planes[i].verticalSubSampling;
> - const unsigned int stride = info_.stride(size_.width, i, 1u);
> - const unsigned int planeSize =
> - stride * ((size_.height + vertSubSample - 1) / vertSubSample);
> -
> - planes_[i] = libcamera::Span<uint8_t>(
> - static_cast<uint8_t *>(address) + offset, planeSize);
> -
> - offset += planeSize;
> + planes_.reserve(planeInfo_.size());
> + for (const auto &info : planeInfo_) {
> + planes_.emplace_back(
> + static_cast<uint8_t *>(address) + info.offset, info.size);
> }
>
> return true;
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list