[libcamera-devel,RFC,5/5] WIP: android: jpeg: Use maps() and clean up
diff mbox series

Message ID 20210811124015.2116188-6-hiroh@chromium.org
State Superseded
Headers show
Series
  • MappedFrameBuffer::maps() returns the plane address
Related show

Commit Message

Hirokazu Honda Aug. 11, 2021, 12:40 p.m. UTC 
WIP

Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
---
 src/android/jpeg/encoder.h               |  7 +++--
 src/android/jpeg/encoder_libjpeg.cpp     | 32 ++++++---------------
 src/android/jpeg/encoder_libjpeg.h       | 10 ++-----
 src/android/jpeg/post_processor_jpeg.cpp | 21 ++++++++++----
 src/android/jpeg/thumbnailer.cpp         | 36 ++++++++++++++++++++----
 src/android/jpeg/thumbnailer.h           |  4 ++-
 src/libcamera/mapped_framebuffer.cpp     |  2 ++
 7 files changed, 66 insertions(+), 46 deletions(-)

Patch
diff mbox series 

diff --git a/src/android/jpeg/encoder.h b/src/android/jpeg/encoder.h
index a28522f4..2b9b8fca 100644
--- a/src/android/jpeg/encoder.h
+++ b/src/android/jpeg/encoder.h
@@ -8,17 +8,18 @@ 
 #define __ANDROID_JPEG_ENCODER_H__
 
 #include <libcamera/base/span.h>
-
-#include <libcamera/framebuffer.h>
 #include <libcamera/stream.h>
 
