From patchwork Fri Feb 26 13:29:28 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 11401 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id EAD65BD1F1 for ; Fri, 26 Feb 2021 13:29:22 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 6A02968A78; Fri, 26 Feb 2021 14:29:22 +0100 (CET) Received: from relay12.mail.gandi.net (relay12.mail.gandi.net [217.70.178.232]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7A4D668A73 for ; Fri, 26 Feb 2021 14:29:16 +0100 (CET) Received: from uno.LocalDomain (93-61-96-190.ip145.fastwebnet.it [93.61.96.190]) (Authenticated sender: jacopo@jmondi.org) by relay12.mail.gandi.net (Postfix) with ESMTPSA id 34B1A200005 for ; Fri, 26 Feb 2021 13:29:16 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Fri, 26 Feb 2021 14:29:28 +0100 Message-Id: <20210226132932.165484-9-jacopo@jmondi.org> X-Mailer: git-send-email 2.30.0 In-Reply-To: <20210226132932.165484-1-jacopo@jmondi.org> References: <20210226132932.165484-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 08/12] android: post_processor: Use CameraBuffer API X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 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 Reviewed-by: Laurent Pinchart --- src/android/camera_buffer.h | 2 +- src/android/jpeg/encoder.h | 4 +++- src/android/jpeg/encoder_libjpeg.cpp | 19 ++++++++-------- src/android/jpeg/encoder_libjpeg.h | 4 ++-- src/android/jpeg/post_processor_jpeg.cpp | 28 ++++++++++-------------- src/android/jpeg/post_processor_jpeg.h | 2 +- src/android/post_processor.h | 4 +++- src/android/yuv/post_processor_yuv.cpp | 20 ++++++++--------- src/android/yuv/post_processor_yuv.h | 4 ++-- 9 files changed, 42 insertions(+), 45 deletions(-) diff --git a/src/android/camera_buffer.h b/src/android/camera_buffer.h index 2a91e6a3c9c1..b251e4514864 100644 --- a/src/android/camera_buffer.h +++ b/src/android/camera_buffer.h @@ -11,7 +11,7 @@ #include -class CameraBuffer : public libcamera::MappedBuffer +class CameraBuffer { public: CameraBuffer(buffer_handle_t camera3Buffer, int flags); diff --git a/src/android/jpeg/encoder.h b/src/android/jpeg/encoder.h index 8d449369869f..a3c47c09a06e 100644 --- a/src/android/jpeg/encoder.h +++ b/src/android/jpeg/encoder.h @@ -11,6 +11,8 @@ #include #include +#include "../camera_buffer.h" + class Encoder { public: @@ -18,7 +20,7 @@ public: virtual int configure(const libcamera::StreamConfiguration &cfg) = 0; virtual int encode(const libcamera::FrameBuffer &source, - libcamera::Span destination, + uint8_t *destination, size_t destinationSize, libcamera::Span exifData, unsigned int quality) = 0; }; diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp index f006e1d1999a..a965620703df 100644 --- a/src/android/jpeg/encoder_libjpeg.cpp +++ b/src/android/jpeg/encoder_libjpeg.cpp @@ -177,8 +177,9 @@ void EncoderLibJpeg::compressNV(Span frame) } } -int EncoderLibJpeg::encode(const FrameBuffer &source, Span dest, - Span exifData, unsigned int quality) +int EncoderLibJpeg::encode(const FrameBuffer &source, uint8_t *destination, + size_t destinationSize, Span exifData, + unsigned int quality) { MappedFrameBuffer frame(&source, PROT_READ); if (!frame.isValid()) { @@ -187,15 +188,13 @@ int EncoderLibJpeg::encode(const FrameBuffer &source, Span dest, return frame.error(); } - return encode(frame.maps()[0], dest, exifData, quality); + return encode(frame.maps()[0], destination, destinationSize, exifData, quality); } -int EncoderLibJpeg::encode(Span src, Span dest, - Span exifData, unsigned int quality) +int EncoderLibJpeg::encode(Span src, uint8_t *destination, + size_t destinationSize, Span exifData, + unsigned int quality) { - unsigned char *destination = dest.data(); - unsigned long size = dest.size(); - jpeg_set_quality(&compress_, quality, TRUE); /* @@ -206,7 +205,7 @@ int EncoderLibJpeg::encode(Span src, Span dest, * \todo Implement our own custom memory destination to prevent * reallocation and prefer failure with correct reporting. */ - jpeg_mem_dest(&compress_, &destination, &size); + jpeg_mem_dest(&compress_, &destination, &destinationSize); jpeg_start_compress(&compress_, TRUE); @@ -226,5 +225,5 @@ int EncoderLibJpeg::encode(Span src, Span dest, jpeg_finish_compress(&compress_); - return size; + return destinationSize; } diff --git a/src/android/jpeg/encoder_libjpeg.h b/src/android/jpeg/encoder_libjpeg.h index 838da7728382..bda5c16089df 100644 --- a/src/android/jpeg/encoder_libjpeg.h +++ b/src/android/jpeg/encoder_libjpeg.h @@ -22,11 +22,11 @@ public: int configure(const libcamera::StreamConfiguration &cfg) override; int encode(const libcamera::FrameBuffer &source, - libcamera::Span destination, + uint8_t *destination, size_t destinationSize, libcamera::Span exifData, unsigned int quality) override; int encode(libcamera::Span source, - libcamera::Span destination, + uint8_t *destination, size_t destinationSize, libcamera::Span exifData, unsigned int quality); diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp index ab5b63844067..d6eeb962e81d 100644 --- a/src/android/jpeg/post_processor_jpeg.cpp +++ b/src/android/jpeg/post_processor_jpeg.cpp @@ -73,7 +73,9 @@ void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source, thumbnail->resize(rawThumbnail.size()); int jpeg_size = thumbnailEncoder_.encode(rawThumbnail, - *thumbnail, {}, quality); + thumbnail->data(), + thumbnail->size(), + {}, quality); thumbnail->resize(jpeg_size); LOG(JPEG, Debug) @@ -83,13 +85,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,27 +176,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), + destination->planeSize(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() - + /* Fill in the JPEG blob header. */ + uint8_t *resultPtr = destination->plane(0) + + destination->planeSize(0) - sizeof(struct camera3_jpeg_blob); auto *blob = reinterpret_cast(resultPtr); blob->jpeg_blob_id = CAMERA3_JPEG_BLOB_ID; 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/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 +#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..f1487185a95a 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), destinationStride_[0], - destination->maps()[1].data(), + destination->plane(1), 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.planeSize(0) < destinationLength_[0] || + destination.planeSize(1) < destinationLength_[1]) { LOG(YUV, Error) << "The destination planes lengths are too small, actual size: {" - << destination.maps()[0].size() << ", " - << destination.maps()[1].size() + << destination.planeSize(0) << ", " + << destination.planeSize(1) << "}, 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..f8b1ba23fa6c 100644 --- a/src/android/yuv/post_processor_yuv.h +++ b/src/android/yuv/post_processor_yuv.h @@ -21,13 +21,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);