[libcamera-devel] [PATCH 07/10] android: post_processor: Use CameraBuffer API
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Mar 2 00:55:08 CET 2021
Hi Jacopo,
Thank you for the patch.
On Mon, Mar 01, 2021 at 04:01:08PM +0100, Jacopo Mondi wrote:
> Use the newly introduced CameraBuffer class as the type for the
> destination buffer in the PostProcessor class hierarchy in place of the
> libcamera::MappedFrameBuffer one and use its API to retrieve the length
> and the location of the CameraBuffer plane allocated for JPEG
> post-processing.
>
> Remove all the assumption on the underlying memory storage and only go
> through the CameraBuffer API when dealing with memory buffers. To do so
> rework the Encoder interface to use a raw pointer and an explicit size
> to remove access to the Span<uint8_t> maps that serve as memory storage
> for the current implementation but might not be ideal for other memory
> backend.
>
> Now that the whole PostProcessor hierarchy has been converted to use
> the CameraBuffer API remove libcamera::MappedBuffer as base class
> of the CameraBuffer interface and only reply on its interface.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> src/android/camera_buffer.h | 4 +---
> src/android/jpeg/post_processor_jpeg.cpp | 25 ++++++++----------------
> src/android/jpeg/post_processor_jpeg.h | 2 +-
> src/android/mm/generic_camera_buffer.cpp | 1 +
> src/android/post_processor.h | 4 +++-
> src/android/yuv/post_processor_yuv.cpp | 20 +++++++++----------
> src/android/yuv/post_processor_yuv.h | 6 ++++--
> 7 files changed, 28 insertions(+), 34 deletions(-)
>
> diff --git a/src/android/camera_buffer.h b/src/android/camera_buffer.h
> index ca4f4527c690..2311cdaf96b2 100644
> --- a/src/android/camera_buffer.h
> +++ b/src/android/camera_buffer.h
> @@ -10,11 +10,9 @@
> #include <hardware/camera3.h>
>
> #include <libcamera/class.h>
> -#include <libcamera/internal/buffer.h>
> #include <libcamera/span.h>
>
> -class CameraBuffer final : public libcamera::Extensible,
> - public libcamera::MappedBuffer
> +class CameraBuffer final : public libcamera::Extensible
> {
> LIBCAMERA_DECLARE_PRIVATE(CameraBuffer)
>
> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp
> index ab5b63844067..83244ce6769e 100644
> --- a/src/android/jpeg/post_processor_jpeg.cpp
> +++ b/src/android/jpeg/post_processor_jpeg.cpp
> @@ -83,13 +83,15 @@ void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source,
> }
>
> int PostProcessorJpeg::process(const FrameBuffer &source,
> - libcamera::MappedBuffer *destination,
> + CameraBuffer *destination,
> const CameraMetadata &requestMetadata,
> CameraMetadata *resultMetadata)
> {
> if (!encoder_)
> return 0;
>
> + ASSERT(destination->numPlanes() == 1);
> +
> camera_metadata_ro_entry_t entry;
> int ret;
>
> @@ -172,28 +174,17 @@ int PostProcessorJpeg::process(const FrameBuffer &source,
> const uint8_t quality = ret ? *entry.data.u8 : 95;
> resultMetadata->addEntry(ANDROID_JPEG_QUALITY, &quality, 1);
>
> - int jpeg_size = encoder_->encode(source, destination->maps()[0],
> + int jpeg_size = encoder_->encode(source, destination->plane(0),
> exif.data(), quality);
> if (jpeg_size < 0) {
> LOG(JPEG, Error) << "Failed to encode stream image";
> return jpeg_size;
> }
>
> - /*
> - * Fill in the JPEG blob header.
> - *
> - * The mapped size of the buffer is being returned as
> - * substantially larger than the requested JPEG_MAX_SIZE
> - * (which is referenced from maxJpegBufferSize_). Utilise
> - * this static size to ensure the correct offset of the blob is
> - * determined.
> - *
> - * \todo Investigate if the buffer size mismatch is an issue or
> - * expected behaviour.
> - */
> - uint8_t *resultPtr = destination->maps()[0].data() +
> - cameraDevice_->maxJpegBufferSize() -
> - sizeof(struct camera3_jpeg_blob);
> + /* Fill in the JPEG blob header. */
> + uint8_t *resultPtr = destination->plane(0).data()
> + + destination->plane(0).size()
> + - sizeof(struct camera3_jpeg_blob);
> auto *blob = reinterpret_cast<struct camera3_jpeg_blob *>(resultPtr);
> blob->jpeg_blob_id = CAMERA3_JPEG_BLOB_ID;
> blob->jpeg_size = jpeg_size;
> diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h
> index 7689de73c664..5d2d4ab224b1 100644
> --- a/src/android/jpeg/post_processor_jpeg.h
> +++ b/src/android/jpeg/post_processor_jpeg.h
> @@ -25,7 +25,7 @@ public:
> int configure(const libcamera::StreamConfiguration &incfg,
> const libcamera::StreamConfiguration &outcfg) override;
> int process(const libcamera::FrameBuffer &source,
> - libcamera::MappedBuffer *destination,
> + CameraBuffer *destination,
> const CameraMetadata &requestMetadata,
> CameraMetadata *resultMetadata) override;
>
> diff --git a/src/android/mm/generic_camera_buffer.cpp b/src/android/mm/generic_camera_buffer.cpp
> index eedf16b76542..ea85be805260 100644
> --- a/src/android/mm/generic_camera_buffer.cpp
> +++ b/src/android/mm/generic_camera_buffer.cpp
> @@ -7,6 +7,7 @@
>
> #include "../camera_buffer.h"
>
> +#include <libcamera/internal/buffer.h>
> #include "libcamera/internal/log.h"
>
> using namespace libcamera;
> diff --git a/src/android/post_processor.h b/src/android/post_processor.h
> index ac40d3414892..4944078b490c 100644
> --- a/src/android/post_processor.h
> +++ b/src/android/post_processor.h
> @@ -12,6 +12,8 @@
>
> #include <libcamera/internal/buffer.h>
>
> +#include "camera_buffer.h"
> +
> class CameraMetadata;
>
> class PostProcessor
> @@ -22,7 +24,7 @@ public:
> virtual int configure(const libcamera::StreamConfiguration &inCfg,
> const libcamera::StreamConfiguration &outCfg) = 0;
> virtual int process(const libcamera::FrameBuffer &source,
> - libcamera::MappedBuffer *destination,
> + CameraBuffer *destination,
> const CameraMetadata &requestMetadata,
> CameraMetadata *resultMetadata) = 0;
> };
> diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp
> index 1318349ad66b..b67364c8f147 100644
> --- a/src/android/yuv/post_processor_yuv.cpp
> +++ b/src/android/yuv/post_processor_yuv.cpp
> @@ -48,7 +48,7 @@ int PostProcessorYuv::configure(const StreamConfiguration &inCfg,
> }
>
> int PostProcessorYuv::process(const FrameBuffer &source,
> - libcamera::MappedBuffer *destination,
> + CameraBuffer *destination,
> [[maybe_unused]] const CameraMetadata &requestMetadata,
> [[maybe_unused]] CameraMetadata *metadata)
> {
> @@ -66,9 +66,9 @@ int PostProcessorYuv::process(const FrameBuffer &source,
> sourceMapped.maps()[1].data(),
> sourceStride_[1],
> sourceSize_.width, sourceSize_.height,
> - destination->maps()[0].data(),
> + destination->plane(0).data(),
> destinationStride_[0],
> - destination->maps()[1].data(),
> + destination->plane(1).data(),
> destinationStride_[1],
> destinationSize_.width,
> destinationSize_.height,
> @@ -82,16 +82,16 @@ int PostProcessorYuv::process(const FrameBuffer &source,
> }
>
> bool PostProcessorYuv::isValidBuffers(const FrameBuffer &source,
> - const libcamera::MappedBuffer &destination) const
> + const CameraBuffer &destination) const
> {
> if (source.planes().size() != 2) {
> LOG(YUV, Error) << "Invalid number of source planes: "
> << source.planes().size();
> return false;
> }
> - if (destination.maps().size() != 2) {
> + if (destination.numPlanes() != 2) {
> LOG(YUV, Error) << "Invalid number of destination planes: "
> - << destination.maps().size();
> + << destination.numPlanes();
> return false;
> }
>
> @@ -106,12 +106,12 @@ bool PostProcessorYuv::isValidBuffers(const FrameBuffer &source,
> << sourceLength_[1] << "}";
> return false;
> }
> - if (destination.maps()[0].size() < destinationLength_[0] ||
> - destination.maps()[1].size() < destinationLength_[1]) {
> + if (destination.plane(0).size() < destinationLength_[0] ||
> + destination.plane(1).size() < destinationLength_[1]) {
> LOG(YUV, Error)
> << "The destination planes lengths are too small, actual size: {"
> - << destination.maps()[0].size() << ", "
> - << destination.maps()[1].size()
> + << destination.plane(0).size() << ", "
> + << destination.plane(1).size()
> << "}, expected size: {"
> << sourceLength_[0] << ", "
> << sourceLength_[1] << "}";
> diff --git a/src/android/yuv/post_processor_yuv.h b/src/android/yuv/post_processor_yuv.h
> index c58b4cf790fc..94620dd0a870 100644
> --- a/src/android/yuv/post_processor_yuv.h
> +++ b/src/android/yuv/post_processor_yuv.h
> @@ -9,6 +9,8 @@
>
> #include "../post_processor.h"
>
> +#include "../camera_buffer.h"
As post_processor.h defines the process() function and thus pulls the
CameraBuffer type definition, you don't need to include camera_buffer.h
here.
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> +
> #include <libcamera/geometry.h>
>
> class CameraDevice;
> @@ -21,13 +23,13 @@ public:
> int configure(const libcamera::StreamConfiguration &incfg,
> const libcamera::StreamConfiguration &outcfg) override;
> int process(const libcamera::FrameBuffer &source,
> - libcamera::MappedBuffer *destination,
> + CameraBuffer *destination,
> const CameraMetadata &requestMetadata,
> CameraMetadata *metadata) override;
>
> private:
> bool isValidBuffers(const libcamera::FrameBuffer &source,
> - const libcamera::MappedBuffer &destination) const;
> + const CameraBuffer &destination) const;
> void calculateLengths(const libcamera::StreamConfiguration &inCfg,
> const libcamera::StreamConfiguration &outCfg);
>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list