From patchwork Thu Oct 8 14:10:37 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 10023 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 C4762BEEE0 for ; Thu, 8 Oct 2020 14:11:04 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 90625605C9; Thu, 8 Oct 2020 16:11:04 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=uajain.com header.i=@uajain.com header.b="QJzHx1xb"; dkim-atps=neutral Received: from mail.uajain.com (static.126.159.217.95.clients.your-server.de [95.217.159.126]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id DD1496035D for ; Thu, 8 Oct 2020 16:11:02 +0200 (CEST) From: Umang Jain DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=uajain.com; s=mail; t=1602166262; bh=7Y50rK2MDtOwPqLQIQhnr4SYGGAMG0W+e1fkCoMt0hw=; h=From:To:Cc:Subject:In-Reply-To:References; b=QJzHx1xbYG5j4dkHKE+9e6p6JHX9ce8F5d9fW8SC19+Ab0g3FeUkL2uLC8QZ9IwMo WNyBsfJL6+ID/N2TgfeT6mx/hbNK5gig6sJODnigxwZHgAiiFIdZk4Er4CRnf55RDG zn0T1uJ2Nu+eHJw4FB44aKSzTYHuc1rKmZ+qHokWQ/Wro+MNakS/RZFYyh2u326+Gb sFzyk4Bg8Ykgj61ICwmth+o9r69ULjuvqcVTNxoZL607O84DOO/0E7XQd1soLdx/lw HIqIaJP0z6T4uoaD/dk9gctCFEYqLzcutmIJ4qJZHteJjPs09N0u/HRLSSo8bDD3xb HDpHDKfzc5CiQ== To: libcamera-devel@lists.libcamera.org Date: Thu, 8 Oct 2020 19:40:37 +0530 Message-Id: <20201008141038.83425-3-email@uajain.com> In-Reply-To: <20201008141038.83425-1-email@uajain.com> References: <20201008141038.83425-1-email@uajain.com> Mime-Version: 1.0 Subject: [libcamera-devel] [RFC PATCH 2/3] android: jpeg: Port to PostProcessor interface 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" Remove the existing Encoder interface completely and use the PostProcessor interface instead. Now the ::encode() function will be called by PostProcessor::process() internally and will not be directly exposed in CameraStream. Similarly, other operations can be introduced internally in the PostProcessorJpeg, and can be called through its process() interface. Signed-off-by: Umang Jain --- src/android/camera_device.h | 1 - src/android/camera_stream.cpp | 74 +++------------ src/android/camera_stream.h | 9 +- src/android/jpeg/encoder.h | 25 ----- ...er_libjpeg.cpp => post_processor_jpeg.cpp} | 92 +++++++++++++++++-- ...ncoder_libjpeg.h => post_processor_jpeg.h} | 27 ++++-- src/android/meson.build | 2 +- 7 files changed, 122 insertions(+), 108 deletions(-) delete mode 100644 src/android/jpeg/encoder.h rename src/android/jpeg/{encoder_libjpeg.cpp => post_processor_jpeg.cpp} (67%) rename src/android/jpeg/{encoder_libjpeg.h => post_processor_jpeg.h} (55%) diff --git a/src/android/camera_device.h b/src/android/camera_device.h index 777d1a3..25de12e 100644 --- a/src/android/camera_device.h +++ b/src/android/camera_device.h @@ -25,7 +25,6 @@ #include "libcamera/internal/message.h" #include "camera_stream.h" -#include "jpeg/encoder.h" class CameraMetadata; diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp index eac1480..ed3bb41 100644 --- a/src/android/camera_stream.cpp +++ b/src/android/camera_stream.cpp @@ -9,9 +9,7 @@ #include "camera_device.h" #include "camera_metadata.h" -#include "jpeg/encoder.h" -#include "jpeg/encoder_libjpeg.h" -#include "jpeg/exif.h" +#include "jpeg/post_processor_jpeg.h" using namespace libcamera; @@ -24,8 +22,15 @@ CameraStream::CameraStream(CameraDevice *cameraDevice, Type type, { config_ = cameraDevice_->cameraConfiguration(); - if (type_ == Type::Internal || type_ == Type::Mapped) - encoder_ = std::make_unique(); + if (type_ == Type::Internal || type_ == Type::Mapped) { + /* + * \todo There might be multiple post-processors. The logic + * which should be instantiated here, is deferred for future. + * For now, we only have PostProcessJpeg and that is what we + * will instantiate here. + */ + postProcessor_ = std::make_unique(); + } if (type == Type::Internal) { allocator_ = std::make_unique(cameraDevice_->camera()); @@ -45,8 +50,8 @@ Stream *CameraStream::stream() const int CameraStream::configure() { - if (encoder_) { - int ret = encoder_->configure(configuration()); + if (postProcessor_) { + int ret = postProcessor_->configure(configuration()); if (ret) return ret; } @@ -69,60 +74,11 @@ int CameraStream::configure() int CameraStream::process(const libcamera::FrameBuffer &source, MappedCamera3Buffer *dest, CameraMetadata *metadata) { - if (!encoder_) + if (!postProcessor_) return 0; - /* Set EXIF metadata for various tags. */ - Exif exif; - /* \todo Set Make and Model from external vendor tags. */ - exif.setMake("libcamera"); - exif.setModel("cameraModel"); - exif.setOrientation(cameraDevice_->orientation()); - exif.setSize(configuration().size); - /* - * We set the frame's EXIF timestamp as the time of encode. - * Since the precision we need for EXIF timestamp is only one - * second, it is good enough. - */ - exif.setTimestamp(std::time(nullptr)); - if (exif.generate() != 0) - LOG(HAL, Error) << "Failed to generate valid EXIF data"; - - int jpeg_size = encoder_->encode(&source, dest->maps()[0], exif.data()); - if (jpeg_size < 0) { - LOG(HAL, 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 = dest->maps()[0].data() + - cameraDevice_->maxJpegBufferSize() - - sizeof(struct camera3_jpeg_blob); - auto *blob = reinterpret_cast(resultPtr); - blob->jpeg_blob_id = CAMERA3_JPEG_BLOB_ID; - blob->jpeg_size = jpeg_size; - - /* Update the JPEG result Metadata. */ - metadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1); - - const uint32_t jpeg_quality = 95; - metadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 1); - - const uint32_t jpeg_orientation = 0; - metadata->addEntry(ANDROID_JPEG_ORIENTATION, &jpeg_orientation, 1); - - return 0; + return postProcessor_->process(&source, dest->maps()[0], + metadata, cameraDevice_); } FrameBuffer *CameraStream::getBuffer() diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h index 8df0101..8d0f2e3 100644 --- a/src/android/camera_stream.h +++ b/src/android/camera_stream.h @@ -19,7 +19,12 @@ #include #include -class Encoder; +/* + * \todo Ideally we want to replace this include with forward-declaration. + * If we do that. currently we get a compile error. + */ +#include "post_processor.h" + class CameraDevice; class CameraMetadata; class MappedCamera3Buffer; @@ -135,7 +140,7 @@ private: */ unsigned int index_; - std::unique_ptr encoder_; + std::unique_ptr postProcessor_; std::unique_ptr allocator_; std::vector buffers_; /* diff --git a/src/android/jpeg/encoder.h b/src/android/jpeg/encoder.h deleted file mode 100644 index cf26d67..0000000 --- a/src/android/jpeg/encoder.h +++ /dev/null @@ -1,25 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ -/* - * Copyright (C) 2020, Google Inc. - * - * encoder.h - Image encoding interface - */ -#ifndef __ANDROID_JPEG_ENCODER_H__ -#define __ANDROID_JPEG_ENCODER_H__ - -#include -#include -#include - -class Encoder -{ -public: - virtual ~Encoder() {}; - - virtual int configure(const libcamera::StreamConfiguration &cfg) = 0; - virtual int encode(const libcamera::FrameBuffer *source, - const libcamera::Span &destination, - const libcamera::Span &exifData) = 0; -}; - -#endif /* __ANDROID_JPEG_ENCODER_H__ */ diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp similarity index 67% rename from src/android/jpeg/encoder_libjpeg.cpp rename to src/android/jpeg/post_processor_jpeg.cpp index 510613c..eeb4e95 100644 --- a/src/android/jpeg/encoder_libjpeg.cpp +++ b/src/android/jpeg/post_processor_jpeg.cpp @@ -2,10 +2,14 @@ /* * Copyright (C) 2020, Google Inc. * - * encoder_libjpeg.cpp - JPEG encoding using libjpeg native API + * post_processor_jpeg.cpp - JPEG Post Processor */ -#include "encoder_libjpeg.h" +#include "post_processor_jpeg.h" + +#include "exif.h" + +#include "../camera_device.h" #include #include @@ -25,6 +29,14 @@ using namespace libcamera; +#define extract_va_arg(type, arg, last) \ +{ \ + va_list ap; \ + va_start(ap, last); \ + arg = va_arg(ap, type); \ + va_end(ap); \ +} + LOG_DEFINE_CATEGORY(JPEG) namespace { @@ -67,7 +79,7 @@ const struct JPEGPixelFormatInfo &findPixelInfo(const PixelFormat &format) } /* namespace */ -EncoderLibJpeg::EncoderLibJpeg() +PostProcessorJpeg::PostProcessorJpeg() : quality_(95) { /* \todo Expand error handling coverage with a custom handler. */ @@ -76,12 +88,12 @@ EncoderLibJpeg::EncoderLibJpeg() jpeg_create_compress(&compress_); } -EncoderLibJpeg::~EncoderLibJpeg() +PostProcessorJpeg::~PostProcessorJpeg() { jpeg_destroy_compress(&compress_); } -int EncoderLibJpeg::configure(const StreamConfiguration &cfg) +int PostProcessorJpeg::configure(const StreamConfiguration &cfg) { const struct JPEGPixelFormatInfo info = findPixelInfo(cfg.pixelFormat); if (info.colorSpace == JCS_UNKNOWN) @@ -104,7 +116,67 @@ int EncoderLibJpeg::configure(const StreamConfiguration &cfg) return 0; } -void EncoderLibJpeg::compressRGB(const libcamera::MappedBuffer *frame) +int PostProcessorJpeg::process(const libcamera::FrameBuffer *source, + const libcamera::Span &destination, + CameraMetadata *metadata, ...) +{ + CameraDevice *device = nullptr; + extract_va_arg(CameraDevice *, device, metadata); + + /* Set EXIF metadata for various tags. */ + Exif exif; + /* \todo Set Make and Model from external vendor tags. */ + exif.setMake("libcamera"); + exif.setModel("cameraModel"); + exif.setOrientation(device->orientation()); + exif.setSize(Size {compress_.image_width, compress_.image_height}); + /* + * We set the frame's EXIF timestamp as the time of encode. + * Since the precision we need for EXIF timestamp is only one + * second, it is good enough. + */ + exif.setTimestamp(std::time(nullptr)); + if (exif.generate() != 0) + LOG(JPEG, Error) << "Failed to generate valid EXIF data"; + + int jpeg_size = encode(source, destination, exif.data()); + 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.data() + + device->maxJpegBufferSize() - + sizeof(struct camera3_jpeg_blob); + auto *blob = reinterpret_cast(resultPtr); + blob->jpeg_blob_id = CAMERA3_JPEG_BLOB_ID; + blob->jpeg_size = jpeg_size; + + /* Update the JPEG result Metadata. */ + metadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1); + + const uint32_t jpeg_quality = 95; + metadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 1); + + const uint32_t jpeg_orientation = 0; + metadata->addEntry(ANDROID_JPEG_ORIENTATION, &jpeg_orientation, 1); + + return 0; +} + +void PostProcessorJpeg::compressRGB(const libcamera::MappedBuffer *frame) { unsigned char *src = static_cast(frame->maps()[0].data()); /* \todo Stride information should come from buffer configuration. */ @@ -122,7 +194,7 @@ void EncoderLibJpeg::compressRGB(const libcamera::MappedBuffer *frame) * Compress the incoming buffer from a supported NV format. * This naively unpacks the semi-planar NV12 to a YUV888 format for libjpeg. */ -void EncoderLibJpeg::compressNV(const libcamera::MappedBuffer *frame) +void PostProcessorJpeg::compressNV(const libcamera::MappedBuffer *frame) { uint8_t tmprowbuf[compress_.image_width * 3]; @@ -179,9 +251,9 @@ void EncoderLibJpeg::compressNV(const libcamera::MappedBuffer *frame) } } -int EncoderLibJpeg::encode(const FrameBuffer *source, - const libcamera::Span &dest, - const libcamera::Span &exifData) +int PostProcessorJpeg::encode(const FrameBuffer *source, + const libcamera::Span &dest, + const libcamera::Span &exifData) { MappedFrameBuffer frame(source, PROT_READ); if (!frame.isValid()) { diff --git a/src/android/jpeg/encoder_libjpeg.h b/src/android/jpeg/post_processor_jpeg.h similarity index 55% rename from src/android/jpeg/encoder_libjpeg.h rename to src/android/jpeg/post_processor_jpeg.h index 1e8df05..7f9ce70 100644 --- a/src/android/jpeg/encoder_libjpeg.h +++ b/src/android/jpeg/post_processor_jpeg.h @@ -2,30 +2,37 @@ /* * Copyright (C) 2020, Google Inc. * - * encoder_libjpeg.h - JPEG encoding using libjpeg + * post_processor_jpeg.h - JPEG Post Processor */ -#ifndef __ANDROID_JPEG_ENCODER_LIBJPEG_H__ -#define __ANDROID_JPEG_ENCODER_LIBJPEG_H__ +#ifndef __ANDROID_POST_PROCESSOR_JPEG_H__ +#define __ANDROID_POST_PROCESSOR_JPEG_H__ -#include "encoder.h" +#include "../post_processor.h" +#include "../camera_metadata.h" #include "libcamera/internal/buffer.h" #include "libcamera/internal/formats.h" #include -class EncoderLibJpeg : public Encoder +class PostProcessorJpeg : public PostProcessor { public: - EncoderLibJpeg(); - ~EncoderLibJpeg(); + PostProcessorJpeg(); + ~PostProcessorJpeg(); int configure(const libcamera::StreamConfiguration &cfg) override; + int process(const libcamera::FrameBuffer *source, + const libcamera::Span &destination, + CameraMetadata *metadata, + ...) override; + + +private: int encode(const libcamera::FrameBuffer *source, const libcamera::Span &destination, - const libcamera::Span &exifData) override; + const libcamera::Span &exifData); -private: void compressRGB(const libcamera::MappedBuffer *frame); void compressNV(const libcamera::MappedBuffer *frame); @@ -40,4 +47,4 @@ private: bool nvSwap_; }; -#endif /* __ANDROID_JPEG_ENCODER_LIBJPEG_H__ */ +#endif /* __ANDROID_POST_PROCESSOR_JPEG_H__ */ diff --git a/src/android/meson.build b/src/android/meson.build index 802bb89..02b3b47 100644 --- a/src/android/meson.build +++ b/src/android/meson.build @@ -21,8 +21,8 @@ android_hal_sources = files([ 'camera_metadata.cpp', 'camera_ops.cpp', 'camera_stream.cpp', - 'jpeg/encoder_libjpeg.cpp', 'jpeg/exif.cpp', + 'jpeg/post_processor_jpeg.cpp', ]) android_camera_metadata_sources = files([