[{"id":26221,"web_url":"https://patchwork.libcamera.org/comment/26221/","msgid":"","date":"2023-01-15T23:34:11","subject":"Re: [libcamera-devel] [PATCH v8 5/7] Move generateThumbnail from\n\tPostProcessorJpeg to Encoder","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Harvey,\n\nThank you for the patch.\n\nOn Wed, Dec 14, 2022 at 09:33:28AM +0000, Harvey Yang via libcamera-devel wrote:\n> From: Harvey Yang \n> \n> In the following patch, generateThumbnail will have a different\n> implementation in the jea encoder. Therefore, this patch moves the\n> generateThumbnail function from PostProcessorJpeg to Encoder.\n> \n> Signed-off-by: Harvey Yang \n> ---\n> src/android/jpeg/encoder.h | 4 ++\n> src/android/jpeg/encoder_libjpeg.cpp | 54 +++++++++++++++++++++---\n> src/android/jpeg/encoder_libjpeg.h | 15 ++++---\n> src/android/jpeg/post_processor_jpeg.cpp | 52 +----------------------\n> src/android/jpeg/post_processor_jpeg.h | 11 +----\n> 5 files changed, 66 insertions(+), 70 deletions(-)\n> \n> diff --git a/src/android/jpeg/encoder.h b/src/android/jpeg/encoder.h\n> index b974d367..5f9ef890 100644\n> --- a/src/android/jpeg/encoder.h\n> +++ b/src/android/jpeg/encoder.h\n> @@ -22,4 +22,8 @@ public:\n> \t\t\t libcamera::Span destination,\n> \t\t\t libcamera::Span exifData,\n> \t\t\t unsigned int quality) = 0;\n> +\tvirtual void generateThumbnail(const libcamera::FrameBuffer &source,\n> +\t\t\t\t const libcamera::Size &targetSize,\n> +\t\t\t\t unsigned int quality,\n> +\t\t\t\t std::vector *thumbnail) = 0;\n> };\n> diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp\n> index d849547f..cc5f37be 100644\n> --- a/src/android/jpeg/encoder_libjpeg.cpp\n> +++ b/src/android/jpeg/encoder_libjpeg.cpp\n> @@ -74,7 +74,7 @@ EncoderLibJpeg::~EncoderLibJpeg() = default;\n> \n> int EncoderLibJpeg::configure(const StreamConfiguration &cfg)\n> {\n> -\treturn encoder_.configure(cfg);\n> +\treturn captureEncoder_.configure(cfg);\n> }\n> \n> EncoderLibJpeg::Encoder::Encoder()\n> @@ -197,14 +197,56 @@ int EncoderLibJpeg::encode(const FrameBuffer &source, Span dest,\n> \t\treturn frame.error();\n> \t}\n> \n> -\treturn encoder_.encode(frame.planes(), dest, exifData, quality);\n> +\treturn captureEncoder_.encode(frame.planes(), dest, exifData, quality);\n> }\n> \n> -int EncoderLibJpeg::encode(const std::vector> &src,\n> -\t\t\t\t Span dest, Span exifData,\n> -\t\t\t\t unsigned int quality)\n> +void EncoderLibJpeg::generateThumbnail(const libcamera::FrameBuffer &source,\n> +\t\t\t\t const libcamera::Size &targetSize,\n> +\t\t\t\t unsigned int quality,\n> +\t\t\t\t std::vector *thumbnail)\n> {\n> -\treturn encoder_.encode(src, std::move(dest), std::move(exifData), quality);\n> +\t/* Stores the raw scaled-down thumbnail bytes. */\n> +\tstd::vector rawThumbnail;\n> +\n> +\tthumbnailer_.createThumbnail(source, targetSize, &rawThumbnail);\n> +\n> +\tStreamConfiguration thCfg;\n> +\tthCfg.size = targetSize;\n> +\tthCfg.pixelFormat = thumbnailer_.pixelFormat();\n> +\tint ret = thumbnailEncoder_.configure(thCfg);\n> +\n> +\tif (!rawThumbnail.empty() && !ret) {\n> +\t\t/*\n> +\t\t * \\todo Avoid value-initialization of all elements of the\n> +\t\t * vector.\n> +\t\t */\n> +\t\tthumbnail->resize(rawThumbnail.size());\n> +\n> +\t\t/*\n> +\t\t * Split planes manually as the encoder expects a vector of\n> +\t\t * planes.\n> +\t\t *\n> +\t\t * \\todo Pass a vector of planes directly to\n> +\t\t * Thumbnailer::createThumbnailer above and remove the manual\n> +\t\t * planes split from here.\n> +\t\t */\n> +\t\tstd::vector> thumbnailPlanes;\n> +\t\tconst PixelFormatInfo &formatNV12 =\n> +\t\t\tPixelFormatInfo::info(formats::NV12);\n> +\t\tsize_t yPlaneSize = formatNV12.planeSize(targetSize, 0);\n> +\t\tsize_t uvPlaneSize = formatNV12.planeSize(targetSize, 1);\n> +\t\tthumbnailPlanes.push_back({ rawThumbnail.data(), yPlaneSize });\n> +\t\tthumbnailPlanes.push_back({ rawThumbnail.data() + yPlaneSize,\n> +\t\t\t\t\t uvPlaneSize });\n> +\n> +\t\tint jpegSize = thumbnailEncoder_.encode(thumbnailPlanes, *thumbnail, {},\n> +\t\t\t\t\t\t\tquality);\n\nNitpicking,\n\n\t\tint jpegSize = thumbnailEncoder_.encode(thumbnailPlanes, *thumbnail,\n\t\t\t\t\t\t\t{}, quality);\n\n> +\t\tthumbnail->resize(jpegSize);\n> +\n> +\t\tLOG(JPEG, Debug)\n> +\t\t\t<< \"Thumbnail compress returned \"\n> +\t\t\t<< jpegSize << \" bytes\";\n> +\t}\n> }\n> \n> int EncoderLibJpeg::Encoder::encode(const std::vector> &src,\n> diff --git a/src/android/jpeg/encoder_libjpeg.h b/src/android/jpeg/encoder_libjpeg.h\n> index 97182b96..fa077b8d 100644\n> --- a/src/android/jpeg/encoder_libjpeg.h\n> +++ b/src/android/jpeg/encoder_libjpeg.h\n> @@ -15,6 +15,8 @@\n> \n> #include \n> \n> +#include \"thumbnailer.h\"\n> +\n> class EncoderLibJpeg : public Encoder\n> {\n> public:\n> @@ -26,10 +28,10 @@ public:\n> \t\t libcamera::Span destination,\n> \t\t libcamera::Span exifData,\n> \t\t unsigned int quality) override;\n> -\tint encode(const std::vector> &planes,\n> -\t\t libcamera::Span destination,\n> -\t\t libcamera::Span exifData,\n> -\t\t unsigned int quality);\n> +\tvoid generateThumbnail(const libcamera::FrameBuffer &source,\n> +\t\t\t const libcamera::Size &targetSize,\n> +\t\t\t unsigned int quality,\n> +\t\t\t std::vector *thumbnail) override;\n> \n> private:\n> \tclass Encoder\n> @@ -57,5 +59,8 @@ private:\n> \t\tbool nvSwap_;\n> \t};\n> \n> -\tEncoder encoder_;\n> +\tEncoder captureEncoder_;\n> +\tEncoder thumbnailEncoder_;\n> +\n> +\tThumbnailer thumbnailer_;\n> };\n> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp\n> index 0cf56716..69b18a2e 100644\n> --- a/src/android/jpeg/post_processor_jpeg.cpp\n> +++ b/src/android/jpeg/post_processor_jpeg.cpp\n> @@ -44,60 +44,11 @@ int PostProcessorJpeg::configure(const StreamConfiguration &inCfg,\n> \n> \tstreamSize_ = outCfg.size;\n> \n> -\tthumbnailer_.configure(inCfg.size, inCfg.pixelFormat);\n> -\n> \tencoder_ = std::make_unique();\n> \n> \treturn encoder_->configure(inCfg);\n> }\n> \n> -void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source,\n> -\t\t\t\t\t const Size &targetSize,\n> -\t\t\t\t\t unsigned int quality,\n> -\t\t\t\t\t std::vector *thumbnail)\n> -{\n> -\t/* Stores the raw scaled-down thumbnail bytes. */\n> -\tstd::vector rawThumbnail;\n> -\n> -\tthumbnailer_.createThumbnail(source, targetSize, &rawThumbnail);\n> -\n> -\tStreamConfiguration thCfg;\n> -\tthCfg.size = targetSize;\n> -\tthCfg.pixelFormat = thumbnailer_.pixelFormat();\n> -\tint ret = thumbnailEncoder_.configure(thCfg);\n> -\n> -\tif (!rawThumbnail.empty() && !ret) {\n> -\t\t/*\n> -\t\t * \\todo Avoid value-initialization of all elements of the\n> -\t\t * vector.\n> -\t\t */\n> -\t\tthumbnail->resize(rawThumbnail.size());\n> -\n> -\t\t/*\n> -\t\t * Split planes manually as the encoder expects a vector of\n> -\t\t * planes.\n> -\t\t *\n> -\t\t * \\todo Pass a vector of planes directly to\n> -\t\t * Thumbnailer::createThumbnailer above and remove the manual\n> -\t\t * planes split from here.\n> -\t\t */\n> -\t\tstd::vector> thumbnailPlanes;\n> -\t\tconst PixelFormatInfo &formatNV12 = PixelFormatInfo::info(formats::NV12);\n> -\t\tsize_t yPlaneSize = formatNV12.planeSize(targetSize, 0);\n> -\t\tsize_t uvPlaneSize = formatNV12.planeSize(targetSize, 1);\n> -\t\tthumbnailPlanes.push_back({ rawThumbnail.data(), yPlaneSize });\n> -\t\tthumbnailPlanes.push_back({ rawThumbnail.data() + yPlaneSize, uvPlaneSize });\n> -\n> -\t\tint jpeg_size = thumbnailEncoder_.encode(thumbnailPlanes,\n> -\t\t\t\t\t\t\t *thumbnail, {}, quality);\n> -\t\tthumbnail->resize(jpeg_size);\n> -\n> -\t\tLOG(JPEG, Debug)\n> -\t\t\t<< \"Thumbnail compress returned \"\n> -\t\t\t<< jpeg_size << \" bytes\";\n> -\t}\n> -}\n> -\n> void PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBuffer)\n> {\n> \tASSERT(encoder_);\n> @@ -164,7 +115,8 @@ void PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBu\n> \n> \t\tif (thumbnailSize != Size(0, 0)) {\n> \t\t\tstd::vector thumbnail;\n> -\t\t\tgenerateThumbnail(source, thumbnailSize, quality, &thumbnail);\n> +\t\t\tencoder_->generateThumbnail(source, thumbnailSize,\n> +\t\t\t\t\t\t quality, &thumbnail);\n> \t\t\tif (!thumbnail.empty())\n> \t\t\t\texif.setThumbnail(std::move(thumbnail), Exif::Compression::JPEG);\n> \t\t}\n> diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h\n> index 98309b01..55b23d7d 100644\n> --- a/src/android/jpeg/post_processor_jpeg.h\n> +++ b/src/android/jpeg/post_processor_jpeg.h\n> @@ -8,11 +8,11 @@\n> #pragma once\n> \n> #include \"../post_processor.h\"\n> -#include \"encoder_libjpeg.h\"\n> -#include \"thumbnailer.h\"\n> \n> #include \n> \n> +#include \"encoder.h\"\n> +\n\nThis can be replaced by a forward declaration.\n\nNo need to resubmit for just this, I can also handle it when applying,\nadding my\n\nReviewed-by: Laurent Pinchart \n\n> class CameraDevice;\n> \n> class PostProcessorJpeg : public PostProcessor\n> @@ -25,14 +25,7 @@ public:\n> \tvoid process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) override;\n> \n> private:\n> -\tvoid generateThumbnail(const libcamera::FrameBuffer &source,\n> -\t\t\t const libcamera::Size &targetSize,\n> -\t\t\t unsigned int quality,\n> -\t\t\t std::vector *thumbnail);\n> -\n> \tCameraDevice *const cameraDevice_;\n> \tstd::unique_ptr encoder_;\n> \tlibcamera::Size streamSize_;\n> -\tEncoderLibJpeg thumbnailEncoder_;\n> -\tThumbnailer thumbnailer_;\n> };","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 DBA36C3240\n\tfor ;\n\tSun, 15 Jan 2023 23:34:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 23B47625D8;\n\tMon, 16 Jan 2023 00:34:13 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9C8FF625D0\n\tfor ;\n\tMon, 16 Jan 2023 00:34:11 +0100 (CET)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DFE19997;\n\tMon, 16 Jan 2023 00:34:10 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1673825653;\n\tbh=uQrT29NpeWDKCPFCjByn1JjCN8Qaq4wA6hob8Zw2Kqo=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=pUR3jo6MPsT+GrCHmqr9teg59R/r1MyaMiokBQ+d777QneCYmwkd3l7iuKXLPCZ44\n\tjgcybORIwepr84SRhoqZXq/KwMk124qWiTeHF1ikvkOV5wz5HxE/QokYV6EeDQrz+b\n\thSoCQ/oyoaZ7B1ppBl8OzakkjR5Eyk9OxeAnWOY6q8QNp73t41AUjTU65cJ+kOk/1n\n\to1kxkUTBVsM4qoNHFsIBqrfCXCXgxbqrr4D4K6MpHVZ8HLz2hwJaQVQ3X8hb5w1PAX\n\t+kWmQtuMK6A5fOsyptR55JQeXV9Ma868eBXornCznJZ7CTk5B56Z06uyYlBggvWr5t\n\tu6gvzZvxIM9Zw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1673825651;\n\tbh=uQrT29NpeWDKCPFCjByn1JjCN8Qaq4wA6hob8Zw2Kqo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=gfzn1rnkVdhd6UI2U+OgWp09Kj36vRENiAYP8YqpBXc0aGGxebPUTE2ugP92hiib2\n\tXzLSw+tr333C3wt7X3PNgR6yUH0kyW1o9ta1/I+WRNpEp68ch1K8MQ/fKOT/kPJpwe\n\tsc/5ilOgl8TJk4IQ8xXQesA/y9UgJ+VCSYEuzwR4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"gfzn1rnk\"; dkim-atps=neutral","Date":"Mon, 16 Jan 2023 01:34:11 +0200","To":"Harvey Yang ","Message-ID":"","References":"<20221214093330.3345421-1-chenghaoyang@google.com>\n\t<20221214093330.3345421-6-chenghaoyang@google.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20221214093330.3345421-6-chenghaoyang@google.com>","Subject":"Re: [libcamera-devel] [PATCH v8 5/7] Move generateThumbnail from\n\tPostProcessorJpeg to Encoder","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]