[{"id":13235,"web_url":"https://patchwork.libcamera.org/comment/13235/","msgid":"<20201016063253.GK3829@pendragon.ideasonboard.com>","date":"2020-10-16T06:32:53","subject":"Re: [libcamera-devel] [PATCH v2 2/2] android: jpeg: Port to\n\tPostProcessor interface","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nThank you for the patch.\n\nOn Fri, Oct 16, 2020 at 11:07:54AM +0530, Umang Jain wrote:\n> Port the CameraStream's JPEG-encoding bits to PostProcessorJpeg.\n> This encapsulates the encoder and EXIF generation code into the\n> PostProcessorJpeg layer and removes these specifics related to JPEG,\n> from the CameraStream itself.\n> \n> Signed-off-by: Umang Jain \n> Tested-by: Kieran Bingham \n> Reviewed-by: Kieran Bingham \n\nReviewed-by: Laurent Pinchart \n\nKieran, would you be able to test this new version, and push it if all\nlooks good to you ?\n\n> ---\n> src/android/camera_device.cpp | 1 +\n> src/android/camera_stream.cpp | 77 ++++-------------\n> src/android/camera_stream.h | 4 +-\n> src/android/jpeg/encoder_libjpeg.cpp | 2 +-\n> src/android/jpeg/post_processor_jpeg.cpp | 105 +++++++++++++++++++++++\n> src/android/jpeg/post_processor_jpeg.h | 36 ++++++++\n> src/android/meson.build | 1 +\n> 7 files changed, 164 insertions(+), 62 deletions(-)\n> create mode 100644 src/android/jpeg/post_processor_jpeg.cpp\n> create mode 100644 src/android/jpeg/post_processor_jpeg.h\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 0983232..d706cf4 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -7,6 +7,7 @@\n> \n> #include \"camera_device.h\"\n> #include \"camera_ops.h\"\n> +#include \"post_processor.h\"\n> \n> #include \n> #include \n> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> index d8e45c2..1b8afa8 100644\n> --- a/src/android/camera_stream.cpp\n> +++ b/src/android/camera_stream.cpp\n> @@ -9,9 +9,9 @@\n> \n> #include \"camera_device.h\"\n> #include \"camera_metadata.h\"\n> -#include \"jpeg/encoder.h\"\n> -#include \"jpeg/encoder_libjpeg.h\"\n> -#include \"jpeg/exif.h\"\n> +#include \"jpeg/post_processor_jpeg.h\"\n> +\n> +#include \n> \n> using namespace libcamera;\n> \n> @@ -45,8 +45,15 @@ CameraStream::CameraStream(CameraDevice *cameraDevice, Type type,\n> {\n> \tconfig_ = cameraDevice_->cameraConfiguration();\n> \n> -\tif (type_ == Type::Internal || type_ == Type::Mapped)\n> -\t\tencoder_ = std::make_unique();\n> +\tif (type_ == Type::Internal || type_ == Type::Mapped) {\n> +\t\t/*\n> +\t\t * \\todo There might be multiple post-processors. The logic\n> +\t\t * which should be instantiated here, is deferred for the\n> +\t\t * future. For now, we only have PostProcessorJpeg and that\n> +\t\t * is what we instantiate here.\n> +\t\t */\n> +\t\tpostProcessor_ = std::make_unique(cameraDevice_);\n> +\t}\n> \n> \tif (type == Type::Internal) {\n> \t\tallocator_ = std::make_unique(cameraDevice_->camera());\n> @@ -66,8 +73,10 @@ Stream *CameraStream::stream() const\n> \n> int CameraStream::configure()\n> {\n> -\tif (encoder_) {\n> -\t\tint ret = encoder_->configure(configuration());\n> +\tif (postProcessor_) {\n> +\t\tStreamConfiguration output = configuration();\n> +\t\toutput.pixelFormat = formats::MJPEG;\n> +\t\tint ret = postProcessor_->configure(configuration(), output);\n> \t\tif (ret)\n> \t\t\treturn ret;\n> \t}\n> @@ -90,60 +99,10 @@ int CameraStream::configure()\n> int CameraStream::process(const libcamera::FrameBuffer &source,\n> \t\t\t MappedCamera3Buffer *dest, CameraMetadata *metadata)\n> {\n> -\tif (!encoder_)\n> +\tif (!postProcessor_)\n> \t\treturn 0;\n> \n> -\t/* Set EXIF metadata for various tags. */\n> -\tExif exif;\n> -\t/* \\todo Set Make and Model from external vendor tags. */\n> -\texif.setMake(\"libcamera\");\n> -\texif.setModel(\"cameraModel\");\n> -\texif.setOrientation(cameraDevice_->orientation());\n> -\texif.setSize(configuration().size);\n> -\t/*\n> -\t * We set the frame's EXIF timestamp as the time of encode.\n> -\t * Since the precision we need for EXIF timestamp is only one\n> -\t * second, it is good enough.\n> -\t */\n> -\texif.setTimestamp(std::time(nullptr));\n> -\tif (exif.generate() != 0)\n> -\t\tLOG(HAL, Error) << \"Failed to generate valid EXIF data\";\n> -\n> -\tint jpeg_size = encoder_->encode(&source, dest->maps()[0], exif.data());\n> -\tif (jpeg_size < 0) {\n> -\t\tLOG(HAL, Error) << \"Failed to encode stream image\";\n> -\t\treturn jpeg_size;\n> -\t}\n> -\n> -\t/*\n> -\t * Fill in the JPEG blob header.\n> -\t *\n> -\t * The mapped size of the buffer is being returned as\n> -\t * substantially larger than the requested JPEG_MAX_SIZE\n> -\t * (which is referenced from maxJpegBufferSize_). Utilise\n> -\t * this static size to ensure the correct offset of the blob is\n> -\t * determined.\n> -\t *\n> -\t * \\todo Investigate if the buffer size mismatch is an issue or\n> -\t * expected behaviour.\n> -\t */\n> -\tuint8_t *resultPtr = dest->maps()[0].data() +\n> -\t\t\t cameraDevice_->maxJpegBufferSize() -\n> -\t\t\t sizeof(struct camera3_jpeg_blob);\n> -\tauto *blob = reinterpret_cast(resultPtr);\n> -\tblob->jpeg_blob_id = CAMERA3_JPEG_BLOB_ID;\n> -\tblob->jpeg_size = jpeg_size;\n> -\n> -\t/* Update the JPEG result Metadata. */\n> -\tmetadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1);\n> -\n> -\tconst uint32_t jpeg_quality = 95;\n> -\tmetadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 1);\n> -\n> -\tconst uint32_t jpeg_orientation = 0;\n> -\tmetadata->addEntry(ANDROID_JPEG_ORIENTATION, &jpeg_orientation, 1);\n> -\n> -\treturn 0;\n> +\treturn postProcessor_->process(&source, dest->maps()[0], metadata);\n> }\n> \n> FrameBuffer *CameraStream::getBuffer()\n> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n> index d3925fb..c367a5f 100644\n> --- a/src/android/camera_stream.h\n> +++ b/src/android/camera_stream.h\n> @@ -19,10 +19,10 @@\n> #include \n> #include \n> \n> -class Encoder;\n> class CameraDevice;\n> class CameraMetadata;\n> class MappedCamera3Buffer;\n> +class PostProcessor;\n> \n> class CameraStream\n> {\n> @@ -130,7 +130,6 @@ private:\n> \tcamera3_stream_t *camera3Stream_;\n> \tunsigned int index_;\n> \n> -\tstd::unique_ptr encoder_;\n> \tstd::unique_ptr allocator_;\n> \tstd::vector buffers_;\n> \t/*\n> @@ -138,6 +137,7 @@ private:\n> \t * an std::vector in CameraDevice.\n> \t */\n> \tstd::unique_ptr mutex_;\n> +\tstd::unique_ptr postProcessor_;\n> };\n> \n> #endif /* __ANDROID_CAMERA_STREAM__ */\n> diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp\n> index a77f5b2..8995ba5 100644\n> --- a/src/android/jpeg/encoder_libjpeg.cpp\n> +++ b/src/android/jpeg/encoder_libjpeg.cpp\n> @@ -25,7 +25,7 @@\n> \n> using namespace libcamera;\n> \n> -LOG_DEFINE_CATEGORY(JPEG)\n> +LOG_DECLARE_CATEGORY(JPEG);\n> \n> namespace {\n> \n> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp\n> new file mode 100644\n> index 0000000..753c28e\n> --- /dev/null\n> +++ b/src/android/jpeg/post_processor_jpeg.cpp\n> @@ -0,0 +1,105 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * post_processor_jpeg.cpp - JPEG Post Processor\n> + */\n> +\n> +#include \"post_processor_jpeg.h\"\n> +\n> +#include \"../camera_device.h\"\n> +#include \"../camera_metadata.h\"\n> +#include \"encoder_libjpeg.h\"\n> +#include \"exif.h\"\n> +\n> +#include \n> +\n> +#include \"libcamera/internal/log.h\"\n> +\n> +using namespace libcamera;\n> +\n> +LOG_DEFINE_CATEGORY(JPEG);\n> +\n> +PostProcessorJpeg::PostProcessorJpeg(CameraDevice *device)\n> +\t: cameraDevice_(device)\n> +{\n> +}\n> +\n> +int PostProcessorJpeg::configure(const StreamConfiguration &inCfg,\n> +\t\t\t\t const StreamConfiguration &outCfg)\n> +{\n> +\tif (inCfg.size != outCfg.size) {\n> +\t\tLOG(JPEG, Error) << \"Mismatch of input and output stream sizes\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tif (outCfg.pixelFormat != formats::MJPEG) {\n> +\t\tLOG(JPEG, Error) << \"Output stream pixel format is not JPEG\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tstreamSize_ = outCfg.size;\n> +\tencoder_ = std::make_unique();\n> +\n> +\treturn encoder_->configure(inCfg);\n> +}\n> +\n> +int PostProcessorJpeg::process(const libcamera::FrameBuffer *source,\n> +\t\t\t const libcamera::Span &destination,\n> +\t\t\t CameraMetadata *metadata)\n> +{\n> +\tif (!encoder_)\n> +\t\treturn 0;\n> +\n> +\t/* Set EXIF metadata for various tags. */\n> +\tExif exif;\n> +\t/* \\todo Set Make and Model from external vendor tags. */\n> +\texif.setMake(\"libcamera\");\n> +\texif.setModel(\"cameraModel\");\n> +\texif.setOrientation(cameraDevice_->orientation());\n> +\texif.setSize(streamSize_);\n> +\t/*\n> +\t * We set the frame's EXIF timestamp as the time of encode.\n> +\t * Since the precision we need for EXIF timestamp is only one\n> +\t * second, it is good enough.\n> +\t */\n> +\texif.setTimestamp(std::time(nullptr));\n> +\tif (exif.generate() != 0)\n> +\t\tLOG(JPEG, Error) << \"Failed to generate valid EXIF data\";\n> +\n> +\tint jpeg_size = encoder_->encode(source, destination, exif.data());\n> +\tif (jpeg_size < 0) {\n> +\t\tLOG(JPEG, Error) << \"Failed to encode stream image\";\n> +\t\treturn jpeg_size;\n> +\t}\n> +\n> +\t/*\n> +\t * Fill in the JPEG blob header.\n> +\t *\n> +\t * The mapped size of the buffer is being returned as\n> +\t * substantially larger than the requested JPEG_MAX_SIZE\n> +\t * (which is referenced from maxJpegBufferSize_). Utilise\n> +\t * this static size to ensure the correct offset of the blob is\n> +\t * determined.\n> +\t *\n> +\t * \\todo Investigate if the buffer size mismatch is an issue or\n> +\t * expected behaviour.\n> +\t */\n> +\tuint8_t *resultPtr = destination.data() +\n> +\t\t\t cameraDevice_->maxJpegBufferSize() -\n> +\t\t\t sizeof(struct camera3_jpeg_blob);\n> +\tauto *blob = reinterpret_cast(resultPtr);\n> +\tblob->jpeg_blob_id = CAMERA3_JPEG_BLOB_ID;\n> +\tblob->jpeg_size = jpeg_size;\n> +\n> +\t/* Update the JPEG result Metadata. */\n> +\tmetadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1);\n> +\n> +\tconst uint32_t jpeg_quality = 95;\n> +\tmetadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 1);\n> +\n> +\tconst uint32_t jpeg_orientation = 0;\n> +\tmetadata->addEntry(ANDROID_JPEG_ORIENTATION, &jpeg_orientation, 1);\n> +\n> +\treturn 0;\n> +}\n> diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h\n> new file mode 100644\n> index 0000000..62c8650\n> --- /dev/null\n> +++ b/src/android/jpeg/post_processor_jpeg.h\n> @@ -0,0 +1,36 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * post_processor_jpeg.h - JPEG Post Processor\n> + */\n> +#ifndef __ANDROID_POST_PROCESSOR_JPEG_H__\n> +#define __ANDROID_POST_PROCESSOR_JPEG_H__\n> +\n> +#include \"../post_processor.h\"\n> +\n> +#include \n> +\n> +#include \"libcamera/internal/buffer.h\"\n> +\n> +class Encoder;\n> +class CameraDevice;\n> +\n> +class PostProcessorJpeg : public PostProcessor\n> +{\n> +public:\n> +\tPostProcessorJpeg(CameraDevice *device);\n> +\n> +\tint configure(const libcamera::StreamConfiguration &incfg,\n> +\t\t const libcamera::StreamConfiguration &outcfg) override;\n> +\tint process(const libcamera::FrameBuffer *source,\n> +\t\t const libcamera::Span &destination,\n> +\t\t CameraMetadata *metadata) override;\n> +\n> +private:\n> +\tCameraDevice *cameraDevice_;\n> +\tstd::unique_ptr encoder_;\n> +\tlibcamera::Size streamSize_;\n> +};\n> +\n> +#endif /* __ANDROID_POST_PROCESSOR_JPEG_H__ */\n> diff --git a/src/android/meson.build b/src/android/meson.build\n> index b2b2293..5a01bea 100644\n> --- a/src/android/meson.build\n> +++ b/src/android/meson.build\n> @@ -24,6 +24,7 @@ android_hal_sources = files([\n> 'camera_worker.cpp',\n> 'jpeg/encoder_libjpeg.cpp',\n> 'jpeg/exif.cpp',\n> + 'jpeg/post_processor_jpeg.cpp',\n> ])\n> \n> android_camera_metadata_sources = files([","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 5734FBDB13\n\tfor ;\n\tFri, 16 Oct 2020 06:33:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E480E6108B;\n\tFri, 16 Oct 2020 08:33:40 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1F7B560CE6\n\tfor ;\n\tFri, 16 Oct 2020 08:33:40 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 99C95528;\n\tFri, 16 Oct 2020 08:33:39 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"pJ6uIfnW\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1602830019;\n\tbh=hvUJeJcKxMs6wyJXa/yPQpfqod1tuMIo8WIYLVkmzGA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=pJ6uIfnWZHkf94cywGC4T5rFPfpQyfcGvvVhP4GnEOjMGrgQ9iXtGHH4zD6JW+TSh\n\teInvvFtkcp4+bKjOIu0wdSPsQPRpT3f1w1AfqmpPyDHskScYDpDfdAhLcSe5NWQ+On\n\tJYjNqjfrif6yvF2D3OAVfhOcZYrlt7PwKXWVJnR0=","Date":"Fri, 16 Oct 2020 09:32:53 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Message-ID":"<20201016063253.GK3829@pendragon.ideasonboard.com>","References":"<20201016053754.17251-1-email@uajain.com>\n\t<20201016053754.17251-3-email@uajain.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201016053754.17251-3-email@uajain.com>","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] android: jpeg: Port to\n\tPostProcessor interface","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13236,"web_url":"https://patchwork.libcamera.org/comment/13236/","msgid":"<617edcee-9cbc-e3ac-8c82-8ebbc68b3f28@uajain.com>","date":"2020-10-16T07:15:20","subject":"Re: [libcamera-devel] [PATCH v2 2/2] android: jpeg: Port to\n\tPostProcessor interface","submitter":{"id":1,"url":"https://patchwork.libcamera.org/api/people/1/","name":"Umang Jain","email":"email@uajain.com"},"content":"Hi,\n\nOn 10/16/20 12:02 PM, Laurent Pinchart wrote:\n> Hi Umang,\n>\n> Thank you for the patch.\n>\n> On Fri, Oct 16, 2020 at 11:07:54AM +0530, Umang Jain wrote:\n>> Port the CameraStream's JPEG-encoding bits to PostProcessorJpeg.\n>> This encapsulates the encoder and EXIF generation code into the\n>> PostProcessorJpeg layer and removes these specifics related to JPEG,\n>> from the CameraStream itself.\n>>\n>> Signed-off-by: Umang Jain \n>> Tested-by: Kieran Bingham \n>> Reviewed-by: Kieran Bingham \n> Reviewed-by: Laurent Pinchart \n>\n> Kieran, would you be able to test this new version, and push it if all\n> looks good to you ?\nYes please, one final testing should be done before actual pushing.\nThanks for the reviews. Thanks to Kieran for removing blockers on this\nand testing efforts :)\n>\n>> ---\n>> src/android/camera_device.cpp | 1 +\n>> src/android/camera_stream.cpp | 77 ++++-------------\n>> src/android/camera_stream.h | 4 +-\n>> src/android/jpeg/encoder_libjpeg.cpp | 2 +-\n>> src/android/jpeg/post_processor_jpeg.cpp | 105 +++++++++++++++++++++++\n>> src/android/jpeg/post_processor_jpeg.h | 36 ++++++++\n>> src/android/meson.build | 1 +\n>> 7 files changed, 164 insertions(+), 62 deletions(-)\n>> create mode 100644 src/android/jpeg/post_processor_jpeg.cpp\n>> create mode 100644 src/android/jpeg/post_processor_jpeg.h\n>>\n>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n>> index 0983232..d706cf4 100644\n>> --- a/src/android/camera_device.cpp\n>> +++ b/src/android/camera_device.cpp\n>> @@ -7,6 +7,7 @@\n>> \n>> #include \"camera_device.h\"\n>> #include \"camera_ops.h\"\n>> +#include \"post_processor.h\"\n>> \n>> #include \n>> #include \n>> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n>> index d8e45c2..1b8afa8 100644\n>> --- a/src/android/camera_stream.cpp\n>> +++ b/src/android/camera_stream.cpp\n>> @@ -9,9 +9,9 @@\n>> \n>> #include \"camera_device.h\"\n>> #include \"camera_metadata.h\"\n>> -#include \"jpeg/encoder.h\"\n>> -#include \"jpeg/encoder_libjpeg.h\"\n>> -#include \"jpeg/exif.h\"\n>> +#include \"jpeg/post_processor_jpeg.h\"\n>> +\n>> +#include \n>> \n>> using namespace libcamera;\n>> \n>> @@ -45,8 +45,15 @@ CameraStream::CameraStream(CameraDevice *cameraDevice, Type type,\n>> {\n>> \tconfig_ = cameraDevice_->cameraConfiguration();\n>> \n>> -\tif (type_ == Type::Internal || type_ == Type::Mapped)\n>> -\t\tencoder_ = std::make_unique();\n>> +\tif (type_ == Type::Internal || type_ == Type::Mapped) {\n>> +\t\t/*\n>> +\t\t * \\todo There might be multiple post-processors. The logic\n>> +\t\t * which should be instantiated here, is deferred for the\n>> +\t\t * future. For now, we only have PostProcessorJpeg and that\n>> +\t\t * is what we instantiate here.\n>> +\t\t */\n>> +\t\tpostProcessor_ = std::make_unique(cameraDevice_);\n>> +\t}\n>> \n>> \tif (type == Type::Internal) {\n>> \t\tallocator_ = std::make_unique(cameraDevice_->camera());\n>> @@ -66,8 +73,10 @@ Stream *CameraStream::stream() const\n>> \n>> int CameraStream::configure()\n>> {\n>> -\tif (encoder_) {\n>> -\t\tint ret = encoder_->configure(configuration());\n>> +\tif (postProcessor_) {\n>> +\t\tStreamConfiguration output = configuration();\n>> +\t\toutput.pixelFormat = formats::MJPEG;\n>> +\t\tint ret = postProcessor_->configure(configuration(), output);\n>> \t\tif (ret)\n>> \t\t\treturn ret;\n>> \t}\n>> @@ -90,60 +99,10 @@ int CameraStream::configure()\n>> int CameraStream::process(const libcamera::FrameBuffer &source,\n>> \t\t\t MappedCamera3Buffer *dest, CameraMetadata *metadata)\n>> {\n>> -\tif (!encoder_)\n>> +\tif (!postProcessor_)\n>> \t\treturn 0;\n>> \n>> -\t/* Set EXIF metadata for various tags. */\n>> -\tExif exif;\n>> -\t/* \\todo Set Make and Model from external vendor tags. */\n>> -\texif.setMake(\"libcamera\");\n>> -\texif.setModel(\"cameraModel\");\n>> -\texif.setOrientation(cameraDevice_->orientation());\n>> -\texif.setSize(configuration().size);\n>> -\t/*\n>> -\t * We set the frame's EXIF timestamp as the time of encode.\n>> -\t * Since the precision we need for EXIF timestamp is only one\n>> -\t * second, it is good enough.\n>> -\t */\n>> -\texif.setTimestamp(std::time(nullptr));\n>> -\tif (exif.generate() != 0)\n>> -\t\tLOG(HAL, Error) << \"Failed to generate valid EXIF data\";\n>> -\n>> -\tint jpeg_size = encoder_->encode(&source, dest->maps()[0], exif.data());\n>> -\tif (jpeg_size < 0) {\n>> -\t\tLOG(HAL, Error) << \"Failed to encode stream image\";\n>> -\t\treturn jpeg_size;\n>> -\t}\n>> -\n>> -\t/*\n>> -\t * Fill in the JPEG blob header.\n>> -\t *\n>> -\t * The mapped size of the buffer is being returned as\n>> -\t * substantially larger than the requested JPEG_MAX_SIZE\n>> -\t * (which is referenced from maxJpegBufferSize_). Utilise\n>> -\t * this static size to ensure the correct offset of the blob is\n>> -\t * determined.\n>> -\t *\n>> -\t * \\todo Investigate if the buffer size mismatch is an issue or\n>> -\t * expected behaviour.\n>> -\t */\n>> -\tuint8_t *resultPtr = dest->maps()[0].data() +\n>> -\t\t\t cameraDevice_->maxJpegBufferSize() -\n>> -\t\t\t sizeof(struct camera3_jpeg_blob);\n>> -\tauto *blob = reinterpret_cast(resultPtr);\n>> -\tblob->jpeg_blob_id = CAMERA3_JPEG_BLOB_ID;\n>> -\tblob->jpeg_size = jpeg_size;\n>> -\n>> -\t/* Update the JPEG result Metadata. */\n>> -\tmetadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1);\n>> -\n>> -\tconst uint32_t jpeg_quality = 95;\n>> -\tmetadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 1);\n>> -\n>> -\tconst uint32_t jpeg_orientation = 0;\n>> -\tmetadata->addEntry(ANDROID_JPEG_ORIENTATION, &jpeg_orientation, 1);\n>> -\n>> -\treturn 0;\n>> +\treturn postProcessor_->process(&source, dest->maps()[0], metadata);\n>> }\n>> \n>> FrameBuffer *CameraStream::getBuffer()\n>> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n>> index d3925fb..c367a5f 100644\n>> --- a/src/android/camera_stream.h\n>> +++ b/src/android/camera_stream.h\n>> @@ -19,10 +19,10 @@\n>> #include \n>> #include \n>> \n>> -class Encoder;\n>> class CameraDevice;\n>> class CameraMetadata;\n>> class MappedCamera3Buffer;\n>> +class PostProcessor;\n>> \n>> class CameraStream\n>> {\n>> @@ -130,7 +130,6 @@ private:\n>> \tcamera3_stream_t *camera3Stream_;\n>> \tunsigned int index_;\n>> \n>> -\tstd::unique_ptr encoder_;\n>> \tstd::unique_ptr allocator_;\n>> \tstd::vector buffers_;\n>> \t/*\n>> @@ -138,6 +137,7 @@ private:\n>> \t * an std::vector in CameraDevice.\n>> \t */\n>> \tstd::unique_ptr mutex_;\n>> +\tstd::unique_ptr postProcessor_;\n>> };\n>> \n>> #endif /* __ANDROID_CAMERA_STREAM__ */\n>> diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp\n>> index a77f5b2..8995ba5 100644\n>> --- a/src/android/jpeg/encoder_libjpeg.cpp\n>> +++ b/src/android/jpeg/encoder_libjpeg.cpp\n>> @@ -25,7 +25,7 @@\n>> \n>> using namespace libcamera;\n>> \n>> -LOG_DEFINE_CATEGORY(JPEG)\n>> +LOG_DECLARE_CATEGORY(JPEG);\n>> \n>> namespace {\n>> \n>> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp\n>> new file mode 100644\n>> index 0000000..753c28e\n>> --- /dev/null\n>> +++ b/src/android/jpeg/post_processor_jpeg.cpp\n>> @@ -0,0 +1,105 @@\n>> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n>> +/*\n>> + * Copyright (C) 2020, Google Inc.\n>> + *\n>> + * post_processor_jpeg.cpp - JPEG Post Processor\n>> + */\n>> +\n>> +#include \"post_processor_jpeg.h\"\n>> +\n>> +#include \"../camera_device.h\"\n>> +#include \"../camera_metadata.h\"\n>> +#include \"encoder_libjpeg.h\"\n>> +#include \"exif.h\"\n>> +\n>> +#include \n>> +\n>> +#include \"libcamera/internal/log.h\"\n>> +\n>> +using namespace libcamera;\n>> +\n>> +LOG_DEFINE_CATEGORY(JPEG);\n>> +\n>> +PostProcessorJpeg::PostProcessorJpeg(CameraDevice *device)\n>> +\t: cameraDevice_(device)\n>> +{\n>> +}\n>> +\n>> +int PostProcessorJpeg::configure(const StreamConfiguration &inCfg,\n>> +\t\t\t\t const StreamConfiguration &outCfg)\n>> +{\n>> +\tif (inCfg.size != outCfg.size) {\n>> +\t\tLOG(JPEG, Error) << \"Mismatch of input and output stream sizes\";\n>> +\t\treturn -EINVAL;\n>> +\t}\n>> +\n>> +\tif (outCfg.pixelFormat != formats::MJPEG) {\n>> +\t\tLOG(JPEG, Error) << \"Output stream pixel format is not JPEG\";\n>> +\t\treturn -EINVAL;\n>> +\t}\n>> +\n>> +\tstreamSize_ = outCfg.size;\n>> +\tencoder_ = std::make_unique();\n>> +\n>> +\treturn encoder_->configure(inCfg);\n>> +}\n>> +\n>> +int PostProcessorJpeg::process(const libcamera::FrameBuffer *source,\n>> +\t\t\t const libcamera::Span &destination,\n>> +\t\t\t CameraMetadata *metadata)\n>> +{\n>> +\tif (!encoder_)\n>> +\t\treturn 0;\n>> +\n>> +\t/* Set EXIF metadata for various tags. */\n>> +\tExif exif;\n>> +\t/* \\todo Set Make and Model from external vendor tags. */\n>> +\texif.setMake(\"libcamera\");\n>> +\texif.setModel(\"cameraModel\");\n>> +\texif.setOrientation(cameraDevice_->orientation());\n>> +\texif.setSize(streamSize_);\n>> +\t/*\n>> +\t * We set the frame's EXIF timestamp as the time of encode.\n>> +\t * Since the precision we need for EXIF timestamp is only one\n>> +\t * second, it is good enough.\n>> +\t */\n>> +\texif.setTimestamp(std::time(nullptr));\n>> +\tif (exif.generate() != 0)\n>> +\t\tLOG(JPEG, Error) << \"Failed to generate valid EXIF data\";\n>> +\n>> +\tint jpeg_size = encoder_->encode(source, destination, exif.data());\n>> +\tif (jpeg_size < 0) {\n>> +\t\tLOG(JPEG, Error) << \"Failed to encode stream image\";\n>> +\t\treturn jpeg_size;\n>> +\t}\n>> +\n>> +\t/*\n>> +\t * Fill in the JPEG blob header.\n>> +\t *\n>> +\t * The mapped size of the buffer is being returned as\n>> +\t * substantially larger than the requested JPEG_MAX_SIZE\n>> +\t * (which is referenced from maxJpegBufferSize_). Utilise\n>> +\t * this static size to ensure the correct offset of the blob is\n>> +\t * determined.\n>> +\t *\n>> +\t * \\todo Investigate if the buffer size mismatch is an issue or\n>> +\t * expected behaviour.\n>> +\t */\n>> +\tuint8_t *resultPtr = destination.data() +\n>> +\t\t\t cameraDevice_->maxJpegBufferSize() -\n>> +\t\t\t sizeof(struct camera3_jpeg_blob);\n>> +\tauto *blob = reinterpret_cast(resultPtr);\n>> +\tblob->jpeg_blob_id = CAMERA3_JPEG_BLOB_ID;\n>> +\tblob->jpeg_size = jpeg_size;\n>> +\n>> +\t/* Update the JPEG result Metadata. */\n>> +\tmetadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1);\n>> +\n>> +\tconst uint32_t jpeg_quality = 95;\n>> +\tmetadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 1);\n>> +\n>> +\tconst uint32_t jpeg_orientation = 0;\n>> +\tmetadata->addEntry(ANDROID_JPEG_ORIENTATION, &jpeg_orientation, 1);\n>> +\n>> +\treturn 0;\n>> +}\n>> diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h\n>> new file mode 100644\n>> index 0000000..62c8650\n>> --- /dev/null\n>> +++ b/src/android/jpeg/post_processor_jpeg.h\n>> @@ -0,0 +1,36 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2020, Google Inc.\n>> + *\n>> + * post_processor_jpeg.h - JPEG Post Processor\n>> + */\n>> +#ifndef __ANDROID_POST_PROCESSOR_JPEG_H__\n>> +#define __ANDROID_POST_PROCESSOR_JPEG_H__\n>> +\n>> +#include \"../post_processor.h\"\n>> +\n>> +#include \n>> +\n>> +#include \"libcamera/internal/buffer.h\"\n>> +\n>> +class Encoder;\n>> +class CameraDevice;\n>> +\n>> +class PostProcessorJpeg : public PostProcessor\n>> +{\n>> +public:\n>> +\tPostProcessorJpeg(CameraDevice *device);\n>> +\n>> +\tint configure(const libcamera::StreamConfiguration &incfg,\n>> +\t\t const libcamera::StreamConfiguration &outcfg) override;\n>> +\tint process(const libcamera::FrameBuffer *source,\n>> +\t\t const libcamera::Span &destination,\n>> +\t\t CameraMetadata *metadata) override;\n>> +\n>> +private:\n>> +\tCameraDevice *cameraDevice_;\n>> +\tstd::unique_ptr encoder_;\n>> +\tlibcamera::Size streamSize_;\n>> +};\n>> +\n>> +#endif /* __ANDROID_POST_PROCESSOR_JPEG_H__ */\n>> diff --git a/src/android/meson.build b/src/android/meson.build\n>> index b2b2293..5a01bea 100644\n>> --- a/src/android/meson.build\n>> +++ b/src/android/meson.build\n>> @@ -24,6 +24,7 @@ android_hal_sources = files([\n>> 'camera_worker.cpp',\n>> 'jpeg/encoder_libjpeg.cpp',\n>> 'jpeg/exif.cpp',\n>> + 'jpeg/post_processor_jpeg.cpp',\n>> ])\n>> \n>> android_camera_metadata_sources = files([","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id C3725BDB1F\n\tfor ;\n\tFri, 16 Oct 2020 07:15:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4B6B960530;\n\tFri, 16 Oct 2020 09:15:27 +0200 (CEST)","from mail.uajain.com (static.126.159.217.95.clients.your-server.de\n\t[95.217.159.126])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D545160530\n\tfor ;\n\tFri, 16 Oct 2020 09:15:25 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=uajain.com header.i=@uajain.com\n\theader.b=\"faKett/A\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=uajain.com; s=mail;\n\tt=1602832525; bh=S/eJjtxisqciyqlJ4jlg1lkmvi422wvH2XvKxYphtKc=;\n\th=Subject:To:Cc:References:From:In-Reply-To;\n\tb=faKett/AovZ9GuqkFcgx8mGjH+TPWoaF6+fnnjcWrtlfJo69poq7atmCZ32xyJk2M\n\t1DezD67FXm6QWgPL/bZIdj+P/3NRlMUL4M9WKm2tx7B5XtqQSOPWUJpQcsoy3jv3vN\n\tzLQVqvIrccRAO7u8P04GkJUSCyOhwsLlrTsepHTWStm5bQMUYupE8uSVKPE61atvJd\n\tRMS2LmJprSQDCodsuvA9W07QDyDgHcpPQwVDvrOsnNvv7b3kMAnVlPg1yMaD6daes6\n\tMVh+aIBV1IoJfVDQyikQCgF4fC83yP9PUnB5qJLMcjqk84F7+ArF3npUY0G7tzVlqq\n\tuLKXZN4jqNwow==","To":"Laurent Pinchart ","References":"<20201016053754.17251-1-email@uajain.com>\n\t<20201016053754.17251-3-email@uajain.com>\n\t<20201016063253.GK3829@pendragon.ideasonboard.com>","From":"Umang Jain ","Message-ID":"<617edcee-9cbc-e3ac-8c82-8ebbc68b3f28@uajain.com>","Date":"Fri, 16 Oct 2020 12:45:20 +0530","Mime-Version":"1.0","In-Reply-To":"<20201016063253.GK3829@pendragon.ideasonboard.com>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] android: jpeg: Port to\n\tPostProcessor interface","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Transfer-Encoding":"7bit","Content-Type":"text/plain; charset=\"us-ascii\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13239,"web_url":"https://patchwork.libcamera.org/comment/13239/","msgid":"<92d54fd6-5c90-29bb-dd6c-2f9c3a7ccebd@ideasonboard.com>","date":"2020-10-16T10:25:13","subject":"Re: [libcamera-devel] [PATCH v2 2/2] android: jpeg: Port to\n\tPostProcessor interface","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Umang, Laurent,\n\nOn 16/10/2020 08:15, Umang Jain wrote:\n> Hi,\n> \n> On 10/16/20 12:02 PM, Laurent Pinchart wrote:\n>> Hi Umang,\n>>\n>> Thank you for the patch.\n>>\n>> On Fri, Oct 16, 2020 at 11:07:54AM +0530, Umang Jain wrote:\n>>> Port the CameraStream's JPEG-encoding bits to PostProcessorJpeg.\n>>> This encapsulates the encoder and EXIF generation code into the\n>>> PostProcessorJpeg layer and removes these specifics related to JPEG,\n>>> from the CameraStream itself.\n>>>\n>>> Signed-off-by: Umang Jain \n>>> Tested-by: Kieran Bingham \n>>> Reviewed-by: Kieran Bingham \n>> Reviewed-by: Laurent Pinchart \n>>\n>> Kieran, would you be able to test this new version, and push it if all\n>> looks good to you ?\n> Yes please, one final testing should be done before actual pushing.\n> Thanks for the reviews. Thanks to Kieran for removing blockers on this\n> and testing efforts :)\n\nOk - I had a few hit and misses due to apparent hardware failure (the\ncamera didn't power up, now up with a full power cycle) - and then the\ncros build not actually building the code.\n\nall of that is solved, and confirmed that it's definitely Umang's code\nrunning on the device ... successfully ;-)\n\nI'll push these patches now.\n\nThanks\n\nKieran\n\n\n>>\n>>> ---\n>>>   src/android/camera_device.cpp            |   1 +\n>>>   src/android/camera_stream.cpp            |  77 ++++-------------\n>>>   src/android/camera_stream.h              |   4 +-\n>>>   src/android/jpeg/encoder_libjpeg.cpp     |   2 +-\n>>>   src/android/jpeg/post_processor_jpeg.cpp | 105 +++++++++++++++++++++++\n>>>   src/android/jpeg/post_processor_jpeg.h   |  36 ++++++++\n>>>   src/android/meson.build                  |   1 +\n>>>   7 files changed, 164 insertions(+), 62 deletions(-)\n>>>   create mode 100644 src/android/jpeg/post_processor_jpeg.cpp\n>>>   create mode 100644 src/android/jpeg/post_processor_jpeg.h\n>>>\n>>> diff --git a/src/android/camera_device.cpp\n>>> b/src/android/camera_device.cpp\n>>> index 0983232..d706cf4 100644\n>>> --- a/src/android/camera_device.cpp\n>>> +++ b/src/android/camera_device.cpp\n>>> @@ -7,6 +7,7 @@\n>>>     #include \"camera_device.h\"\n>>>   #include \"camera_ops.h\"\n>>> +#include \"post_processor.h\"\n>>>     #include \n>>>   #include \n>>> diff --git a/src/android/camera_stream.cpp\n>>> b/src/android/camera_stream.cpp\n>>> index d8e45c2..1b8afa8 100644\n>>> --- a/src/android/camera_stream.cpp\n>>> +++ b/src/android/camera_stream.cpp\n>>> @@ -9,9 +9,9 @@\n>>>     #include \"camera_device.h\"\n>>>   #include \"camera_metadata.h\"\n>>> -#include \"jpeg/encoder.h\"\n>>> -#include \"jpeg/encoder_libjpeg.h\"\n>>> -#include \"jpeg/exif.h\"\n>>> +#include \"jpeg/post_processor_jpeg.h\"\n>>> +\n>>> +#include \n>>>     using namespace libcamera;\n>>>   @@ -45,8 +45,15 @@ CameraStream::CameraStream(CameraDevice\n>>> *cameraDevice, Type type,\n>>>   {\n>>>       config_ = cameraDevice_->cameraConfiguration();\n>>>   -    if (type_ == Type::Internal || type_ == Type::Mapped)\n>>> -        encoder_ = std::make_unique();\n>>> +    if (type_ == Type::Internal || type_ == Type::Mapped) {\n>>> +        /*\n>>> +         * \\todo There might be multiple post-processors. The logic\n>>> +         * which should be instantiated here, is deferred for the\n>>> +         * future. For now, we only have PostProcessorJpeg and that\n>>> +         * is what we instantiate here.\n>>> +         */\n>>> +        postProcessor_ =\n>>> std::make_unique(cameraDevice_);\n>>> +    }\n>>>         if (type == Type::Internal) {\n>>>           allocator_ =\n>>> std::make_unique(cameraDevice_->camera());\n>>> @@ -66,8 +73,10 @@ Stream *CameraStream::stream() const\n>>>     int CameraStream::configure()\n>>>   {\n>>> -    if (encoder_) {\n>>> -        int ret = encoder_->configure(configuration());\n>>> +    if (postProcessor_) {\n>>> +        StreamConfiguration output = configuration();\n>>> +        output.pixelFormat = formats::MJPEG;\n>>> +        int ret = postProcessor_->configure(configuration(), output);\n>>>           if (ret)\n>>>               return ret;\n>>>       }\n>>> @@ -90,60 +99,10 @@ int CameraStream::configure()\n>>>   int CameraStream::process(const libcamera::FrameBuffer &source,\n>>>                 MappedCamera3Buffer *dest, CameraMetadata *metadata)\n>>>   {\n>>> -    if (!encoder_)\n>>> +    if (!postProcessor_)\n>>>           return 0;\n>>>   -    /* Set EXIF metadata for various tags. */\n>>> -    Exif exif;\n>>> -    /* \\todo Set Make and Model from external vendor tags. */\n>>> -    exif.setMake(\"libcamera\");\n>>> -    exif.setModel(\"cameraModel\");\n>>> -    exif.setOrientation(cameraDevice_->orientation());\n>>> -    exif.setSize(configuration().size);\n>>> -    /*\n>>> -     * We set the frame's EXIF timestamp as the time of encode.\n>>> -     * Since the precision we need for EXIF timestamp is only one\n>>> -     * second, it is good enough.\n>>> -     */\n>>> -    exif.setTimestamp(std::time(nullptr));\n>>> -    if (exif.generate() != 0)\n>>> -        LOG(HAL, Error) << \"Failed to generate valid EXIF data\";\n>>> -\n>>> -    int jpeg_size = encoder_->encode(&source, dest->maps()[0],\n>>> exif.data());\n>>> -    if (jpeg_size < 0) {\n>>> -        LOG(HAL, Error) << \"Failed to encode stream image\";\n>>> -        return jpeg_size;\n>>> -    }\n>>> -\n>>> -    /*\n>>> -     * Fill in the JPEG blob header.\n>>> -     *\n>>> -     * The mapped size of the buffer is being returned as\n>>> -     * substantially larger than the requested JPEG_MAX_SIZE\n>>> -     * (which is referenced from maxJpegBufferSize_). Utilise\n>>> -     * this static size to ensure the correct offset of the blob is\n>>> -     * determined.\n>>> -     *\n>>> -     * \\todo Investigate if the buffer size mismatch is an issue or\n>>> -     * expected behaviour.\n>>> -     */\n>>> -    uint8_t *resultPtr = dest->maps()[0].data() +\n>>> -                 cameraDevice_->maxJpegBufferSize() -\n>>> -                 sizeof(struct camera3_jpeg_blob);\n>>> -    auto *blob = reinterpret_cast>> *>(resultPtr);\n>>> -    blob->jpeg_blob_id = CAMERA3_JPEG_BLOB_ID;\n>>> -    blob->jpeg_size = jpeg_size;\n>>> -\n>>> -    /* Update the JPEG result Metadata. */\n>>> -    metadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1);\n>>> -\n>>> -    const uint32_t jpeg_quality = 95;\n>>> -    metadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 1);\n>>> -\n>>> -    const uint32_t jpeg_orientation = 0;\n>>> -    metadata->addEntry(ANDROID_JPEG_ORIENTATION, &jpeg_orientation, 1);\n>>> -\n>>> -    return 0;\n>>> +    return postProcessor_->process(&source, dest->maps()[0], metadata);\n>>>   }\n>>>     FrameBuffer *CameraStream::getBuffer()\n>>> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n>>> index d3925fb..c367a5f 100644\n>>> --- a/src/android/camera_stream.h\n>>> +++ b/src/android/camera_stream.h\n>>> @@ -19,10 +19,10 @@\n>>>   #include \n>>>   #include \n>>>   -class Encoder;\n>>>   class CameraDevice;\n>>>   class CameraMetadata;\n>>>   class MappedCamera3Buffer;\n>>> +class PostProcessor;\n>>>     class CameraStream\n>>>   {\n>>> @@ -130,7 +130,6 @@ private:\n>>>       camera3_stream_t *camera3Stream_;\n>>>       unsigned int index_;\n>>>   -    std::unique_ptr encoder_;\n>>>       std::unique_ptr allocator_;\n>>>       std::vector buffers_;\n>>>       /*\n>>> @@ -138,6 +137,7 @@ private:\n>>>        * an std::vector in CameraDevice.\n>>>        */\n>>>       std::unique_ptr mutex_;\n>>> +    std::unique_ptr postProcessor_;\n>>>   };\n>>>     #endif /* __ANDROID_CAMERA_STREAM__ */\n>>> diff --git a/src/android/jpeg/encoder_libjpeg.cpp\n>>> b/src/android/jpeg/encoder_libjpeg.cpp\n>>> index a77f5b2..8995ba5 100644\n>>> --- a/src/android/jpeg/encoder_libjpeg.cpp\n>>> +++ b/src/android/jpeg/encoder_libjpeg.cpp\n>>> @@ -25,7 +25,7 @@\n>>>     using namespace libcamera;\n>>>   -LOG_DEFINE_CATEGORY(JPEG)\n>>> +LOG_DECLARE_CATEGORY(JPEG);\n>>>     namespace {\n>>>   diff --git a/src/android/jpeg/post_processor_jpeg.cpp\n>>> b/src/android/jpeg/post_processor_jpeg.cpp\n>>> new file mode 100644\n>>> index 0000000..753c28e\n>>> --- /dev/null\n>>> +++ b/src/android/jpeg/post_processor_jpeg.cpp\n>>> @@ -0,0 +1,105 @@\n>>> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n>>> +/*\n>>> + * Copyright (C) 2020, Google Inc.\n>>> + *\n>>> + * post_processor_jpeg.cpp - JPEG Post Processor\n>>> + */\n>>> +\n>>> +#include \"post_processor_jpeg.h\"\n>>> +\n>>> +#include \"../camera_device.h\"\n>>> +#include \"../camera_metadata.h\"\n>>> +#include \"encoder_libjpeg.h\"\n>>> +#include \"exif.h\"\n>>> +\n>>> +#include \n>>> +\n>>> +#include \"libcamera/internal/log.h\"\n>>> +\n>>> +using namespace libcamera;\n>>> +\n>>> +LOG_DEFINE_CATEGORY(JPEG);\n>>> +\n>>> +PostProcessorJpeg::PostProcessorJpeg(CameraDevice *device)\n>>> +    : cameraDevice_(device)\n>>> +{\n>>> +}\n>>> +\n>>> +int PostProcessorJpeg::configure(const StreamConfiguration &inCfg,\n>>> +                 const StreamConfiguration &outCfg)\n>>> +{\n>>> +    if (inCfg.size != outCfg.size) {\n>>> +        LOG(JPEG, Error) << \"Mismatch of input and output stream\n>>> sizes\";\n>>> +        return -EINVAL;\n>>> +    }\n>>> +\n>>> +    if (outCfg.pixelFormat != formats::MJPEG) {\n>>> +        LOG(JPEG, Error) << \"Output stream pixel format is not JPEG\";\n>>> +        return -EINVAL;\n>>> +    }\n>>> +\n>>> +    streamSize_ = outCfg.size;\n>>> +    encoder_ = std::make_unique();\n>>> +\n>>> +    return encoder_->configure(inCfg);\n>>> +}\n>>> +\n>>> +int PostProcessorJpeg::process(const libcamera::FrameBuffer *source,\n>>> +                   const libcamera::Span &destination,\n>>> +                   CameraMetadata *metadata)\n>>> +{\n>>> +    if (!encoder_)\n>>> +        return 0;\n>>> +\n>>> +    /* Set EXIF metadata for various tags. */\n>>> +    Exif exif;\n>>> +    /* \\todo Set Make and Model from external vendor tags. */\n>>> +    exif.setMake(\"libcamera\");\n>>> +    exif.setModel(\"cameraModel\");\n>>> +    exif.setOrientation(cameraDevice_->orientation());\n>>> +    exif.setSize(streamSize_);\n>>> +    /*\n>>> +     * We set the frame's EXIF timestamp as the time of encode.\n>>> +     * Since the precision we need for EXIF timestamp is only one\n>>> +     * second, it is good enough.\n>>> +     */\n>>> +    exif.setTimestamp(std::time(nullptr));\n>>> +    if (exif.generate() != 0)\n>>> +        LOG(JPEG, Error) << \"Failed to generate valid EXIF data\";\n>>> +\n>>> +    int jpeg_size = encoder_->encode(source, destination, exif.data());\n>>> +    if (jpeg_size < 0) {\n>>> +        LOG(JPEG, Error) << \"Failed to encode stream image\";\n>>> +        return jpeg_size;\n>>> +    }\n>>> +\n>>> +    /*\n>>> +     * Fill in the JPEG blob header.\n>>> +     *\n>>> +     * The mapped size of the buffer is being returned as\n>>> +     * substantially larger than the requested JPEG_MAX_SIZE\n>>> +     * (which is referenced from maxJpegBufferSize_). Utilise\n>>> +     * this static size to ensure the correct offset of the blob is\n>>> +     * determined.\n>>> +     *\n>>> +     * \\todo Investigate if the buffer size mismatch is an issue or\n>>> +     * expected behaviour.\n>>> +     */\n>>> +    uint8_t *resultPtr = destination.data() +\n>>> +                 cameraDevice_->maxJpegBufferSize() -\n>>> +                 sizeof(struct camera3_jpeg_blob);\n>>> +    auto *blob = reinterpret_cast>> *>(resultPtr);\n>>> +    blob->jpeg_blob_id = CAMERA3_JPEG_BLOB_ID;\n>>> +    blob->jpeg_size = jpeg_size;\n>>> +\n>>> +    /* Update the JPEG result Metadata. */\n>>> +    metadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1);\n>>> +\n>>> +    const uint32_t jpeg_quality = 95;\n>>> +    metadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 1);\n>>> +\n>>> +    const uint32_t jpeg_orientation = 0;\n>>> +    metadata->addEntry(ANDROID_JPEG_ORIENTATION, &jpeg_orientation, 1);\n>>> +\n>>> +    return 0;\n>>> +}\n>>> diff --git a/src/android/jpeg/post_processor_jpeg.h\n>>> b/src/android/jpeg/post_processor_jpeg.h\n>>> new file mode 100644\n>>> index 0000000..62c8650\n>>> --- /dev/null\n>>> +++ b/src/android/jpeg/post_processor_jpeg.h\n>>> @@ -0,0 +1,36 @@\n>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>> +/*\n>>> + * Copyright (C) 2020, Google Inc.\n>>> + *\n>>> + * post_processor_jpeg.h - JPEG Post Processor\n>>> + */\n>>> +#ifndef __ANDROID_POST_PROCESSOR_JPEG_H__\n>>> +#define __ANDROID_POST_PROCESSOR_JPEG_H__\n>>> +\n>>> +#include \"../post_processor.h\"\n>>> +\n>>> +#include \n>>> +\n>>> +#include \"libcamera/internal/buffer.h\"\n>>> +\n>>> +class Encoder;\n>>> +class CameraDevice;\n>>> +\n>>> +class PostProcessorJpeg : public PostProcessor\n>>> +{\n>>> +public:\n>>> +    PostProcessorJpeg(CameraDevice *device);\n>>> +\n>>> +    int configure(const libcamera::StreamConfiguration &incfg,\n>>> +              const libcamera::StreamConfiguration &outcfg) override;\n>>> +    int process(const libcamera::FrameBuffer *source,\n>>> +            const libcamera::Span &destination,\n>>> +            CameraMetadata *metadata) override;\n>>> +\n>>> +private:\n>>> +    CameraDevice *cameraDevice_;\n>>> +    std::unique_ptr encoder_;\n>>> +    libcamera::Size streamSize_;\n>>> +};\n>>> +\n>>> +#endif /* __ANDROID_POST_PROCESSOR_JPEG_H__ */\n>>> diff --git a/src/android/meson.build b/src/android/meson.build\n>>> index b2b2293..5a01bea 100644\n>>> --- a/src/android/meson.build\n>>> +++ b/src/android/meson.build\n>>> @@ -24,6 +24,7 @@ android_hal_sources = files([\n>>>       'camera_worker.cpp',\n>>>       'jpeg/encoder_libjpeg.cpp',\n>>>       'jpeg/exif.cpp',\n>>> +    'jpeg/post_processor_jpeg.cpp',\n>>>   ])\n>>>     android_camera_metadata_sources = files([\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id E68DBBDB13\n\tfor ;\n\tFri, 16 Oct 2020 10:25:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6E39260CE4;\n\tFri, 16 Oct 2020 12:25:18 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E026260530\n\tfor ;\n\tFri, 16 Oct 2020 12:25:16 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AE399528;\n\tFri, 16 Oct 2020 12:25:15 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"RThb+3j/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1602843916;\n\tbh=3tvJ3lb6lfRECwV2eaJbODt+OM7OoFYM7Fa213uHP+I=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=RThb+3j/yFGipI1qWwwmnMm9Sq9HCMz9Dsj7EdERsRkbyO3s3Jff8+v261W7qQoyB\n\tqP4SpvvSyeJ2NhLrKINUeZS2SJtC4fecMkKhJmYL9m+70r1ig0s6IsdPQ/z5YaeCpH\n\tgSKjJdBzjrPaZz3/2zvmhC1jNADgsIkvjMVVHdZE=","To":"Umang Jain ,\n\tLaurent Pinchart ","References":"<20201016053754.17251-1-email@uajain.com>\n\t<20201016053754.17251-3-email@uajain.com>\n\t<20201016063253.GK3829@pendragon.ideasonboard.com>\n\t<617edcee-9cbc-e3ac-8c82-8ebbc68b3f28@uajain.com>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<92d54fd6-5c90-29bb-dd6c-2f9c3a7ccebd@ideasonboard.com>","Date":"Fri, 16 Oct 2020 11:25:13 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<617edcee-9cbc-e3ac-8c82-8ebbc68b3f28@uajain.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] android: jpeg: Port to\n\tPostProcessor interface","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13244,"web_url":"https://patchwork.libcamera.org/comment/13244/","msgid":"","date":"2020-10-19T05:43:43","subject":"Re: [libcamera-devel] [PATCH v2 2/2] android: jpeg: Port to\n\tPostProcessor interface","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"On Fri, Oct 16, 2020 at 7:25 PM Kieran Bingham\n wrote:\n>\n> Hi Umang, Laurent,\n>\n> On 16/10/2020 08:15, Umang Jain wrote:\n> > Hi,\n> >\n> > On 10/16/20 12:02 PM, Laurent Pinchart wrote:\n> >> Hi Umang,\n> >>\n> >> Thank you for the patch.\n> >>\n> >> On Fri, Oct 16, 2020 at 11:07:54AM +0530, Umang Jain wrote:\n> >>> Port the CameraStream's JPEG-encoding bits to PostProcessorJpeg.\n> >>> This encapsulates the encoder and EXIF generation code into the\n> >>> PostProcessorJpeg layer and removes these specifics related to JPEG,\n> >>> from the CameraStream itself.\n> >>>\n> >>> Signed-off-by: Umang Jain \n> >>> Tested-by: Kieran Bingham \n> >>> Reviewed-by: Kieran Bingham \n> >> Reviewed-by: Laurent Pinchart \n> >>\n> >> Kieran, would you be able to test this new version, and push it if all\n> >> looks good to you ?\n> > Yes please, one final testing should be done before actual pushing.\n> > Thanks for the reviews. Thanks to Kieran for removing blockers on this\n> > and testing efforts :)\n>\n> Ok - I had a few hit and misses due to apparent hardware failure (the\n> camera didn't power up, now up with a full power cycle) - and then the\n> cros build not actually building the code.\n>\n> all of that is solved, and confirmed that it's definitely Umang's code\n> running on the device ... successfully ;-)\n>\n> I'll push these patches now.\n>\n> Thanks\n>\n> Kieran\n>\n>\n> >>\n> >>> ---\n> >>> src/android/camera_device.cpp | 1 +\n> >>> src/android/camera_stream.cpp | 77 ++++-------------\n> >>> src/android/camera_stream.h | 4 +-\n> >>> src/android/jpeg/encoder_libjpeg.cpp | 2 +-\n> >>> src/android/jpeg/post_processor_jpeg.cpp | 105 +++++++++++++++++++++++\n> >>> src/android/jpeg/post_processor_jpeg.h | 36 ++++++++\n> >>> src/android/meson.build | 1 +\n> >>> 7 files changed, 164 insertions(+), 62 deletions(-)\n> >>> create mode 100644 src/android/jpeg/post_processor_jpeg.cpp\n> >>> create mode 100644 src/android/jpeg/post_processor_jpeg.h\n> >>>\n> >>> diff --git a/src/android/camera_device.cpp\n> >>> b/src/android/camera_device.cpp\n> >>> index 0983232..d706cf4 100644\n> >>> --- a/src/android/camera_device.cpp\n> >>> +++ b/src/android/camera_device.cpp\n> >>> @@ -7,6 +7,7 @@\n> >>> #include \"camera_device.h\"\n> >>> #include \"camera_ops.h\"\n> >>> +#include \"post_processor.h\"\n> >>> #include \n> >>> #include \n> >>> diff --git a/src/android/camera_stream.cpp\n> >>> b/src/android/camera_stream.cpp\n> >>> index d8e45c2..1b8afa8 100644\n> >>> --- a/src/android/camera_stream.cpp\n> >>> +++ b/src/android/camera_stream.cpp\n> >>> @@ -9,9 +9,9 @@\n> >>> #include \"camera_device.h\"\n> >>> #include \"camera_metadata.h\"\n> >>> -#include \"jpeg/encoder.h\"\n> >>> -#include \"jpeg/encoder_libjpeg.h\"\n> >>> -#include \"jpeg/exif.h\"\n> >>> +#include \"jpeg/post_processor_jpeg.h\"\n> >>> +\n> >>> +#include \n> >>> using namespace libcamera;\n> >>> @@ -45,8 +45,15 @@ CameraStream::CameraStream(CameraDevice\n> >>> *cameraDevice, Type type,\n> >>> {\n> >>> config_ = cameraDevice_->cameraConfiguration();\n> >>> - if (type_ == Type::Internal || type_ == Type::Mapped)\n> >>> - encoder_ = std::make_unique();\n> >>> + if (type_ == Type::Internal || type_ == Type::Mapped) {\n> >>> + /*\n> >>> + * \\todo There might be multiple post-processors. The logic\n> >>> + * which should be instantiated here, is deferred for the\n> >>> + * future. For now, we only have PostProcessorJpeg and that\n> >>> + * is what we instantiate here.\n> >>> + */\n> >>> + postProcessor_ =\n> >>> std::make_unique(cameraDevice_);\n> >>> + }\n> >>> if (type == Type::Internal) {\n> >>> allocator_ =\n> >>> std::make_unique(cameraDevice_->camera());\n> >>> @@ -66,8 +73,10 @@ Stream *CameraStream::stream() const\n> >>> int CameraStream::configure()\n> >>> {\n> >>> - if (encoder_) {\n> >>> - int ret = encoder_->configure(configuration());\n> >>> + if (postProcessor_) {\n> >>> + StreamConfiguration output = configuration();\n> >>> + output.pixelFormat = formats::MJPEG;\n> >>> + int ret = postProcessor_->configure(configuration(), output);\n\nHmm, this is strange to me.\n|config_| should be used here and |Config::Size| should be set as well\nas Format?\n\n> >>> if (ret)\n> >>> return ret;\n> >>> }\n> >>> @@ -90,60 +99,10 @@ int CameraStream::configure()\n> >>> int CameraStream::process(const libcamera::FrameBuffer &source,\n> >>> MappedCamera3Buffer *dest, CameraMetadata *metadata)\n> >>> {\n> >>> - if (!encoder_)\n> >>> + if (!postProcessor_)\n> >>> return 0;\n> >>> - /* Set EXIF metadata for various tags. */\n> >>> - Exif exif;\n> >>> - /* \\todo Set Make and Model from external vendor tags. */\n> >>> - exif.setMake(\"libcamera\");\n> >>> - exif.setModel(\"cameraModel\");\n> >>> - exif.setOrientation(cameraDevice_->orientation());\n> >>> - exif.setSize(configuration().size);\n> >>> - /*\n> >>> - * We set the frame's EXIF timestamp as the time of encode.\n> >>> - * Since the precision we need for EXIF timestamp is only one\n> >>> - * second, it is good enough.\n> >>> - */\n> >>> - exif.setTimestamp(std::time(nullptr));\n> >>> - if (exif.generate() != 0)\n> >>> - LOG(HAL, Error) << \"Failed to generate valid EXIF data\";\n> >>> -\n> >>> - int jpeg_size = encoder_->encode(&source, dest->maps()[0],\n> >>> exif.data());\n> >>> - if (jpeg_size < 0) {\n> >>> - LOG(HAL, Error) << \"Failed to encode stream image\";\n> >>> - return jpeg_size;\n> >>> - }\n> >>> -\n> >>> - /*\n> >>> - * Fill in the JPEG blob header.\n> >>> - *\n> >>> - * The mapped size of the buffer is being returned as\n> >>> - * substantially larger than the requested JPEG_MAX_SIZE\n> >>> - * (which is referenced from maxJpegBufferSize_). Utilise\n> >>> - * this static size to ensure the correct offset of the blob is\n> >>> - * determined.\n> >>> - *\n> >>> - * \\todo Investigate if the buffer size mismatch is an issue or\n> >>> - * expected behaviour.\n> >>> - */\n> >>> - uint8_t *resultPtr = dest->maps()[0].data() +\n> >>> - cameraDevice_->maxJpegBufferSize() -\n> >>> - sizeof(struct camera3_jpeg_blob);\n> >>> - auto *blob = reinterpret_cast >>> *>(resultPtr);\n> >>> - blob->jpeg_blob_id = CAMERA3_JPEG_BLOB_ID;\n> >>> - blob->jpeg_size = jpeg_size;\n> >>> -\n> >>> - /* Update the JPEG result Metadata. */\n> >>> - metadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1);\n> >>> -\n> >>> - const uint32_t jpeg_quality = 95;\n> >>> - metadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 1);\n> >>> -\n> >>> - const uint32_t jpeg_orientation = 0;\n> >>> - metadata->addEntry(ANDROID_JPEG_ORIENTATION, &jpeg_orientation, 1);\n> >>> -\n> >>> - return 0;\n> >>> + return postProcessor_->process(&source, dest->maps()[0], metadata);\n> >>> }\n> >>> FrameBuffer *CameraStream::getBuffer()\n> >>> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n> >>> index d3925fb..c367a5f 100644\n> >>> --- a/src/android/camera_stream.h\n> >>> +++ b/src/android/camera_stream.h\n> >>> @@ -19,10 +19,10 @@\n> >>> #include \n> >>> #include \n> >>> -class Encoder;\n> >>> class CameraDevice;\n> >>> class CameraMetadata;\n> >>> class MappedCamera3Buffer;\n> >>> +class PostProcessor;\n> >>> class CameraStream\n> >>> {\n> >>> @@ -130,7 +130,6 @@ private:\n> >>> camera3_stream_t *camera3Stream_;\n> >>> unsigned int index_;\n> >>> - std::unique_ptr encoder_;\n> >>> std::unique_ptr allocator_;\n> >>> std::vector buffers_;\n> >>> /*\n> >>> @@ -138,6 +137,7 @@ private:\n> >>> * an std::vector in CameraDevice.\n> >>> */\n> >>> std::unique_ptr mutex_;\n> >>> + std::unique_ptr postProcessor_;\n> >>> };\n> >>> #endif /* __ANDROID_CAMERA_STREAM__ */\n> >>> diff --git a/src/android/jpeg/encoder_libjpeg.cpp\n> >>> b/src/android/jpeg/encoder_libjpeg.cpp\n> >>> index a77f5b2..8995ba5 100644\n> >>> --- a/src/android/jpeg/encoder_libjpeg.cpp\n> >>> +++ b/src/android/jpeg/encoder_libjpeg.cpp\n> >>> @@ -25,7 +25,7 @@\n> >>> using namespace libcamera;\n> >>> -LOG_DEFINE_CATEGORY(JPEG)\n> >>> +LOG_DECLARE_CATEGORY(JPEG);\n> >>> namespace {\n> >>> diff --git a/src/android/jpeg/post_processor_jpeg.cpp\n> >>> b/src/android/jpeg/post_processor_jpeg.cpp\n> >>> new file mode 100644\n> >>> index 0000000..753c28e\n> >>> --- /dev/null\n> >>> +++ b/src/android/jpeg/post_processor_jpeg.cpp\n> >>> @@ -0,0 +1,105 @@\n> >>> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> >>> +/*\n> >>> + * Copyright (C) 2020, Google Inc.\n> >>> + *\n> >>> + * post_processor_jpeg.cpp - JPEG Post Processor\n> >>> + */\n> >>> +\n> >>> +#include \"post_processor_jpeg.h\"\n> >>> +\n> >>> +#include \"../camera_device.h\"\n> >>> +#include \"../camera_metadata.h\"\n> >>> +#include \"encoder_libjpeg.h\"\n> >>> +#include \"exif.h\"\n> >>> +\n> >>> +#include \n> >>> +\n> >>> +#include \"libcamera/internal/log.h\"\n> >>> +\n> >>> +using namespace libcamera;\n> >>> +\n> >>> +LOG_DEFINE_CATEGORY(JPEG);\n> >>> +\n> >>> +PostProcessorJpeg::PostProcessorJpeg(CameraDevice *device)\n> >>> + : cameraDevice_(device)\n> >>> +{\n> >>> +}\n> >>> +\n> >>> +int PostProcessorJpeg::configure(const StreamConfiguration &inCfg,\n> >>> + const StreamConfiguration &outCfg)\n> >>> +{\n> >>> + if (inCfg.size != outCfg.size) {\n> >>> + LOG(JPEG, Error) << \"Mismatch of input and output stream\n> >>> sizes\";\n> >>> + return -EINVAL;\n> >>> + }\n> >>> +\n> >>> + if (outCfg.pixelFormat != formats::MJPEG) {\n> >>> + LOG(JPEG, Error) << \"Output stream pixel format is not JPEG\";\n> >>> + return -EINVAL;\n> >>> + }\n> >>> +\n> >>> + streamSize_ = outCfg.size;\n> >>> + encoder_ = std::make_unique();\n> >>> +\n> >>> + return encoder_->configure(inCfg);\n> >>> +}\n> >>> +\n> >>> +int PostProcessorJpeg::process(const libcamera::FrameBuffer *source,\n> >>> + const libcamera::Span &destination,\n> >>> + CameraMetadata *metadata)\n> >>> +{\n> >>> + if (!encoder_)\n> >>> + return 0;\n> >>> +\n> >>> + /* Set EXIF metadata for various tags. */\n> >>> + Exif exif;\n> >>> + /* \\todo Set Make and Model from external vendor tags. */\n> >>> + exif.setMake(\"libcamera\");\n> >>> + exif.setModel(\"cameraModel\");\n> >>> + exif.setOrientation(cameraDevice_->orientation());\n> >>> + exif.setSize(streamSize_);\n> >>> + /*\n> >>> + * We set the frame's EXIF timestamp as the time of encode.\n> >>> + * Since the precision we need for EXIF timestamp is only one\n> >>> + * second, it is good enough.\n> >>> + */\n> >>> + exif.setTimestamp(std::time(nullptr));\n> >>> + if (exif.generate() != 0)\n> >>> + LOG(JPEG, Error) << \"Failed to generate valid EXIF data\";\n\nShall we handle this case?\n\n> >>> +\n> >>> + int jpeg_size = encoder_->encode(source, destination, exif.data());\n> >>> + if (jpeg_size < 0) {\n> >>> + LOG(JPEG, Error) << \"Failed to encode stream image\";\n> >>> + return jpeg_size;\n> >>> + }\n> >>> +\n> >>> + /*\n> >>> + * Fill in the JPEG blob header.\n> >>> + *\n> >>> + * The mapped size of the buffer is being returned as\n> >>> + * substantially larger than the requested JPEG_MAX_SIZE\n> >>> + * (which is referenced from maxJpegBufferSize_). Utilise\n> >>> + * this static size to ensure the correct offset of the blob is\n> >>> + * determined.\n> >>> + *\n> >>> + * \\todo Investigate if the buffer size mismatch is an issue or\n> >>> + * expected behaviour.\n> >>> + */\n> >>> + uint8_t *resultPtr = destination.data() +\n> >>> + cameraDevice_->maxJpegBufferSize() -\n> >>> + sizeof(struct camera3_jpeg_blob);\n> >>> + auto *blob = reinterpret_cast >>> *>(resultPtr);\n> >>> + blob->jpeg_blob_id = CAMERA3_JPEG_BLOB_ID;\n> >>> + blob->jpeg_size = jpeg_size;\n> >>> +\n> >>> + /* Update the JPEG result Metadata. */\n> >>> + metadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1);\n> >>> +\n> >>> + const uint32_t jpeg_quality = 95;\n> >>> + metadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 1);\n> >>> +\n> >>> + const uint32_t jpeg_orientation = 0;\n> >>> + metadata->addEntry(ANDROID_JPEG_ORIENTATION, &jpeg_orientation, 1);\n> >>> +\n> >>> + return 0;\n> >>> +}\n> >>> diff --git a/src/android/jpeg/post_processor_jpeg.h\n> >>> b/src/android/jpeg/post_processor_jpeg.h\n> >>> new file mode 100644\n> >>> index 0000000..62c8650\n> >>> --- /dev/null\n> >>> +++ b/src/android/jpeg/post_processor_jpeg.h\n> >>> @@ -0,0 +1,36 @@\n> >>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >>> +/*\n> >>> + * Copyright (C) 2020, Google Inc.\n> >>> + *\n> >>> + * post_processor_jpeg.h - JPEG Post Processor\n> >>> + */\n> >>> +#ifndef __ANDROID_POST_PROCESSOR_JPEG_H__\n> >>> +#define __ANDROID_POST_PROCESSOR_JPEG_H__\n> >>> +\n> >>> +#include \"../post_processor.h\"\n> >>> +\n> >>> +#include \n> >>> +\n> >>> +#include \"libcamera/internal/buffer.h\"\n> >>> +\n> >>> +class Encoder;\n> >>> +class CameraDevice;\n> >>> +\n> >>> +class PostProcessorJpeg : public PostProcessor\n> >>> +{\n> >>> +public:\n> >>> + PostProcessorJpeg(CameraDevice *device);\n> >>> +\n> >>> + int configure(const libcamera::StreamConfiguration &incfg,\n> >>> + const libcamera::StreamConfiguration &outcfg) override;\n> >>> + int process(const libcamera::FrameBuffer *source,\n> >>> + const libcamera::Span &destination,\n> >>> + CameraMetadata *metadata) override;\n> >>> +\n> >>> +private:\n> >>> + CameraDevice *cameraDevice_;\n\nnit: CameraDevice const *cameraDevice_ to represent |cameraDevice_| is\nowned by a client.\n\n> >>> + std::unique_ptr encoder_;\n> >>> + libcamera::Size streamSize_;\n> >>> +};\n> >>> +\n> >>> +#endif /* __ANDROID_POST_PROCESSOR_JPEG_H__ */\n> >>> diff --git a/src/android/meson.build b/src/android/meson.build\n> >>> index b2b2293..5a01bea 100644\n> >>> --- a/src/android/meson.build\n> >>> +++ b/src/android/meson.build\n> >>> @@ -24,6 +24,7 @@ android_hal_sources = files([\n> >>> 'camera_worker.cpp',\n> >>> 'jpeg/encoder_libjpeg.cpp',\n> >>> 'jpeg/exif.cpp',\n> >>> + 'jpeg/post_processor_jpeg.cpp',\n> >>> ])\n> >>> android_camera_metadata_sources = files([\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel\n>\n> --\n> Regards\n> --\n> Kieran\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 4B136BDB13\n\tfor ;\n\tMon, 19 Oct 2020 05:43:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 17F68611B3;\n\tMon, 19 Oct 2020 07:43:55 +0200 (CEST)","from mail-ej1-x642.google.com (mail-ej1-x642.google.com\n\t[IPv6:2a00:1450:4864:20::642])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 22D7F60352\n\tfor ;\n\tMon, 19 Oct 2020 07:43:54 +0200 (CEST)","by mail-ej1-x642.google.com with SMTP id z5so6303606ejw.7\n\tfor ;\n\tSun, 18 Oct 2020 22:43:54 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"oZxDisWv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=fcalraEGPUKUCBXIE7LomHhaVjwuYmEtqM5S94+qzz8=;\n\tb=oZxDisWv6H730WOU3y/UQRJAKQ8qjSYK22qQivIgyyAWVdydAEHEOrRLE8Rg+E3ZAc\n\trJIbzfkSjoK26nLdN0pqScq8+oVSujNmW8bELOoNcmIxlc68Y7xXXb5vCN3ou31V6POb\n\tjzeUH40F6BuMsi3vniSvTyBSdEaaYPSbsTNNg=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=fcalraEGPUKUCBXIE7LomHhaVjwuYmEtqM5S94+qzz8=;\n\tb=e2hqC0iG3nJcNKl5WG1gBzURyNhOKZd+NpN0ymFyRyxLnl3sQng+ofUVijGDHZhIQL\n\t1pRaQStEjpV2TD6PhLTx5RwnqRj5pS083nFyROJyn1+KZoRNwzWh8WChh1ysrwPXPZTq\n\tp/1cR8QMNK1sU00jJ2kiIfL5KUBoy087iRNrUt1EVIXXKJfYQjOE+tPol76pfwhxwgJS\n\tdcDlYvWa7J/jBmsgAh0tiq5FE/uokq7wcOA0Sa+s4nrSpOHfgLlD7hlHFzUHEqp2UJ/o\n\tymVz0wbwXB02H5pCplJPVlHs3uGZCtOOPPJ53UeMuEtkRas3gmmWJ5DrLm2L9svsdGYl\n\twhGQ==","X-Gm-Message-State":"AOAM533TgpwI33u8rZ7hmsp+q1yIsyXTZIZnE7tmfj+i4p7VFWJkbmSp\n\tmqeHvOMxXn1AmIwOQDJ5Y+6OKvI1JT6YL6qA8iesow==","X-Google-Smtp-Source":"ABdhPJyqBQnDp8mip5orzs98aSFLs2eRKUkJIu60A3LV5KoxZft++BAMbakUoHHw6LjfqNu6SALgXHPZENDHcWrEtfc=","X-Received":"by 2002:a17:906:c152:: with SMTP id\n\tdp18mr15178801ejc.243.1603086233582; \n\tSun, 18 Oct 2020 22:43:53 -0700 (PDT)","MIME-Version":"1.0","References":"<20201016053754.17251-1-email@uajain.com>\n\t<20201016053754.17251-3-email@uajain.com>\n\t<20201016063253.GK3829@pendragon.ideasonboard.com>\n\t<617edcee-9cbc-e3ac-8c82-8ebbc68b3f28@uajain.com>\n\t<92d54fd6-5c90-29bb-dd6c-2f9c3a7ccebd@ideasonboard.com>","In-Reply-To":"<92d54fd6-5c90-29bb-dd6c-2f9c3a7ccebd@ideasonboard.com>","From":"Hirokazu Honda ","Date":"Mon, 19 Oct 2020 14:43:43 +0900","Message-ID":"","To":"kieran.bingham@ideasonboard.com","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] android: jpeg: Port to\n\tPostProcessor interface","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13246,"web_url":"https://patchwork.libcamera.org/comment/13246/","msgid":"","date":"2020-10-19T05:59:32","subject":"Re: [libcamera-devel] [PATCH v2 2/2] android: jpeg: Port to\n\tPostProcessor interface","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"On Mon, Oct 19, 2020 at 2:43 PM Hirokazu Honda wrote:\n>\n> On Fri, Oct 16, 2020 at 7:25 PM Kieran Bingham\n> wrote:\n> >\n> > Hi Umang, Laurent,\n> >\n> > On 16/10/2020 08:15, Umang Jain wrote:\n> > > Hi,\n> > >\n> > > On 10/16/20 12:02 PM, Laurent Pinchart wrote:\n> > >> Hi Umang,\n> > >>\n> > >> Thank you for the patch.\n> > >>\n> > >> On Fri, Oct 16, 2020 at 11:07:54AM +0530, Umang Jain wrote:\n> > >>> Port the CameraStream's JPEG-encoding bits to PostProcessorJpeg.\n> > >>> This encapsulates the encoder and EXIF generation code into the\n> > >>> PostProcessorJpeg layer and removes these specifics related to JPEG,\n> > >>> from the CameraStream itself.\n> > >>>\n> > >>> Signed-off-by: Umang Jain \n> > >>> Tested-by: Kieran Bingham \n> > >>> Reviewed-by: Kieran Bingham \n> > >> Reviewed-by: Laurent Pinchart \n> > >>\n> > >> Kieran, would you be able to test this new version, and push it if all\n> > >> looks good to you ?\n> > > Yes please, one final testing should be done before actual pushing.\n> > > Thanks for the reviews. Thanks to Kieran for removing blockers on this\n> > > and testing efforts :)\n> >\n> > Ok - I had a few hit and misses due to apparent hardware failure (the\n> > camera didn't power up, now up with a full power cycle) - and then the\n> > cros build not actually building the code.\n> >\n> > all of that is solved, and confirmed that it's definitely Umang's code\n> > running on the device ... successfully ;-)\n> >\n> > I'll push these patches now.\n> >\n> > Thanks\n> >\n> > Kieran\n> >\n> >\n> > >>\n> > >>> ---\n> > >>> src/android/camera_device.cpp | 1 +\n> > >>> src/android/camera_stream.cpp | 77 ++++-------------\n> > >>> src/android/camera_stream.h | 4 +-\n> > >>> src/android/jpeg/encoder_libjpeg.cpp | 2 +-\n> > >>> src/android/jpeg/post_processor_jpeg.cpp | 105 +++++++++++++++++++++++\n> > >>> src/android/jpeg/post_processor_jpeg.h | 36 ++++++++\n> > >>> src/android/meson.build | 1 +\n> > >>> 7 files changed, 164 insertions(+), 62 deletions(-)\n> > >>> create mode 100644 src/android/jpeg/post_processor_jpeg.cpp\n> > >>> create mode 100644 src/android/jpeg/post_processor_jpeg.h\n> > >>>\n> > >>> diff --git a/src/android/camera_device.cpp\n> > >>> b/src/android/camera_device.cpp\n> > >>> index 0983232..d706cf4 100644\n> > >>> --- a/src/android/camera_device.cpp\n> > >>> +++ b/src/android/camera_device.cpp\n> > >>> @@ -7,6 +7,7 @@\n> > >>> #include \"camera_device.h\"\n> > >>> #include \"camera_ops.h\"\n> > >>> +#include \"post_processor.h\"\n> > >>> #include \n> > >>> #include \n> > >>> diff --git a/src/android/camera_stream.cpp\n> > >>> b/src/android/camera_stream.cpp\n> > >>> index d8e45c2..1b8afa8 100644\n> > >>> --- a/src/android/camera_stream.cpp\n> > >>> +++ b/src/android/camera_stream.cpp\n> > >>> @@ -9,9 +9,9 @@\n> > >>> #include \"camera_device.h\"\n> > >>> #include \"camera_metadata.h\"\n> > >>> -#include \"jpeg/encoder.h\"\n> > >>> -#include \"jpeg/encoder_libjpeg.h\"\n> > >>> -#include \"jpeg/exif.h\"\n> > >>> +#include \"jpeg/post_processor_jpeg.h\"\n> > >>> +\n> > >>> +#include \n> > >>> using namespace libcamera;\n> > >>> @@ -45,8 +45,15 @@ CameraStream::CameraStream(CameraDevice\n> > >>> *cameraDevice, Type type,\n> > >>> {\n> > >>> config_ = cameraDevice_->cameraConfiguration();\n> > >>> - if (type_ == Type::Internal || type_ == Type::Mapped)\n> > >>> - encoder_ = std::make_unique();\n> > >>> + if (type_ == Type::Internal || type_ == Type::Mapped) {\n> > >>> + /*\n> > >>> + * \\todo There might be multiple post-processors. The logic\n> > >>> + * which should be instantiated here, is deferred for the\n> > >>> + * future. For now, we only have PostProcessorJpeg and that\n> > >>> + * is what we instantiate here.\n> > >>> + */\n> > >>> + postProcessor_ =\n> > >>> std::make_unique(cameraDevice_);\n> > >>> + }\n> > >>> if (type == Type::Internal) {\n> > >>> allocator_ =\n> > >>> std::make_unique(cameraDevice_->camera());\n> > >>> @@ -66,8 +73,10 @@ Stream *CameraStream::stream() const\n> > >>> int CameraStream::configure()\n> > >>> {\n> > >>> - if (encoder_) {\n> > >>> - int ret = encoder_->configure(configuration());\n> > >>> + if (postProcessor_) {\n> > >>> + StreamConfiguration output = configuration();\n> > >>> + output.pixelFormat = formats::MJPEG;\n> > >>> + int ret = postProcessor_->configure(configuration(), output);\n>\n> Hmm, this is strange to me.\n> |config_| should be used here and |Config::Size| should be set as well\n> as Format?\n>\n\nAh, sorry. I missed configuration() function in this class.\nThis implementation is correct. Sorry about that.\n\n> > >>> if (ret)\n> > >>> return ret;\n> > >>> }\n> > >>> @@ -90,60 +99,10 @@ int CameraStream::configure()\n> > >>> int CameraStream::process(const libcamera::FrameBuffer &source,\n> > >>> MappedCamera3Buffer *dest, CameraMetadata *metadata)\n> > >>> {\n> > >>> - if (!encoder_)\n> > >>> + if (!postProcessor_)\n> > >>> return 0;\n> > >>> - /* Set EXIF metadata for various tags. */\n> > >>> - Exif exif;\n> > >>> - /* \\todo Set Make and Model from external vendor tags. */\n> > >>> - exif.setMake(\"libcamera\");\n> > >>> - exif.setModel(\"cameraModel\");\n> > >>> - exif.setOrientation(cameraDevice_->orientation());\n> > >>> - exif.setSize(configuration().size);\n> > >>> - /*\n> > >>> - * We set the frame's EXIF timestamp as the time of encode.\n> > >>> - * Since the precision we need for EXIF timestamp is only one\n> > >>> - * second, it is good enough.\n> > >>> - */\n> > >>> - exif.setTimestamp(std::time(nullptr));\n> > >>> - if (exif.generate() != 0)\n> > >>> - LOG(HAL, Error) << \"Failed to generate valid EXIF data\";\n> > >>> -\n> > >>> - int jpeg_size = encoder_->encode(&source, dest->maps()[0],\n> > >>> exif.data());\n> > >>> - if (jpeg_size < 0) {\n> > >>> - LOG(HAL, Error) << \"Failed to encode stream image\";\n> > >>> - return jpeg_size;\n> > >>> - }\n> > >>> -\n> > >>> - /*\n> > >>> - * Fill in the JPEG blob header.\n> > >>> - *\n> > >>> - * The mapped size of the buffer is being returned as\n> > >>> - * substantially larger than the requested JPEG_MAX_SIZE\n> > >>> - * (which is referenced from maxJpegBufferSize_). Utilise\n> > >>> - * this static size to ensure the correct offset of the blob is\n> > >>> - * determined.\n> > >>> - *\n> > >>> - * \\todo Investigate if the buffer size mismatch is an issue or\n> > >>> - * expected behaviour.\n> > >>> - */\n> > >>> - uint8_t *resultPtr = dest->maps()[0].data() +\n> > >>> - cameraDevice_->maxJpegBufferSize() -\n> > >>> - sizeof(struct camera3_jpeg_blob);\n> > >>> - auto *blob = reinterpret_cast > >>> *>(resultPtr);\n> > >>> - blob->jpeg_blob_id = CAMERA3_JPEG_BLOB_ID;\n> > >>> - blob->jpeg_size = jpeg_size;\n> > >>> -\n> > >>> - /* Update the JPEG result Metadata. */\n> > >>> - metadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1);\n> > >>> -\n> > >>> - const uint32_t jpeg_quality = 95;\n> > >>> - metadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 1);\n> > >>> -\n> > >>> - const uint32_t jpeg_orientation = 0;\n> > >>> - metadata->addEntry(ANDROID_JPEG_ORIENTATION, &jpeg_orientation, 1);\n> > >>> -\n> > >>> - return 0;\n> > >>> + return postProcessor_->process(&source, dest->maps()[0], metadata);\n> > >>> }\n> > >>> FrameBuffer *CameraStream::getBuffer()\n> > >>> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n> > >>> index d3925fb..c367a5f 100644\n> > >>> --- a/src/android/camera_stream.h\n> > >>> +++ b/src/android/camera_stream.h\n> > >>> @@ -19,10 +19,10 @@\n> > >>> #include \n> > >>> #include \n> > >>> -class Encoder;\n> > >>> class CameraDevice;\n> > >>> class CameraMetadata;\n> > >>> class MappedCamera3Buffer;\n> > >>> +class PostProcessor;\n> > >>> class CameraStream\n> > >>> {\n> > >>> @@ -130,7 +130,6 @@ private:\n> > >>> camera3_stream_t *camera3Stream_;\n> > >>> unsigned int index_;\n> > >>> - std::unique_ptr encoder_;\n> > >>> std::unique_ptr allocator_;\n> > >>> std::vector buffers_;\n> > >>> /*\n> > >>> @@ -138,6 +137,7 @@ private:\n> > >>> * an std::vector in CameraDevice.\n> > >>> */\n> > >>> std::unique_ptr mutex_;\n> > >>> + std::unique_ptr postProcessor_;\n> > >>> };\n> > >>> #endif /* __ANDROID_CAMERA_STREAM__ */\n> > >>> diff --git a/src/android/jpeg/encoder_libjpeg.cpp\n> > >>> b/src/android/jpeg/encoder_libjpeg.cpp\n> > >>> index a77f5b2..8995ba5 100644\n> > >>> --- a/src/android/jpeg/encoder_libjpeg.cpp\n> > >>> +++ b/src/android/jpeg/encoder_libjpeg.cpp\n> > >>> @@ -25,7 +25,7 @@\n> > >>> using namespace libcamera;\n> > >>> -LOG_DEFINE_CATEGORY(JPEG)\n> > >>> +LOG_DECLARE_CATEGORY(JPEG);\n> > >>> namespace {\n> > >>> diff --git a/src/android/jpeg/post_processor_jpeg.cpp\n> > >>> b/src/android/jpeg/post_processor_jpeg.cpp\n> > >>> new file mode 100644\n> > >>> index 0000000..753c28e\n> > >>> --- /dev/null\n> > >>> +++ b/src/android/jpeg/post_processor_jpeg.cpp\n> > >>> @@ -0,0 +1,105 @@\n> > >>> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > >>> +/*\n> > >>> + * Copyright (C) 2020, Google Inc.\n> > >>> + *\n> > >>> + * post_processor_jpeg.cpp - JPEG Post Processor\n> > >>> + */\n> > >>> +\n> > >>> +#include \"post_processor_jpeg.h\"\n> > >>> +\n> > >>> +#include \"../camera_device.h\"\n> > >>> +#include \"../camera_metadata.h\"\n> > >>> +#include \"encoder_libjpeg.h\"\n> > >>> +#include \"exif.h\"\n> > >>> +\n> > >>> +#include \n> > >>> +\n> > >>> +#include \"libcamera/internal/log.h\"\n> > >>> +\n> > >>> +using namespace libcamera;\n> > >>> +\n> > >>> +LOG_DEFINE_CATEGORY(JPEG);\n> > >>> +\n> > >>> +PostProcessorJpeg::PostProcessorJpeg(CameraDevice *device)\n> > >>> + : cameraDevice_(device)\n> > >>> +{\n> > >>> +}\n> > >>> +\n> > >>> +int PostProcessorJpeg::configure(const StreamConfiguration &inCfg,\n> > >>> + const StreamConfiguration &outCfg)\n> > >>> +{\n> > >>> + if (inCfg.size != outCfg.size) {\n> > >>> + LOG(JPEG, Error) << \"Mismatch of input and output stream\n> > >>> sizes\";\n> > >>> + return -EINVAL;\n> > >>> + }\n> > >>> +\n> > >>> + if (outCfg.pixelFormat != formats::MJPEG) {\n> > >>> + LOG(JPEG, Error) << \"Output stream pixel format is not JPEG\";\n> > >>> + return -EINVAL;\n> > >>> + }\n> > >>> +\n> > >>> + streamSize_ = outCfg.size;\n> > >>> + encoder_ = std::make_unique();\n> > >>> +\n> > >>> + return encoder_->configure(inCfg);\n> > >>> +}\n> > >>> +\n> > >>> +int PostProcessorJpeg::process(const libcamera::FrameBuffer *source,\n> > >>> + const libcamera::Span &destination,\n> > >>> + CameraMetadata *metadata)\n> > >>> +{\n> > >>> + if (!encoder_)\n> > >>> + return 0;\n> > >>> +\n> > >>> + /* Set EXIF metadata for various tags. */\n> > >>> + Exif exif;\n> > >>> + /* \\todo Set Make and Model from external vendor tags. */\n> > >>> + exif.setMake(\"libcamera\");\n> > >>> + exif.setModel(\"cameraModel\");\n> > >>> + exif.setOrientation(cameraDevice_->orientation());\n> > >>> + exif.setSize(streamSize_);\n> > >>> + /*\n> > >>> + * We set the frame's EXIF timestamp as the time of encode.\n> > >>> + * Since the precision we need for EXIF timestamp is only one\n> > >>> + * second, it is good enough.\n> > >>> + */\n> > >>> + exif.setTimestamp(std::time(nullptr));\n> > >>> + if (exif.generate() != 0)\n> > >>> + LOG(JPEG, Error) << \"Failed to generate valid EXIF data\";\n>\n> Shall we handle this case?\n>\n> > >>> +\n> > >>> + int jpeg_size = encoder_->encode(source, destination, exif.data());\n> > >>> + if (jpeg_size < 0) {\n> > >>> + LOG(JPEG, Error) << \"Failed to encode stream image\";\n> > >>> + return jpeg_size;\n> > >>> + }\n> > >>> +\n> > >>> + /*\n> > >>> + * Fill in the JPEG blob header.\n> > >>> + *\n> > >>> + * The mapped size of the buffer is being returned as\n> > >>> + * substantially larger than the requested JPEG_MAX_SIZE\n> > >>> + * (which is referenced from maxJpegBufferSize_). Utilise\n> > >>> + * this static size to ensure the correct offset of the blob is\n> > >>> + * determined.\n> > >>> + *\n> > >>> + * \\todo Investigate if the buffer size mismatch is an issue or\n> > >>> + * expected behaviour.\n> > >>> + */\n> > >>> + uint8_t *resultPtr = destination.data() +\n> > >>> + cameraDevice_->maxJpegBufferSize() -\n> > >>> + sizeof(struct camera3_jpeg_blob);\n> > >>> + auto *blob = reinterpret_cast > >>> *>(resultPtr);\n> > >>> + blob->jpeg_blob_id = CAMERA3_JPEG_BLOB_ID;\n> > >>> + blob->jpeg_size = jpeg_size;\n> > >>> +\n> > >>> + /* Update the JPEG result Metadata. */\n> > >>> + metadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1);\n> > >>> +\n> > >>> + const uint32_t jpeg_quality = 95;\n> > >>> + metadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 1);\n> > >>> +\n> > >>> + const uint32_t jpeg_orientation = 0;\n> > >>> + metadata->addEntry(ANDROID_JPEG_ORIENTATION, &jpeg_orientation, 1);\n> > >>> +\n> > >>> + return 0;\n> > >>> +}\n> > >>> diff --git a/src/android/jpeg/post_processor_jpeg.h\n> > >>> b/src/android/jpeg/post_processor_jpeg.h\n> > >>> new file mode 100644\n> > >>> index 0000000..62c8650\n> > >>> --- /dev/null\n> > >>> +++ b/src/android/jpeg/post_processor_jpeg.h\n> > >>> @@ -0,0 +1,36 @@\n> > >>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > >>> +/*\n> > >>> + * Copyright (C) 2020, Google Inc.\n> > >>> + *\n> > >>> + * post_processor_jpeg.h - JPEG Post Processor\n> > >>> + */\n> > >>> +#ifndef __ANDROID_POST_PROCESSOR_JPEG_H__\n> > >>> +#define __ANDROID_POST_PROCESSOR_JPEG_H__\n> > >>> +\n> > >>> +#include \"../post_processor.h\"\n> > >>> +\n> > >>> +#include \n> > >>> +\n> > >>> +#include \"libcamera/internal/buffer.h\"\n> > >>> +\n> > >>> +class Encoder;\n> > >>> +class CameraDevice;\n> > >>> +\n> > >>> +class PostProcessorJpeg : public PostProcessor\n> > >>> +{\n> > >>> +public:\n> > >>> + PostProcessorJpeg(CameraDevice *device);\n> > >>> +\n> > >>> + int configure(const libcamera::StreamConfiguration &incfg,\n> > >>> + const libcamera::StreamConfiguration &outcfg) override;\n> > >>> + int process(const libcamera::FrameBuffer *source,\n> > >>> + const libcamera::Span &destination,\n> > >>> + CameraMetadata *metadata) override;\n> > >>> +\n> > >>> +private:\n> > >>> + CameraDevice *cameraDevice_;\n>\n> nit: CameraDevice const *cameraDevice_ to represent |cameraDevice_| is\n> owned by a client.\n>\n> > >>> + std::unique_ptr encoder_;\n> > >>> + libcamera::Size streamSize_;\n> > >>> +};\n> > >>> +\n> > >>> +#endif /* __ANDROID_POST_PROCESSOR_JPEG_H__ */\n> > >>> diff --git a/src/android/meson.build b/src/android/meson.build\n> > >>> index b2b2293..5a01bea 100644\n> > >>> --- a/src/android/meson.build\n> > >>> +++ b/src/android/meson.build\n> > >>> @@ -24,6 +24,7 @@ android_hal_sources = files([\n> > >>> 'camera_worker.cpp',\n> > >>> 'jpeg/encoder_libjpeg.cpp',\n> > >>> 'jpeg/exif.cpp',\n> > >>> + 'jpeg/post_processor_jpeg.cpp',\n> > >>> ])\n> > >>> android_camera_metadata_sources = files([\n> > >\n> > > _______________________________________________\n> > > libcamera-devel mailing list\n> > > libcamera-devel@lists.libcamera.org\n> > > https://lists.libcamera.org/listinfo/libcamera-devel\n> >\n> > --\n> > Regards\n> > --\n> > Kieran\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id DC5FEBDB13\n\tfor ;\n\tMon, 19 Oct 2020 05:59:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 68E4B61161;\n\tMon, 19 Oct 2020 07:59:44 +0200 (CEST)","from mail-ej1-x643.google.com (mail-ej1-x643.google.com\n\t[IPv6:2a00:1450:4864:20::643])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C445E60352\n\tfor ;\n\tMon, 19 Oct 2020 07:59:42 +0200 (CEST)","by mail-ej1-x643.google.com with SMTP id ce10so12166709ejc.5\n\tfor ;\n\tSun, 18 Oct 2020 22:59:42 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"TYurvd+p\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=Y3YIo3qO7CoGKjt+w3yqQ+ON3y8nHGLka4ZzoJjCMbw=;\n\tb=TYurvd+p5lSdPe88h9J6slcByVWC/VYvqTjQOk4F7FL4xn/ItDmdfx+BCVdrxPOfXl\n\t+ic7218Qsv0SdBjucNZnNZ6w62V3qh9GsIxHHizdUCwbDanMhLHzCP9wEVEox+DhROZR\n\tdt6H8C8V3FOJOn3TF2LCXGjGVcFy+rBtgt3Nk=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=Y3YIo3qO7CoGKjt+w3yqQ+ON3y8nHGLka4ZzoJjCMbw=;\n\tb=EmZVuIPzjTm+cAjHrAkTJI1kKkDK38HRhKPO2hiQEYrQpay7VHyj1VkuBGl/a93iXR\n\t256kC14zjhhTHlepbgHIMBUJoulPhFs/qbyy4NP8a/MvVAQsqEovuey2T4lGkoC6LJzx\n\trU3Qs2/STwBzj93hq5sWJRX3EPkEno99EiYhZWJDwaxgtXWJ9vqEIkfMp76pU+GgXjkZ\n\tlPG/7TFo6id8GN20OOPWwYxv9xUhFzrVI3BOXalVG+ZfbtpBd7npgLiW6PqE3Imv9WTZ\n\tEmA//FlCcHaicVo4/lEJ5NQ3fPC9/tGKHvpK+uofp9HetgpnU3zMjsSTDvw8yuHw8M9p\n\tU0YA==","X-Gm-Message-State":"AOAM532/2/9quq/rJq38QPW8u4pXqttxleqdXs3QmZBDFxXWQFLoPTp+\n\tioXNStFAykVarfraPnwI8ZlRPDKAsDAIge7jMoByaA==","X-Google-Smtp-Source":"ABdhPJzwhYznGiaZiY4RohbSomBmASCJRw/tog+5ZGMbhb2B/axfcCTI5EPh9a2hg50i47K95xaZtGdhk7ASoqD1QLs=","X-Received":"by 2002:a17:906:2894:: with SMTP id\n\to20mr15364676ejd.221.1603087182180; \n\tSun, 18 Oct 2020 22:59:42 -0700 (PDT)","MIME-Version":"1.0","References":"<20201016053754.17251-1-email@uajain.com>\n\t<20201016053754.17251-3-email@uajain.com>\n\t<20201016063253.GK3829@pendragon.ideasonboard.com>\n\t<617edcee-9cbc-e3ac-8c82-8ebbc68b3f28@uajain.com>\n\t<92d54fd6-5c90-29bb-dd6c-2f9c3a7ccebd@ideasonboard.com>\n\t","In-Reply-To":"","From":"Hirokazu Honda ","Date":"Mon, 19 Oct 2020 14:59:32 +0900","Message-ID":"","To":"kieran.bingham@ideasonboard.com","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] android: jpeg: Port to\n\tPostProcessor interface","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13247,"web_url":"https://patchwork.libcamera.org/comment/13247/","msgid":"<1c29b57b-4e9f-f973-0476-f9312575fcec@ideasonboard.com>","date":"2020-10-19T08:29:45","subject":"Re: [libcamera-devel] [PATCH v2 2/2] android: jpeg: Port to\n\tPostProcessor interface","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Hiro,\n\nOn 19/10/2020 06:43, Hirokazu Honda wrote:\n> On Fri, Oct 16, 2020 at 7:25 PM Kieran Bingham\n> wrote:\n>>\n>> Hi Umang, Laurent,\n>>\n>> On 16/10/2020 08:15, Umang Jain wrote:\n>>> Hi,\n>>>\n>>> On 10/16/20 12:02 PM, Laurent Pinchart wrote:\n>>>> Hi Umang,\n>>>>\n>>>> Thank you for the patch.\n>>>>\n>>>> On Fri, Oct 16, 2020 at 11:07:54AM +0530, Umang Jain wrote:\n>>>>> Port the CameraStream's JPEG-encoding bits to PostProcessorJpeg.\n>>>>> This encapsulates the encoder and EXIF generation code into the\n>>>>> PostProcessorJpeg layer and removes these specifics related to JPEG,\n>>>>> from the CameraStream itself.\n>>>>>\n>>>>> Signed-off-by: Umang Jain \n>>>>> Tested-by: Kieran Bingham \n>>>>> Reviewed-by: Kieran Bingham \n>>>> Reviewed-by: Laurent Pinchart \n>>>>\n>>>> Kieran, would you be able to test this new version, and push it if all\n>>>> looks good to you ?\n>>> Yes please, one final testing should be done before actual pushing.\n>>> Thanks for the reviews. Thanks to Kieran for removing blockers on this\n>>> and testing efforts :)\n>>\n>> Ok - I had a few hit and misses due to apparent hardware failure (the\n>> camera didn't power up, now up with a full power cycle) - and then the\n>> cros build not actually building the code.\n>>\n>> all of that is solved, and confirmed that it's definitely Umang's code\n>> running on the device ... successfully ;-)\n>>\n>> I'll push these patches now.\n>>\n>> Thanks\n>>\n>> Kieran\n>>\n>>\n>>>>\n>>>>> ---\n>>>>> src/android/camera_device.cpp | 1 +\n>>>>> src/android/camera_stream.cpp | 77 ++++-------------\n>>>>> src/android/camera_stream.h | 4 +-\n>>>>> src/android/jpeg/encoder_libjpeg.cpp | 2 +-\n>>>>> src/android/jpeg/post_processor_jpeg.cpp | 105 +++++++++++++++++++++++\n>>>>> src/android/jpeg/post_processor_jpeg.h | 36 ++++++++\n>>>>> src/android/meson.build | 1 +\n>>>>> 7 files changed, 164 insertions(+), 62 deletions(-)\n>>>>> create mode 100644 src/android/jpeg/post_processor_jpeg.cpp\n>>>>> create mode 100644 src/android/jpeg/post_processor_jpeg.h\n>>>>>\n>>>>> diff --git a/src/android/camera_device.cpp\n>>>>> b/src/android/camera_device.cpp\n>>>>> index 0983232..d706cf4 100644\n>>>>> --- a/src/android/camera_device.cpp\n>>>>> +++ b/src/android/camera_device.cpp\n>>>>> @@ -7,6 +7,7 @@\n>>>>> #include \"camera_device.h\"\n>>>>> #include \"camera_ops.h\"\n>>>>> +#include \"post_processor.h\"\n>>>>> #include \n>>>>> #include \n>>>>> diff --git a/src/android/camera_stream.cpp\n>>>>> b/src/android/camera_stream.cpp\n>>>>> index d8e45c2..1b8afa8 100644\n>>>>> --- a/src/android/camera_stream.cpp\n>>>>> +++ b/src/android/camera_stream.cpp\n>>>>> @@ -9,9 +9,9 @@\n>>>>> #include \"camera_device.h\"\n>>>>> #include \"camera_metadata.h\"\n>>>>> -#include \"jpeg/encoder.h\"\n>>>>> -#include \"jpeg/encoder_libjpeg.h\"\n>>>>> -#include \"jpeg/exif.h\"\n>>>>> +#include \"jpeg/post_processor_jpeg.h\"\n>>>>> +\n>>>>> +#include \n>>>>> using namespace libcamera;\n>>>>> @@ -45,8 +45,15 @@ CameraStream::CameraStream(CameraDevice\n>>>>> *cameraDevice, Type type,\n>>>>> {\n>>>>> config_ = cameraDevice_->cameraConfiguration();\n>>>>> - if (type_ == Type::Internal || type_ == Type::Mapped)\n>>>>> - encoder_ = std::make_unique();\n>>>>> + if (type_ == Type::Internal || type_ == Type::Mapped) {\n>>>>> + /*\n>>>>> + * \\todo There might be multiple post-processors. The logic\n>>>>> + * which should be instantiated here, is deferred for the\n>>>>> + * future. For now, we only have PostProcessorJpeg and that\n>>>>> + * is what we instantiate here.\n>>>>> + */\n>>>>> + postProcessor_ =\n>>>>> std::make_unique(cameraDevice_);\n>>>>> + }\n>>>>> if (type == Type::Internal) {\n>>>>> allocator_ =\n>>>>> std::make_unique(cameraDevice_->camera());\n>>>>> @@ -66,8 +73,10 @@ Stream *CameraStream::stream() const\n>>>>> int CameraStream::configure()\n>>>>> {\n>>>>> - if (encoder_) {\n>>>>> - int ret = encoder_->configure(configuration());\n>>>>> + if (postProcessor_) {\n>>>>> + StreamConfiguration output = configuration();\n>>>>> + output.pixelFormat = formats::MJPEG;\n>>>>> + int ret = postProcessor_->configure(configuration(), output);\n> \n> Hmm, this is strange to me.\n> |config_| should be used here and |Config::Size| should be set as well\n> as Format?\n> \n\nAs you've already mentioned, this is handled through configuration().\n\nconfig_ is the CameraConfiguration, where configuration() is the Stream\nConfiguration.\n\n\n>>>>> if (ret)\n>>>>> return ret;\n>>>>> }\n>>>>> @@ -90,60 +99,10 @@ int CameraStream::configure()\n>>>>> int CameraStream::process(const libcamera::FrameBuffer &source,\n>>>>> MappedCamera3Buffer *dest, CameraMetadata *metadata)\n>>>>> {\n>>>>> - if (!encoder_)\n>>>>> + if (!postProcessor_)\n>>>>> return 0;\n>>>>> - /* Set EXIF metadata for various tags. */\n>>>>> - Exif exif;\n>>>>> - /* \\todo Set Make and Model from external vendor tags. */\n>>>>> - exif.setMake(\"libcamera\");\n>>>>> - exif.setModel(\"cameraModel\");\n>>>>> - exif.setOrientation(cameraDevice_->orientation());\n>>>>> - exif.setSize(configuration().size);\n>>>>> - /*\n>>>>> - * We set the frame's EXIF timestamp as the time of encode.\n>>>>> - * Since the precision we need for EXIF timestamp is only one\n>>>>> - * second, it is good enough.\n>>>>> - */\n>>>>> - exif.setTimestamp(std::time(nullptr));\n>>>>> - if (exif.generate() != 0)\n>>>>> - LOG(HAL, Error) << \"Failed to generate valid EXIF data\";\n>>>>> -\n>>>>> - int jpeg_size = encoder_->encode(&source, dest->maps()[0],\n>>>>> exif.data());\n>>>>> - if (jpeg_size < 0) {\n>>>>> - LOG(HAL, Error) << \"Failed to encode stream image\";\n>>>>> - return jpeg_size;\n>>>>> - }\n>>>>> -\n>>>>> - /*\n>>>>> - * Fill in the JPEG blob header.\n>>>>> - *\n>>>>> - * The mapped size of the buffer is being returned as\n>>>>> - * substantially larger than the requested JPEG_MAX_SIZE\n>>>>> - * (which is referenced from maxJpegBufferSize_). Utilise\n>>>>> - * this static size to ensure the correct offset of the blob is\n>>>>> - * determined.\n>>>>> - *\n>>>>> - * \\todo Investigate if the buffer size mismatch is an issue or\n>>>>> - * expected behaviour.\n>>>>> - */\n>>>>> - uint8_t *resultPtr = dest->maps()[0].data() +\n>>>>> - cameraDevice_->maxJpegBufferSize() -\n>>>>> - sizeof(struct camera3_jpeg_blob);\n>>>>> - auto *blob = reinterpret_cast>>>> *>(resultPtr);\n>>>>> - blob->jpeg_blob_id = CAMERA3_JPEG_BLOB_ID;\n>>>>> - blob->jpeg_size = jpeg_size;\n>>>>> -\n>>>>> - /* Update the JPEG result Metadata. */\n>>>>> - metadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1);\n>>>>> -\n>>>>> - const uint32_t jpeg_quality = 95;\n>>>>> - metadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 1);\n>>>>> -\n>>>>> - const uint32_t jpeg_orientation = 0;\n>>>>> - metadata->addEntry(ANDROID_JPEG_ORIENTATION, &jpeg_orientation, 1);\n>>>>> -\n>>>>> - return 0;\n>>>>> + return postProcessor_->process(&source, dest->maps()[0], metadata);\n>>>>> }\n>>>>> FrameBuffer *CameraStream::getBuffer()\n>>>>> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n>>>>> index d3925fb..c367a5f 100644\n>>>>> --- a/src/android/camera_stream.h\n>>>>> +++ b/src/android/camera_stream.h\n>>>>> @@ -19,10 +19,10 @@\n>>>>> #include \n>>>>> #include \n>>>>> -class Encoder;\n>>>>> class CameraDevice;\n>>>>> class CameraMetadata;\n>>>>> class MappedCamera3Buffer;\n>>>>> +class PostProcessor;\n>>>>> class CameraStream\n>>>>> {\n>>>>> @@ -130,7 +130,6 @@ private:\n>>>>> camera3_stream_t *camera3Stream_;\n>>>>> unsigned int index_;\n>>>>> - std::unique_ptr encoder_;\n>>>>> std::unique_ptr allocator_;\n>>>>> std::vector buffers_;\n>>>>> /*\n>>>>> @@ -138,6 +137,7 @@ private:\n>>>>> * an std::vector in CameraDevice.\n>>>>> */\n>>>>> std::unique_ptr mutex_;\n>>>>> + std::unique_ptr postProcessor_;\n>>>>> };\n>>>>> #endif /* __ANDROID_CAMERA_STREAM__ */\n>>>>> diff --git a/src/android/jpeg/encoder_libjpeg.cpp\n>>>>> b/src/android/jpeg/encoder_libjpeg.cpp\n>>>>> index a77f5b2..8995ba5 100644\n>>>>> --- a/src/android/jpeg/encoder_libjpeg.cpp\n>>>>> +++ b/src/android/jpeg/encoder_libjpeg.cpp\n>>>>> @@ -25,7 +25,7 @@\n>>>>> using namespace libcamera;\n>>>>> -LOG_DEFINE_CATEGORY(JPEG)\n>>>>> +LOG_DECLARE_CATEGORY(JPEG);\n>>>>> namespace {\n>>>>> diff --git a/src/android/jpeg/post_processor_jpeg.cpp\n>>>>> b/src/android/jpeg/post_processor_jpeg.cpp\n>>>>> new file mode 100644\n>>>>> index 0000000..753c28e\n>>>>> --- /dev/null\n>>>>> +++ b/src/android/jpeg/post_processor_jpeg.cpp\n>>>>> @@ -0,0 +1,105 @@\n>>>>> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n>>>>> +/*\n>>>>> + * Copyright (C) 2020, Google Inc.\n>>>>> + *\n>>>>> + * post_processor_jpeg.cpp - JPEG Post Processor\n>>>>> + */\n>>>>> +\n>>>>> +#include \"post_processor_jpeg.h\"\n>>>>> +\n>>>>> +#include \"../camera_device.h\"\n>>>>> +#include \"../camera_metadata.h\"\n>>>>> +#include \"encoder_libjpeg.h\"\n>>>>> +#include \"exif.h\"\n>>>>> +\n>>>>> +#include \n>>>>> +\n>>>>> +#include \"libcamera/internal/log.h\"\n>>>>> +\n>>>>> +using namespace libcamera;\n>>>>> +\n>>>>> +LOG_DEFINE_CATEGORY(JPEG);\n>>>>> +\n>>>>> +PostProcessorJpeg::PostProcessorJpeg(CameraDevice *device)\n>>>>> + : cameraDevice_(device)\n>>>>> +{\n>>>>> +}\n>>>>> +\n>>>>> +int PostProcessorJpeg::configure(const StreamConfiguration &inCfg,\n>>>>> + const StreamConfiguration &outCfg)\n>>>>> +{\n>>>>> + if (inCfg.size != outCfg.size) {\n>>>>> + LOG(JPEG, Error) << \"Mismatch of input and output stream\n>>>>> sizes\";\n>>>>> + return -EINVAL;\n>>>>> + }\n>>>>> +\n>>>>> + if (outCfg.pixelFormat != formats::MJPEG) {\n>>>>> + LOG(JPEG, Error) << \"Output stream pixel format is not JPEG\";\n>>>>> + return -EINVAL;\n>>>>> + }\n>>>>> +\n>>>>> + streamSize_ = outCfg.size;\n>>>>> + encoder_ = std::make_unique();\n>>>>> +\n>>>>> + return encoder_->configure(inCfg);\n>>>>> +}\n>>>>> +\n>>>>> +int PostProcessorJpeg::process(const libcamera::FrameBuffer *source,\n>>>>> + const libcamera::Span &destination,\n>>>>> + CameraMetadata *metadata)\n>>>>> +{\n>>>>> + if (!encoder_)\n>>>>> + return 0;\n>>>>> +\n>>>>> + /* Set EXIF metadata for various tags. */\n>>>>> + Exif exif;\n>>>>> + /* \\todo Set Make and Model from external vendor tags. */\n>>>>> + exif.setMake(\"libcamera\");\n>>>>> + exif.setModel(\"cameraModel\");\n>>>>> + exif.setOrientation(cameraDevice_->orientation());\n>>>>> + exif.setSize(streamSize_);\n>>>>> + /*\n>>>>> + * We set the frame's EXIF timestamp as the time of encode.\n>>>>> + * Since the precision we need for EXIF timestamp is only one\n>>>>> + * second, it is good enough.\n>>>>> + */\n>>>>> + exif.setTimestamp(std::time(nullptr));\n>>>>> + if (exif.generate() != 0)\n>>>>> + LOG(JPEG, Error) << \"Failed to generate valid EXIF data\";\n> \n> Shall we handle this case?\n\nI recall the discussion from this when it went in originally was that\nthere's not much we can do.\n\nIf we fail to generate the exif, we probably still want to output the\nimage, as it's not fatal, but the error highlights it has happened.\n\nOf course that won't be visible to the end consumer, though the lack of\nexif tags will be.\n\nDo you think that if the exif tags can't be created, the image should\nfail entirely?\n\n\n>>>>> +\n>>>>> + int jpeg_size = encoder_->encode(source, destination, exif.data());\n>>>>> + if (jpeg_size < 0) {\n>>>>> + LOG(JPEG, Error) << \"Failed to encode stream image\";\n>>>>> + return jpeg_size;\n>>>>> + }\n>>>>> +\n>>>>> + /*\n>>>>> + * Fill in the JPEG blob header.\n>>>>> + *\n>>>>> + * The mapped size of the buffer is being returned as\n>>>>> + * substantially larger than the requested JPEG_MAX_SIZE\n>>>>> + * (which is referenced from maxJpegBufferSize_). Utilise\n>>>>> + * this static size to ensure the correct offset of the blob is\n>>>>> + * determined.\n>>>>> + *\n>>>>> + * \\todo Investigate if the buffer size mismatch is an issue or\n>>>>> + * expected behaviour.\n>>>>> + */\n>>>>> + uint8_t *resultPtr = destination.data() +\n>>>>> + cameraDevice_->maxJpegBufferSize() -\n>>>>> + sizeof(struct camera3_jpeg_blob);\n>>>>> + auto *blob = reinterpret_cast>>>> *>(resultPtr);\n>>>>> + blob->jpeg_blob_id = CAMERA3_JPEG_BLOB_ID;\n>>>>> + blob->jpeg_size = jpeg_size;\n>>>>> +\n>>>>> + /* Update the JPEG result Metadata. */\n>>>>> + metadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1);\n>>>>> +\n>>>>> + const uint32_t jpeg_quality = 95;\n>>>>> + metadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 1);\n>>>>> +\n>>>>> + const uint32_t jpeg_orientation = 0;\n>>>>> + metadata->addEntry(ANDROID_JPEG_ORIENTATION, &jpeg_orientation, 1);\n>>>>> +\n>>>>> + return 0;\n>>>>> +}\n>>>>> diff --git a/src/android/jpeg/post_processor_jpeg.h\n>>>>> b/src/android/jpeg/post_processor_jpeg.h\n>>>>> new file mode 100644\n>>>>> index 0000000..62c8650\n>>>>> --- /dev/null\n>>>>> +++ b/src/android/jpeg/post_processor_jpeg.h\n>>>>> @@ -0,0 +1,36 @@\n>>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>>>> +/*\n>>>>> + * Copyright (C) 2020, Google Inc.\n>>>>> + *\n>>>>> + * post_processor_jpeg.h - JPEG Post Processor\n>>>>> + */\n>>>>> +#ifndef __ANDROID_POST_PROCESSOR_JPEG_H__\n>>>>> +#define __ANDROID_POST_PROCESSOR_JPEG_H__\n>>>>> +\n>>>>> +#include \"../post_processor.h\"\n>>>>> +\n>>>>> +#include \n>>>>> +\n>>>>> +#include \"libcamera/internal/buffer.h\"\n>>>>> +\n>>>>> +class Encoder;\n>>>>> +class CameraDevice;\n>>>>> +\n>>>>> +class PostProcessorJpeg : public PostProcessor\n>>>>> +{\n>>>>> +public:\n>>>>> + PostProcessorJpeg(CameraDevice *device);\n>>>>> +\n>>>>> + int configure(const libcamera::StreamConfiguration &incfg,\n>>>>> + const libcamera::StreamConfiguration &outcfg) override;\n>>>>> + int process(const libcamera::FrameBuffer *source,\n>>>>> + const libcamera::Span &destination,\n>>>>> + CameraMetadata *metadata) override;\n>>>>> +\n>>>>> +private:\n>>>>> + CameraDevice *cameraDevice_;\n> \n> nit: CameraDevice const *cameraDevice_ to represent |cameraDevice_| is\n> owned by a client.\n> \n\nConst would be more accurate here indeed I think.\n\nCould you submit that as a patch perhaps please?\n\n\n>>>>> + std::unique_ptr encoder_;\n>>>>> + libcamera::Size streamSize_;\n>>>>> +};\n>>>>> +\n>>>>> +#endif /* __ANDROID_POST_PROCESSOR_JPEG_H__ */\n>>>>> diff --git a/src/android/meson.build b/src/android/meson.build\n>>>>> index b2b2293..5a01bea 100644\n>>>>> --- a/src/android/meson.build\n>>>>> +++ b/src/android/meson.build\n>>>>> @@ -24,6 +24,7 @@ android_hal_sources = files([\n>>>>> 'camera_worker.cpp',\n>>>>> 'jpeg/encoder_libjpeg.cpp',\n>>>>> 'jpeg/exif.cpp',\n>>>>> + 'jpeg/post_processor_jpeg.cpp',\n>>>>> ])\n>>>>> android_camera_metadata_sources = files([\n>>>\n>>> _______________________________________________\n>>> libcamera-devel mailing list\n>>> libcamera-devel@lists.libcamera.org\n>>> https://lists.libcamera.org/listinfo/libcamera-devel\n>>\n>> --\n>> Regards\n>> --\n>> Kieran\n>> _______________________________________________\n>> libcamera-devel mailing list\n>> libcamera-devel@lists.libcamera.org\n>> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id E9B35BDB13\n\tfor ;\n\tMon, 19 Oct 2020 08:29:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5D45A61161;\n\tMon, 19 Oct 2020 10:29:51 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DFA31603AF\n\tfor ;\n\tMon, 19 Oct 2020 10:29:49 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7FBED48;\n\tMon, 19 Oct 2020 10:29:48 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"YCKn6DoP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1603096188;\n\tbh=TJ8B2P4Ixvtx3+jBsRpjavAgnI1NNcdQmp14K5HkReI=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=YCKn6DoP/f9sHMS1cgq3iU3TP8lzd1CtaQgGeS6jj4uqUsUgVsXT8DmnqqzjvxDPW\n\thgftEsaI6O5VyOx+lCizev6e3305f41eBlO3z+uaBQvisCtHyBbw94uWzRkH1WgTSM\n\tYyD642A2q9B+LHd9UAjLueWvdS7gRZwgedfaz3Uc=","To":"Hirokazu Honda ","References":"<20201016053754.17251-1-email@uajain.com>\n\t<20201016053754.17251-3-email@uajain.com>\n\t<20201016063253.GK3829@pendragon.ideasonboard.com>\n\t<617edcee-9cbc-e3ac-8c82-8ebbc68b3f28@uajain.com>\n\t<92d54fd6-5c90-29bb-dd6c-2f9c3a7ccebd@ideasonboard.com>\n\t","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<1c29b57b-4e9f-f973-0476-f9312575fcec@ideasonboard.com>","Date":"Mon, 19 Oct 2020 09:29:45 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] android: jpeg: Port to\n\tPostProcessor interface","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13249,"web_url":"https://patchwork.libcamera.org/comment/13249/","msgid":"","date":"2020-10-19T10:29:54","subject":"Re: [libcamera-devel] [PATCH v2 2/2] android: jpeg: Port to\n\tPostProcessor interface","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"On Mon, Oct 19, 2020 at 5:29 PM Kieran Bingham\n wrote:\n>\n> Hi Hiro,\n>\n> On 19/10/2020 06:43, Hirokazu Honda wrote:\n> > On Fri, Oct 16, 2020 at 7:25 PM Kieran Bingham\n> > wrote:\n> >>\n> >> Hi Umang, Laurent,\n> >>\n> >> On 16/10/2020 08:15, Umang Jain wrote:\n> >>> Hi,\n> >>>\n> >>> On 10/16/20 12:02 PM, Laurent Pinchart wrote:\n> >>>> Hi Umang,\n> >>>>\n> >>>> Thank you for the patch.\n> >>>>\n> >>>> On Fri, Oct 16, 2020 at 11:07:54AM +0530, Umang Jain wrote:\n> >>>>> Port the CameraStream's JPEG-encoding bits to PostProcessorJpeg.\n> >>>>> This encapsulates the encoder and EXIF generation code into the\n> >>>>> PostProcessorJpeg layer and removes these specifics related to JPEG,\n> >>>>> from the CameraStream itself.\n> >>>>>\n> >>>>> Signed-off-by: Umang Jain \n> >>>>> Tested-by: Kieran Bingham \n> >>>>> Reviewed-by: Kieran Bingham \n> >>>> Reviewed-by: Laurent Pinchart \n> >>>>\n> >>>> Kieran, would you be able to test this new version, and push it if all\n> >>>> looks good to you ?\n> >>> Yes please, one final testing should be done before actual pushing.\n> >>> Thanks for the reviews. Thanks to Kieran for removing blockers on this\n> >>> and testing efforts :)\n> >>\n> >> Ok - I had a few hit and misses due to apparent hardware failure (the\n> >> camera didn't power up, now up with a full power cycle) - and then the\n> >> cros build not actually building the code.\n> >>\n> >> all of that is solved, and confirmed that it's definitely Umang's code\n> >> running on the device ... successfully ;-)\n> >>\n> >> I'll push these patches now.\n> >>\n> >> Thanks\n> >>\n> >> Kieran\n> >>\n> >>\n> >>>>\n> >>>>> ---\n> >>>>> src/android/camera_device.cpp | 1 +\n> >>>>> src/android/camera_stream.cpp | 77 ++++-------------\n> >>>>> src/android/camera_stream.h | 4 +-\n> >>>>> src/android/jpeg/encoder_libjpeg.cpp | 2 +-\n> >>>>> src/android/jpeg/post_processor_jpeg.cpp | 105 +++++++++++++++++++++++\n> >>>>> src/android/jpeg/post_processor_jpeg.h | 36 ++++++++\n> >>>>> src/android/meson.build | 1 +\n> >>>>> 7 files changed, 164 insertions(+), 62 deletions(-)\n> >>>>> create mode 100644 src/android/jpeg/post_processor_jpeg.cpp\n> >>>>> create mode 100644 src/android/jpeg/post_processor_jpeg.h\n> >>>>>\n> >>>>> diff --git a/src/android/camera_device.cpp\n> >>>>> b/src/android/camera_device.cpp\n> >>>>> index 0983232..d706cf4 100644\n> >>>>> --- a/src/android/camera_device.cpp\n> >>>>> +++ b/src/android/camera_device.cpp\n> >>>>> @@ -7,6 +7,7 @@\n> >>>>> #include \"camera_device.h\"\n> >>>>> #include \"camera_ops.h\"\n> >>>>> +#include \"post_processor.h\"\n> >>>>> #include \n> >>>>> #include \n> >>>>> diff --git a/src/android/camera_stream.cpp\n> >>>>> b/src/android/camera_stream.cpp\n> >>>>> index d8e45c2..1b8afa8 100644\n> >>>>> --- a/src/android/camera_stream.cpp\n> >>>>> +++ b/src/android/camera_stream.cpp\n> >>>>> @@ -9,9 +9,9 @@\n> >>>>> #include \"camera_device.h\"\n> >>>>> #include \"camera_metadata.h\"\n> >>>>> -#include \"jpeg/encoder.h\"\n> >>>>> -#include \"jpeg/encoder_libjpeg.h\"\n> >>>>> -#include \"jpeg/exif.h\"\n> >>>>> +#include \"jpeg/post_processor_jpeg.h\"\n> >>>>> +\n> >>>>> +#include \n> >>>>> using namespace libcamera;\n> >>>>> @@ -45,8 +45,15 @@ CameraStream::CameraStream(CameraDevice\n> >>>>> *cameraDevice, Type type,\n> >>>>> {\n> >>>>> config_ = cameraDevice_->cameraConfiguration();\n> >>>>> - if (type_ == Type::Internal || type_ == Type::Mapped)\n> >>>>> - encoder_ = std::make_unique();\n> >>>>> + if (type_ == Type::Internal || type_ == Type::Mapped) {\n> >>>>> + /*\n> >>>>> + * \\todo There might be multiple post-processors. The logic\n> >>>>> + * which should be instantiated here, is deferred for the\n> >>>>> + * future. For now, we only have PostProcessorJpeg and that\n> >>>>> + * is what we instantiate here.\n> >>>>> + */\n> >>>>> + postProcessor_ =\n> >>>>> std::make_unique(cameraDevice_);\n> >>>>> + }\n> >>>>> if (type == Type::Internal) {\n> >>>>> allocator_ =\n> >>>>> std::make_unique(cameraDevice_->camera());\n> >>>>> @@ -66,8 +73,10 @@ Stream *CameraStream::stream() const\n> >>>>> int CameraStream::configure()\n> >>>>> {\n> >>>>> - if (encoder_) {\n> >>>>> - int ret = encoder_->configure(configuration());\n> >>>>> + if (postProcessor_) {\n> >>>>> + StreamConfiguration output = configuration();\n> >>>>> + output.pixelFormat = formats::MJPEG;\n> >>>>> + int ret = postProcessor_->configure(configuration(), output);\n> >\n> > Hmm, this is strange to me.\n> > |config_| should be used here and |Config::Size| should be set as well\n> > as Format?\n> >\n>\n> As you've already mentioned, this is handled through configuration().\n>\n> config_ is the CameraConfiguration, where configuration() is the Stream\n> Configuration.\n>\n>\n> >>>>> if (ret)\n> >>>>> return ret;\n> >>>>> }\n> >>>>> @@ -90,60 +99,10 @@ int CameraStream::configure()\n> >>>>> int CameraStream::process(const libcamera::FrameBuffer &source,\n> >>>>> MappedCamera3Buffer *dest, CameraMetadata *metadata)\n> >>>>> {\n> >>>>> - if (!encoder_)\n> >>>>> + if (!postProcessor_)\n> >>>>> return 0;\n> >>>>> - /* Set EXIF metadata for various tags. */\n> >>>>> - Exif exif;\n> >>>>> - /* \\todo Set Make and Model from external vendor tags. */\n> >>>>> - exif.setMake(\"libcamera\");\n> >>>>> - exif.setModel(\"cameraModel\");\n> >>>>> - exif.setOrientation(cameraDevice_->orientation());\n> >>>>> - exif.setSize(configuration().size);\n> >>>>> - /*\n> >>>>> - * We set the frame's EXIF timestamp as the time of encode.\n> >>>>> - * Since the precision we need for EXIF timestamp is only one\n> >>>>> - * second, it is good enough.\n> >>>>> - */\n> >>>>> - exif.setTimestamp(std::time(nullptr));\n> >>>>> - if (exif.generate() != 0)\n> >>>>> - LOG(HAL, Error) << \"Failed to generate valid EXIF data\";\n> >>>>> -\n> >>>>> - int jpeg_size = encoder_->encode(&source, dest->maps()[0],\n> >>>>> exif.data());\n> >>>>> - if (jpeg_size < 0) {\n> >>>>> - LOG(HAL, Error) << \"Failed to encode stream image\";\n> >>>>> - return jpeg_size;\n> >>>>> - }\n> >>>>> -\n> >>>>> - /*\n> >>>>> - * Fill in the JPEG blob header.\n> >>>>> - *\n> >>>>> - * The mapped size of the buffer is being returned as\n> >>>>> - * substantially larger than the requested JPEG_MAX_SIZE\n> >>>>> - * (which is referenced from maxJpegBufferSize_). Utilise\n> >>>>> - * this static size to ensure the correct offset of the blob is\n> >>>>> - * determined.\n> >>>>> - *\n> >>>>> - * \\todo Investigate if the buffer size mismatch is an issue or\n> >>>>> - * expected behaviour.\n> >>>>> - */\n> >>>>> - uint8_t *resultPtr = dest->maps()[0].data() +\n> >>>>> - cameraDevice_->maxJpegBufferSize() -\n> >>>>> - sizeof(struct camera3_jpeg_blob);\n> >>>>> - auto *blob = reinterpret_cast >>>>> *>(resultPtr);\n> >>>>> - blob->jpeg_blob_id = CAMERA3_JPEG_BLOB_ID;\n> >>>>> - blob->jpeg_size = jpeg_size;\n> >>>>> -\n> >>>>> - /* Update the JPEG result Metadata. */\n> >>>>> - metadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1);\n> >>>>> -\n> >>>>> - const uint32_t jpeg_quality = 95;\n> >>>>> - metadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 1);\n> >>>>> -\n> >>>>> - const uint32_t jpeg_orientation = 0;\n> >>>>> - metadata->addEntry(ANDROID_JPEG_ORIENTATION, &jpeg_orientation, 1);\n> >>>>> -\n> >>>>> - return 0;\n> >>>>> + return postProcessor_->process(&source, dest->maps()[0], metadata);\n> >>>>> }\n> >>>>> FrameBuffer *CameraStream::getBuffer()\n> >>>>> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h\n> >>>>> index d3925fb..c367a5f 100644\n> >>>>> --- a/src/android/camera_stream.h\n> >>>>> +++ b/src/android/camera_stream.h\n> >>>>> @@ -19,10 +19,10 @@\n> >>>>> #include \n> >>>>> #include \n> >>>>> -class Encoder;\n> >>>>> class CameraDevice;\n> >>>>> class CameraMetadata;\n> >>>>> class MappedCamera3Buffer;\n> >>>>> +class PostProcessor;\n> >>>>> class CameraStream\n> >>>>> {\n> >>>>> @@ -130,7 +130,6 @@ private:\n> >>>>> camera3_stream_t *camera3Stream_;\n> >>>>> unsigned int index_;\n> >>>>> - std::unique_ptr encoder_;\n> >>>>> std::unique_ptr allocator_;\n> >>>>> std::vector buffers_;\n> >>>>> /*\n> >>>>> @@ -138,6 +137,7 @@ private:\n> >>>>> * an std::vector in CameraDevice.\n> >>>>> */\n> >>>>> std::unique_ptr mutex_;\n> >>>>> + std::unique_ptr postProcessor_;\n> >>>>> };\n> >>>>> #endif /* __ANDROID_CAMERA_STREAM__ */\n> >>>>> diff --git a/src/android/jpeg/encoder_libjpeg.cpp\n> >>>>> b/src/android/jpeg/encoder_libjpeg.cpp\n> >>>>> index a77f5b2..8995ba5 100644\n> >>>>> --- a/src/android/jpeg/encoder_libjpeg.cpp\n> >>>>> +++ b/src/android/jpeg/encoder_libjpeg.cpp\n> >>>>> @@ -25,7 +25,7 @@\n> >>>>> using namespace libcamera;\n> >>>>> -LOG_DEFINE_CATEGORY(JPEG)\n> >>>>> +LOG_DECLARE_CATEGORY(JPEG);\n> >>>>> namespace {\n> >>>>> diff --git a/src/android/jpeg/post_processor_jpeg.cpp\n> >>>>> b/src/android/jpeg/post_processor_jpeg.cpp\n> >>>>> new file mode 100644\n> >>>>> index 0000000..753c28e\n> >>>>> --- /dev/null\n> >>>>> +++ b/src/android/jpeg/post_processor_jpeg.cpp\n> >>>>> @@ -0,0 +1,105 @@\n> >>>>> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> >>>>> +/*\n> >>>>> + * Copyright (C) 2020, Google Inc.\n> >>>>> + *\n> >>>>> + * post_processor_jpeg.cpp - JPEG Post Processor\n> >>>>> + */\n> >>>>> +\n> >>>>> +#include \"post_processor_jpeg.h\"\n> >>>>> +\n> >>>>> +#include \"../camera_device.h\"\n> >>>>> +#include \"../camera_metadata.h\"\n> >>>>> +#include \"encoder_libjpeg.h\"\n> >>>>> +#include \"exif.h\"\n> >>>>> +\n> >>>>> +#include \n> >>>>> +\n> >>>>> +#include \"libcamera/internal/log.h\"\n> >>>>> +\n> >>>>> +using namespace libcamera;\n> >>>>> +\n> >>>>> +LOG_DEFINE_CATEGORY(JPEG);\n> >>>>> +\n> >>>>> +PostProcessorJpeg::PostProcessorJpeg(CameraDevice *device)\n> >>>>> + : cameraDevice_(device)\n> >>>>> +{\n> >>>>> +}\n> >>>>> +\n> >>>>> +int PostProcessorJpeg::configure(const StreamConfiguration &inCfg,\n> >>>>> + const StreamConfiguration &outCfg)\n> >>>>> +{\n> >>>>> + if (inCfg.size != outCfg.size) {\n> >>>>> + LOG(JPEG, Error) << \"Mismatch of input and output stream\n> >>>>> sizes\";\n> >>>>> + return -EINVAL;\n> >>>>> + }\n> >>>>> +\n> >>>>> + if (outCfg.pixelFormat != formats::MJPEG) {\n> >>>>> + LOG(JPEG, Error) << \"Output stream pixel format is not JPEG\";\n> >>>>> + return -EINVAL;\n> >>>>> + }\n> >>>>> +\n> >>>>> + streamSize_ = outCfg.size;\n> >>>>> + encoder_ = std::make_unique();\n> >>>>> +\n> >>>>> + return encoder_->configure(inCfg);\n> >>>>> +}\n> >>>>> +\n> >>>>> +int PostProcessorJpeg::process(const libcamera::FrameBuffer *source,\n> >>>>> + const libcamera::Span &destination,\n> >>>>> + CameraMetadata *metadata)\n> >>>>> +{\n> >>>>> + if (!encoder_)\n> >>>>> + return 0;\n> >>>>> +\n> >>>>> + /* Set EXIF metadata for various tags. */\n> >>>>> + Exif exif;\n> >>>>> + /* \\todo Set Make and Model from external vendor tags. */\n> >>>>> + exif.setMake(\"libcamera\");\n> >>>>> + exif.setModel(\"cameraModel\");\n> >>>>> + exif.setOrientation(cameraDevice_->orientation());\n> >>>>> + exif.setSize(streamSize_);\n> >>>>> + /*\n> >>>>> + * We set the frame's EXIF timestamp as the time of encode.\n> >>>>> + * Since the precision we need for EXIF timestamp is only one\n> >>>>> + * second, it is good enough.\n> >>>>> + */\n> >>>>> + exif.setTimestamp(std::time(nullptr));\n> >>>>> + if (exif.generate() != 0)\n> >>>>> + LOG(JPEG, Error) << \"Failed to generate valid EXIF data\";\n> >\n> > Shall we handle this case?\n>\n> I recall the discussion from this when it went in originally was that\n> there's not much we can do.\n>\n> If we fail to generate the exif, we probably still want to output the\n> image, as it's not fatal, but the error highlights it has happened.\n>\n> Of course that won't be visible to the end consumer, though the lack of\n> exif tags will be.\n>\n> Do you think that if the exif tags can't be created, the image should\n> fail entirely?\n>\n>\n\nI got it. I don't know much about this exif process.\nSo if the image encoding process can proceed with this fatal, I am ok\nto not handle this as the entire fatal.\n\n> >>>>> +\n> >>>>> + int jpeg_size = encoder_->encode(source, destination, exif.data());\n> >>>>> + if (jpeg_size < 0) {\n> >>>>> + LOG(JPEG, Error) << \"Failed to encode stream image\";\n> >>>>> + return jpeg_size;\n> >>>>> + }\n> >>>>> +\n> >>>>> + /*\n> >>>>> + * Fill in the JPEG blob header.\n> >>>>> + *\n> >>>>> + * The mapped size of the buffer is being returned as\n> >>>>> + * substantially larger than the requested JPEG_MAX_SIZE\n> >>>>> + * (which is referenced from maxJpegBufferSize_). Utilise\n> >>>>> + * this static size to ensure the correct offset of the blob is\n> >>>>> + * determined.\n> >>>>> + *\n> >>>>> + * \\todo Investigate if the buffer size mismatch is an issue or\n> >>>>> + * expected behaviour.\n> >>>>> + */\n> >>>>> + uint8_t *resultPtr = destination.data() +\n> >>>>> + cameraDevice_->maxJpegBufferSize() -\n> >>>>> + sizeof(struct camera3_jpeg_blob);\n> >>>>> + auto *blob = reinterpret_cast >>>>> *>(resultPtr);\n> >>>>> + blob->jpeg_blob_id = CAMERA3_JPEG_BLOB_ID;\n> >>>>> + blob->jpeg_size = jpeg_size;\n> >>>>> +\n> >>>>> + /* Update the JPEG result Metadata. */\n> >>>>> + metadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1);\n> >>>>> +\n> >>>>> + const uint32_t jpeg_quality = 95;\n> >>>>> + metadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 1);\n> >>>>> +\n> >>>>> + const uint32_t jpeg_orientation = 0;\n> >>>>> + metadata->addEntry(ANDROID_JPEG_ORIENTATION, &jpeg_orientation, 1);\n> >>>>> +\n> >>>>> + return 0;\n> >>>>> +}\n> >>>>> diff --git a/src/android/jpeg/post_processor_jpeg.h\n> >>>>> b/src/android/jpeg/post_processor_jpeg.h\n> >>>>> new file mode 100644\n> >>>>> index 0000000..62c8650\n> >>>>> --- /dev/null\n> >>>>> +++ b/src/android/jpeg/post_processor_jpeg.h\n> >>>>> @@ -0,0 +1,36 @@\n> >>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >>>>> +/*\n> >>>>> + * Copyright (C) 2020, Google Inc.\n> >>>>> + *\n> >>>>> + * post_processor_jpeg.h - JPEG Post Processor\n> >>>>> + */\n> >>>>> +#ifndef __ANDROID_POST_PROCESSOR_JPEG_H__\n> >>>>> +#define __ANDROID_POST_PROCESSOR_JPEG_H__\n> >>>>> +\n> >>>>> +#include \"../post_processor.h\"\n> >>>>> +\n> >>>>> +#include \n> >>>>> +\n> >>>>> +#include \"libcamera/internal/buffer.h\"\n> >>>>> +\n> >>>>> +class Encoder;\n> >>>>> +class CameraDevice;\n> >>>>> +\n> >>>>> +class PostProcessorJpeg : public PostProcessor\n> >>>>> +{\n> >>>>> +public:\n> >>>>> + PostProcessorJpeg(CameraDevice *device);\n> >>>>> +\n> >>>>> + int configure(const libcamera::StreamConfiguration &incfg,\n> >>>>> + const libcamera::StreamConfiguration &outcfg) override;\n> >>>>> + int process(const libcamera::FrameBuffer *source,\n> >>>>> + const libcamera::Span &destination,\n> >>>>> + CameraMetadata *metadata) override;\n> >>>>> +\n> >>>>> +private:\n> >>>>> + CameraDevice *cameraDevice_;\n> >\n> > nit: CameraDevice const *cameraDevice_ to represent |cameraDevice_| is\n> > owned by a client.\n> >\n>\n> Const would be more accurate here indeed I think.\n>\n> Could you submit that as a patch perhaps please?\n>\n\nI submitted the patch. PTAL.\n\nRegards,\n-Hiro\n\n>\n> >>>>> + std::unique_ptr encoder_;\n> >>>>> + libcamera::Size streamSize_;\n> >>>>> +};\n> >>>>> +\n> >>>>> +#endif /* __ANDROID_POST_PROCESSOR_JPEG_H__ */\n> >>>>> diff --git a/src/android/meson.build b/src/android/meson.build\n> >>>>> index b2b2293..5a01bea 100644\n> >>>>> --- a/src/android/meson.build\n> >>>>> +++ b/src/android/meson.build\n> >>>>> @@ -24,6 +24,7 @@ android_hal_sources = files([\n> >>>>> 'camera_worker.cpp',\n> >>>>> 'jpeg/encoder_libjpeg.cpp',\n> >>>>> 'jpeg/exif.cpp',\n> >>>>> + 'jpeg/post_processor_jpeg.cpp',\n> >>>>> ])\n> >>>>> android_camera_metadata_sources = files([\n> >>>\n> >>> _______________________________________________\n> >>> libcamera-devel mailing list\n> >>> libcamera-devel@lists.libcamera.org\n> >>> https://lists.libcamera.org/listinfo/libcamera-devel\n> >>\n> >> --\n> >> Regards\n> >> --\n> >> Kieran\n> >> _______________________________________________\n> >> libcamera-devel mailing list\n> >> libcamera-devel@lists.libcamera.org\n> >> https://lists.libcamera.org/listinfo/libcamera-devel\n>\n> --\n> Regards\n> --\n> Kieran","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 36F1CBDB1F\n\tfor ;\n\tMon, 19 Oct 2020 10:30:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BF36161373;\n\tMon, 19 Oct 2020 12:30:07 +0200 (CEST)","from mail-ed1-x542.google.com (mail-ed1-x542.google.com\n\t[IPv6:2a00:1450:4864:20::542])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0B52D60CE6\n\tfor ;\n\tMon, 19 Oct 2020 12:30:07 +0200 (CEST)","by mail-ed1-x542.google.com with SMTP id t20so9641740edr.11\n\tfor ;\n\tMon, 19 Oct 2020 03:30:07 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"DtlfPVjR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=H6Bzp54kJAedq1rzddEMELho6O9lFSiRxCS9Je9xOMY=;\n\tb=DtlfPVjRpXSyTIRZFc1DG2WIEyyr1DTUSQrDrjimh25AQoD2I8COGqR4pfKI1IbcWv\n\tKzSPKxQM8o8GchiNjfgIXCa3eGL67rTRh2hApsZHPCOA0VYoR7oP9swS8AwWqadU7y31\n\t3lIh41bi4RXedO9hhco3oUz4DF838DtZJn8E4=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=H6Bzp54kJAedq1rzddEMELho6O9lFSiRxCS9Je9xOMY=;\n\tb=FY+CxlGpfF4avdwPQxAh1stjRP9riwiuLVjTqLPwHzi3x49xEmqEsXlitNtP6f6wj0\n\tGrV4PAUr4mnmJl9S99LKtoNTosD+ZfgkDakBEzJ8Hsg0QoQFeumRy9TACIP2ElGjwNpn\n\t7TAb8Iwi2guKg7nOsUIQfZ2FAa/nQZuwQ3RYCu3HIM9fsj4QEhtunYu2p/rRD+ijCyvd\n\t5Sy9TiNj3kUozKyAqp1dJzn7DVzX4gwJpPYLYPbMzGuqRo7k6j7DqEtBqhSkxhwBYf9j\n\txHJgELXQQY0sNPMaIzjSgEFqWozlY9tHxxZWtypOIY2JtiKRya5lBr86/clx8XTtyIQb\n\tZQOA==","X-Gm-Message-State":"AOAM5318aNhh7XLMXDR1qfkB7yF/NBzl6O7WTOKBv0yWrhG42UTUgFXy\n\tMJWSn2pXkgmi8QDghtwLSApgxwL/A/k0qaL3RRvQNA==","X-Google-Smtp-Source":"ABdhPJz6LWKvBrsXv6K4VPTcab/ryf59RFJa4w1569QdvInM0hP5x6FYM3W+Plfrnl2UyVLbcNCugRQiHsNucV24z8I=","X-Received":"by 2002:a05:6402:602:: with SMTP id\n\tn2mr17668476edv.327.1603103406459; \n\tMon, 19 Oct 2020 03:30:06 -0700 (PDT)","MIME-Version":"1.0","References":"<20201016053754.17251-1-email@uajain.com>\n\t<20201016053754.17251-3-email@uajain.com>\n\t<20201016063253.GK3829@pendragon.ideasonboard.com>\n\t<617edcee-9cbc-e3ac-8c82-8ebbc68b3f28@uajain.com>\n\t<92d54fd6-5c90-29bb-dd6c-2f9c3a7ccebd@ideasonboard.com>\n\t\n\t<1c29b57b-4e9f-f973-0476-f9312575fcec@ideasonboard.com>","In-Reply-To":"<1c29b57b-4e9f-f973-0476-f9312575fcec@ideasonboard.com>","From":"Hirokazu Honda ","Date":"Mon, 19 Oct 2020 19:29:54 +0900","Message-ID":"","To":"kieran.bingham@ideasonboard.com","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] android: jpeg: Port to\n\tPostProcessor interface","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]