+#include "libcamera/internal/framebuffer.h"
+#include "libcamera/internal/mapped_framebuffer.h"
+
 class Encoder
 {
 public:
 	virtual ~Encoder() = default;
 
 	virtual int configure(const libcamera::StreamConfiguration &cfg) = 0;
-	virtual int encode(const libcamera::FrameBuffer &source,
+	virtual int encode(const libcamera::MappedFrameBuffer &source,
 			   libcamera::Span<uint8_t> destination,
 			   libcamera::Span<const uint8_t> exifData,
 			   unsigned int quality) = 0;
diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp
index a7a63601..188aabdb 100644
--- a/src/android/jpeg/encoder_libjpeg.cpp
+++ b/src/android/jpeg/encoder_libjpeg.cpp
@@ -94,7 +94,6 @@  int EncoderLibJpeg::configure(const StreamConfiguration &cfg)
 	compress_.input_components = info.colorSpace == JCS_GRAYSCALE ? 1 : 3;
 
 	jpeg_set_defaults(&compress_);
-
 	pixelFormatInfo_ = &info.pixelFormatInfo;
 
 	nv_ = pixelFormatInfo_->numPlanes() == 2;
@@ -103,9 +102,9 @@  int EncoderLibJpeg::configure(const StreamConfiguration &cfg)
 	return 0;
 }
 
-void EncoderLibJpeg::compressRGB(Span<const uint8_t> frame)
+void EncoderLibJpeg::compressRGB(const libcamera::MappedFrameBuffer &frame)
 {
-	unsigned char *src = const_cast<unsigned char *>(frame.data());
+	unsigned char *src = const_cast<unsigned char *>(frame.maps()[0].data());
 	/* \todo Stride information should come from buffer configuration. */
 	unsigned int stride = pixelFormatInfo_->stride(compress_.image_width, 0);
 
@@ -121,7 +120,7 @@  void EncoderLibJpeg::compressRGB(Span<const uint8_t> frame)
  * Compress the incoming buffer from a supported NV format.
  * This naively unpacks the semi-planar NV12 to a YUV888 format for libjpeg.
  */
-void EncoderLibJpeg::compressNV(Span<const uint8_t> frame)
+void EncoderLibJpeg::compressNV(const libcamera::MappedFrameBuffer &frame)
 {
 	uint8_t tmprowbuf[compress_.image_width * 3];
 
@@ -143,8 +142,8 @@  void EncoderLibJpeg::compressNV(Span<const uint8_t> frame)
 	unsigned int cb_pos = nvSwap_ ? 1 : 0;
 	unsigned int cr_pos = nvSwap_ ? 0 : 1;
 
-	const unsigned char *src = frame.data();
-	const unsigned char *src_c = src + y_stride * compress_.image_height;
+	const unsigned char *src = frame.maps()[0].data();
+	const unsigned char *src_c = frame.maps()[1].data();
 
 	JSAMPROW row_pointer[1];
 	row_pointer[0] = &tmprowbuf[0];
@@ -152,7 +151,7 @@  void EncoderLibJpeg::compressNV(Span<const uint8_t> frame)
 	for (unsigned int y = 0; y < compress_.image_height; y++) {
 		unsigned char *dst = &tmprowbuf[0];
 
-		const unsigned char *src_y = src + y * compress_.image_width;
+		const unsigned char *src_y = src + y * y_stride;
 		const unsigned char *src_cb = src_c + (y / vertSubSample) * c_stride + cb_pos;
 		const unsigned char *src_cr = src_c + (y / vertSubSample) * c_stride + cr_pos;
 
@@ -178,20 +177,7 @@  void EncoderLibJpeg::compressNV(Span<const uint8_t> frame)
 	}
 }
 
-int EncoderLibJpeg::encode(const FrameBuffer &source, Span<uint8_t> dest,
-			   Span<const uint8_t> exifData, unsigned int quality)
-{
-	MappedFrameBuffer frame(&source, MappedFrameBuffer::MapFlag::Read);
-	if (!frame.isValid()) {
-		LOG(JPEG, Error) << "Failed to map FrameBuffer : "
-				 << strerror(frame.error());
-		return frame.error();
-	}
-
-	return encode(frame.maps()[0], dest, exifData, quality);
-}
-
-int EncoderLibJpeg::encode(Span<const uint8_t> src, Span<uint8_t> dest,
+int EncoderLibJpeg::encode(const MappedFrameBuffer &source, Span<uint8_t> dest,
 			   Span<const uint8_t> exifData, unsigned int quality)
 {
 	unsigned char *destination = dest.data();
@@ -221,9 +207,9 @@  int EncoderLibJpeg::encode(Span<const uint8_t> src, Span<uint8_t> dest,
 			 << "x" << compress_.image_height;
 
 	if (nv_)
-		compressNV(src);
+		compressNV(source);
 	else
-		compressRGB(src);
+		compressRGB(source);
 
 	jpeg_finish_compress(&compress_);
 
diff --git a/src/android/jpeg/encoder_libjpeg.h b/src/android/jpeg/encoder_libjpeg.h
index 61fbd1a6..588756aa 100644
--- a/src/android/jpeg/encoder_libjpeg.h
+++ b/src/android/jpeg/encoder_libjpeg.h
@@ -20,18 +20,14 @@  public:
 	~EncoderLibJpeg();
 
 	int configure(const libcamera::StreamConfiguration &cfg) override;
-	int encode(const libcamera::FrameBuffer &source,
+	int encode(const libcamera::MappedFrameBuffer &source,
 		   libcamera::Span<uint8_t> destination,
 		   libcamera::Span<const uint8_t> exifData,
 		   unsigned int quality) override;
-	int encode(libcamera::Span<const uint8_t> source,
-		   libcamera::Span<uint8_t> destination,
-		   libcamera::Span<const uint8_t> exifData,
-		   unsigned int quality);
 
 private:
-	void compressRGB(libcamera::Span<const uint8_t> frame);
-	void compressNV(libcamera::Span<const uint8_t> frame);
+	void compressRGB(const libcamera::MappedFrameBuffer &frame);
+	void compressNV(const libcamera::MappedFrameBuffer &frame);
 
 	struct jpeg_compress_struct compress_;
 	struct jpeg_error_mgr jerr_;
diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp
index 3160a784..354e4fdd 100644
--- a/src/android/jpeg/post_processor_jpeg.cpp
+++ b/src/android/jpeg/post_processor_jpeg.cpp
@@ -56,8 +56,7 @@  void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source,
 					  std::vector<unsigned char> *thumbnail)
 {
 	/* Stores the raw scaled-down thumbnail bytes. */
-	std::vector<unsigned char> rawThumbnail;
-
+	std::unique_ptr<MappedFrameBuffer> rawThumbnail;
 	thumbnailer_.createThumbnail(source, targetSize, &rawThumbnail);
 
 	StreamConfiguration thCfg;
@@ -65,14 +64,17 @@  void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source,
 	thCfg.pixelFormat = thumbnailer_.pixelFormat();
 	int ret = thumbnailEncoder_.configure(thCfg);
 
-	if (!rawThumbnail.empty() && !ret) {
+	if (rawThumbnail && !ret) {
 		/*
 		 * \todo Avoid value-initialization of all elements of the
 		 * vector.
 		 */
-		thumbnail->resize(rawThumbnail.size());
+		size_t thumbnail_size = 0;
+		for (const MappedBuffer::Plane &plane : rawThumbnail->maps())
+			thumbnail_size += plane.size();
+		thumbnail->resize(thumbnail_size);
 
-		int jpeg_size = thumbnailEncoder_.encode(rawThumbnail,
+		int jpeg_size = thumbnailEncoder_.encode(*rawThumbnail,
 							 *thumbnail, {}, quality);
 		thumbnail->resize(jpeg_size);
 
@@ -177,7 +179,14 @@  int PostProcessorJpeg::process(const FrameBuffer &source,
 	const uint8_t quality = ret ? *entry.data.u8 : 95;
 	resultMetadata->addEntry(ANDROID_JPEG_QUALITY, quality);
 
-	int jpeg_size = encoder_->encode(source, destination->plane(0),
+	MappedFrameBuffer mapped(&source, MappedFrameBuffer::MapFlag::Read);
+	if (!mapped.isValid()) {
+		LOG(JPEG, Error) << "Failed to map FrameBuffer : "
+				 << strerror(mapped.error());
+		return mapped.error();
+	}
+
+	int jpeg_size = encoder_->encode(mapped, destination->plane(0),
 					 exif.data(), quality);
 	if (jpeg_size < 0) {
 		LOG(JPEG, Error) << "Failed to encode stream image";
diff --git a/src/android/jpeg/thumbnailer.cpp b/src/android/jpeg/thumbnailer.cpp
index 79d83926..e8fa9f84 100644
--- a/src/android/jpeg/thumbnailer.cpp
+++ b/src/android/jpeg/thumbnailer.cpp
@@ -8,6 +8,7 @@ 
 #include "thumbnailer.h"
 
 #include <libcamera/base/log.h>
+#include <libcamera/base/span.h>
 
 #include <libcamera/formats.h>
 
@@ -17,6 +18,24 @@  using namespace libcamera;
 
 LOG_DEFINE_CATEGORY(Thumbnailer)
 
+namespace {
+class MappedThumbnailBuffer : public MappedFrameBuffer
+{
+public:
+	MappedThumbnailBuffer(std::vector<uint8_t> buffer,
+			      const std::vector<Span<uint8_t>> &planes)
+		: MappedFrameBuffer(nullptr, MapFlag::Read),
+		  buffer_(std::move(buffer))
+	{
+		planes_ = planes;
+	}
+
+private:
+	std::vector<uint8_t> buffer_;
+};
+
+} // namespace
+
 Thumbnailer::Thumbnailer()
 	: valid_(false)
 {
@@ -39,7 +58,7 @@  void Thumbnailer::configure(const Size &sourceSize, PixelFormat pixelFormat)
 
 void Thumbnailer::createThumbnail(const FrameBuffer &source,
 				  const Size &targetSize,
-				  std::vector<unsigned char> *destination)
+				  std::unique_ptr<MappedFrameBuffer> *destination)
 {
 	MappedFrameBuffer frame(&source, MappedFrameBuffer::MapFlag::Read);
 	if (!frame.isValid()) {
@@ -63,14 +82,19 @@  void Thumbnailer::createThumbnail(const FrameBuffer &source,
 
 	/* Image scaling block implementing nearest-neighbour algorithm. */
 	unsigned char *src = static_cast<unsigned char *>(frame.maps()[0].data());
-	unsigned char *srcC = src + sh * sw;
+	unsigned char *srcC = static_cast<unsigned char *>(frame.maps()[1].data());
 	unsigned char *srcCb, *srcCr;
 	unsigned char *dstY, *srcY;
 
-	size_t dstSize = (th * tw) + ((th / 2) * tw);
-	destination->resize(dstSize);
-	unsigned char *dst = destination->data();
-	unsigned char *dstC = dst + th * tw;
+	std::vector<uint8_t> buffer(th * tw + (th / 2) * tw);
+	std::vector<Span<uint8_t>> dstPlanes = {
+		{ buffer.data(), th * tw },
+		{ buffer.data() + th * tw, (th / 2) * tw }
+	};
+	unsigned char *dst = static_cast<unsigned char *>(dstPlanes[0].data());
+	unsigned char *dstC = static_cast<unsigned char *>(dstPlanes[1].data());
+	*destination = std::make_unique<MappedThumbnailBuffer>(std::move(buffer),
+							       dstPlanes);
 
 	for (unsigned int y = 0; y < th; y += 2) {
 		unsigned int sourceY = (sh * y + th / 2) / th;
diff --git a/src/android/jpeg/thumbnailer.h b/src/android/jpeg/thumbnailer.h
index 4d086c49..9c938588 100644
--- a/src/android/jpeg/thumbnailer.h
+++ b/src/android/jpeg/thumbnailer.h
@@ -11,6 +11,7 @@ 
 #include <libcamera/geometry.h>
 
 #include "libcamera/internal/formats.h"
+#include "libcamera/internal/mapped_framebuffer.h"
 
 class Thumbnailer
 {
@@ -21,7 +22,8 @@  public:
 		       libcamera::PixelFormat pixelFormat);
 	void createThumbnail(const libcamera::FrameBuffer &source,
 			     const libcamera::Size &targetSize,
-			     std::vector<unsigned char> *dest);
+			     std::unique_ptr<libcamera::MappedFrameBuffer> *dest);
+
 	const libcamera::PixelFormat &pixelFormat() const { return pixelFormat_; }
 
 private:
diff --git a/src/libcamera/mapped_framebuffer.cpp b/src/libcamera/mapped_framebuffer.cpp
index 5d5b02f1..e5647127 100644
--- a/src/libcamera/mapped_framebuffer.cpp
+++ b/src/libcamera/mapped_framebuffer.cpp
@@ -177,6 +177,8 @@  MappedBuffer::~MappedBuffer()
  */
 MappedFrameBuffer::MappedFrameBuffer(const FrameBuffer *buffer, MapFlags flags)
 {
+	if (!buffer)
+		return;
 	ASSERT(!buffer->planes().empty());
 	planes_.reserve(buffer->planes().size());