[libcamera-devel] [RFC PATCH 2/3] android: jpeg: Port to PostProcessor interface

Umang Jain email at uajain.com
Thu Oct 8 16:10:37 CEST 2020


Remove the existing Encoder interface completely and use the
PostProcessor interface instead.

Now the ::encode() function will be called by PostProcessor::process()
internally and will not be directly exposed in CameraStream. Similarly,
other operations can be introduced internally in the PostProcessorJpeg,
and can be called through its process() interface.

Signed-off-by: Umang Jain <email at uajain.com>
---
 src/android/camera_device.h                   |  1 -
 src/android/camera_stream.cpp                 | 74 +++------------
 src/android/camera_stream.h                   |  9 +-
 src/android/jpeg/encoder.h                    | 25 -----
 ...er_libjpeg.cpp => post_processor_jpeg.cpp} | 92 +++++++++++++++++--
 ...ncoder_libjpeg.h => post_processor_jpeg.h} | 27 ++++--
 src/android/meson.build                       |  2 +-
 7 files changed, 122 insertions(+), 108 deletions(-)
 delete mode 100644 src/android/jpeg/encoder.h
 rename src/android/jpeg/{encoder_libjpeg.cpp => post_processor_jpeg.cpp} (67%)
 rename src/android/jpeg/{encoder_libjpeg.h => post_processor_jpeg.h} (55%)

diff --git a/src/android/camera_device.h b/src/android/camera_device.h
index 777d1a3..25de12e 100644
--- a/src/android/camera_device.h
+++ b/src/android/camera_device.h
@@ -25,7 +25,6 @@
 #include "libcamera/internal/message.h"
 
 #include "camera_stream.h"
-#include "jpeg/encoder.h"
 
 class CameraMetadata;
 
diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
index eac1480..ed3bb41 100644
--- a/src/android/camera_stream.cpp
+++ b/src/android/camera_stream.cpp
@@ -9,9 +9,7 @@
 
 #include "camera_device.h"
 #include "camera_metadata.h"
-#include "jpeg/encoder.h"
-#include "jpeg/encoder_libjpeg.h"
-#include "jpeg/exif.h"
+#include "jpeg/post_processor_jpeg.h"
 
 using namespace libcamera;
 
@@ -24,8 +22,15 @@ CameraStream::CameraStream(CameraDevice *cameraDevice, Type type,
 {
 	config_ = cameraDevice_->cameraConfiguration();
 
-	if (type_ == Type::Internal || type_ == Type::Mapped)
-		encoder_ = std::make_unique<EncoderLibJpeg>();
+	if (type_ == Type::Internal || type_ == Type::Mapped) {
+		/*
+		 * \todo There might be multiple post-processors. The logic
+		 * which should be instantiated here, is deferred for future.
+		 * For now, we only have PostProcessJpeg and that is what we
+		 * will instantiate here.
+		 */
+		postProcessor_ = std::make_unique<PostProcessorJpeg>();
+	}
 
 	if (type == Type::Internal) {
 		allocator_ = std::make_unique<FrameBufferAllocator>(cameraDevice_->camera());
@@ -45,8 +50,8 @@ Stream *CameraStream::stream() const
 
 int CameraStream::configure()
 {
-	if (encoder_) {
-		int ret = encoder_->configure(configuration());
+	if (postProcessor_) {
+		int ret = postProcessor_->configure(configuration());
 		if (ret)
 			return ret;
 	}
@@ -69,60 +74,11 @@ int CameraStream::configure()
 int CameraStream::process(const libcamera::FrameBuffer &source,
 			  MappedCamera3Buffer *dest, CameraMetadata *metadata)
 {
-	if (!encoder_)
+	if (!postProcessor_)
 		return 0;
 
-	/* Set EXIF metadata for various tags. */
-	Exif exif;
-	/* \todo Set Make and Model from external vendor tags. */
-	exif.setMake("libcamera");
-	exif.setModel("cameraModel");
-	exif.setOrientation(cameraDevice_->orientation());
-	exif.setSize(configuration().size);
-	/*
-	 * We set the frame's EXIF timestamp as the time of encode.
-	 * Since the precision we need for EXIF timestamp is only one
-	 * second, it is good enough.
-	 */
-	exif.setTimestamp(std::time(nullptr));
-	if (exif.generate() != 0)
-		LOG(HAL, Error) << "Failed to generate valid EXIF data";
-
-	int jpeg_size = encoder_->encode(&source, dest->maps()[0], exif.data());
-	if (jpeg_size < 0) {
-		LOG(HAL, Error) << "Failed to encode stream image";
-		return jpeg_size;
-	}
-
-	/*
-	 * Fill in the JPEG blob header.
-	 *
-	 * The mapped size of the buffer is being returned as
-	 * substantially larger than the requested JPEG_MAX_SIZE
-	 * (which is referenced from maxJpegBufferSize_). Utilise
-	 * this static size to ensure the correct offset of the blob is
-	 * determined.
-	 *
-	 * \todo Investigate if the buffer size mismatch is an issue or
-	 * expected behaviour.
-	 */
-	uint8_t *resultPtr = dest->maps()[0].data() +
-			     cameraDevice_->maxJpegBufferSize() -
-			     sizeof(struct camera3_jpeg_blob);
-	auto *blob = reinterpret_cast<struct camera3_jpeg_blob *>(resultPtr);
-	blob->jpeg_blob_id = CAMERA3_JPEG_BLOB_ID;
-	blob->jpeg_size = jpeg_size;
-
-	/* Update the JPEG result Metadata. */
-	metadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1);
-
-	const uint32_t jpeg_quality = 95;
-	metadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 1);
-
-	const uint32_t jpeg_orientation = 0;
-	metadata->addEntry(ANDROID_JPEG_ORIENTATION, &jpeg_orientation, 1);
-
-	return 0;
+	return postProcessor_->process(&source, dest->maps()[0],
+				       metadata, cameraDevice_);
 }
 
 FrameBuffer *CameraStream::getBuffer()
diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h
index 8df0101..8d0f2e3 100644
--- a/src/android/camera_stream.h
+++ b/src/android/camera_stream.h
@@ -19,7 +19,12 @@
 #include <libcamera/geometry.h>
 #include <libcamera/pixel_format.h>
 
-class Encoder;
+/*
+ * \todo Ideally we want to replace this include with forward-declaration.
+ * If we do that. currently we get a compile error.
+ */
+#include "post_processor.h"
+
 class CameraDevice;
 class CameraMetadata;
 class MappedCamera3Buffer;
@@ -135,7 +140,7 @@ private:
 	 */
 	unsigned int index_;
 
-	std::unique_ptr<Encoder> encoder_;
+	std::unique_ptr<PostProcessor> postProcessor_;
 	std::unique_ptr<libcamera::FrameBufferAllocator> allocator_;
 	std::vector<libcamera::FrameBuffer *> buffers_;
 	/*
diff --git a/src/android/jpeg/encoder.h b/src/android/jpeg/encoder.h
deleted file mode 100644
index cf26d67..0000000
--- a/src/android/jpeg/encoder.h
+++ /dev/null
@@ -1,25 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-or-later */
-/*
- * Copyright (C) 2020, Google Inc.
- *
- * encoder.h - Image encoding interface
- */
-#ifndef __ANDROID_JPEG_ENCODER_H__
-#define __ANDROID_JPEG_ENCODER_H__
-
-#include <libcamera/buffer.h>
-#include <libcamera/span.h>
-#include <libcamera/stream.h>
-
-class Encoder
-{
-public:
-	virtual ~Encoder() {};
-
-	virtual int configure(const libcamera::StreamConfiguration &cfg) = 0;
-	virtual int encode(const libcamera::FrameBuffer *source,
-			   const libcamera::Span<uint8_t> &destination,
-			   const libcamera::Span<const uint8_t> &exifData) = 0;
-};
-
-#endif /* __ANDROID_JPEG_ENCODER_H__ */
diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp
similarity index 67%
rename from src/android/jpeg/encoder_libjpeg.cpp
rename to src/android/jpeg/post_processor_jpeg.cpp
index 510613c..eeb4e95 100644
--- a/src/android/jpeg/encoder_libjpeg.cpp
+++ b/src/android/jpeg/post_processor_jpeg.cpp
@@ -2,10 +2,14 @@
 /*
  * Copyright (C) 2020, Google Inc.
  *
- * encoder_libjpeg.cpp - JPEG encoding using libjpeg native API
+ * post_processor_jpeg.cpp - JPEG Post Processor
  */
 
-#include "encoder_libjpeg.h"
+#include "post_processor_jpeg.h"
+
+#include "exif.h"
+
+#include "../camera_device.h"
 
 #include <fcntl.h>
 #include <iomanip>
@@ -25,6 +29,14 @@
 
 using namespace libcamera;
 
+#define extract_va_arg(type, arg, last) \
+{                                       \
+        va_list ap;                     \
+        va_start(ap, last);             \
+        arg = va_arg(ap, type);         \
+        va_end(ap);                     \
+}
+
 LOG_DEFINE_CATEGORY(JPEG)
 
 namespace {
@@ -67,7 +79,7 @@ const struct JPEGPixelFormatInfo &findPixelInfo(const PixelFormat &format)
 
 } /* namespace */
 
-EncoderLibJpeg::EncoderLibJpeg()
+PostProcessorJpeg::PostProcessorJpeg()
 	: quality_(95)
 {
 	/* \todo Expand error handling coverage with a custom handler. */
@@ -76,12 +88,12 @@ EncoderLibJpeg::EncoderLibJpeg()
 	jpeg_create_compress(&compress_);
 }
 
-EncoderLibJpeg::~EncoderLibJpeg()
+PostProcessorJpeg::~PostProcessorJpeg()
 {
 	jpeg_destroy_compress(&compress_);
 }
 
-int EncoderLibJpeg::configure(const StreamConfiguration &cfg)
+int PostProcessorJpeg::configure(const StreamConfiguration &cfg)
 {
 	const struct JPEGPixelFormatInfo info = findPixelInfo(cfg.pixelFormat);
 	if (info.colorSpace == JCS_UNKNOWN)
@@ -104,7 +116,67 @@ int EncoderLibJpeg::configure(const StreamConfiguration &cfg)
 	return 0;
 }
 
-void EncoderLibJpeg::compressRGB(const libcamera::MappedBuffer *frame)
+int PostProcessorJpeg::process(const libcamera::FrameBuffer *source,
+			       const libcamera::Span<uint8_t> &destination,
+			       CameraMetadata *metadata, ...)
+{
+	CameraDevice *device = nullptr;
+	extract_va_arg(CameraDevice *, device, metadata);
+
+	/* Set EXIF metadata for various tags. */
+	Exif exif;
+	/* \todo Set Make and Model from external vendor tags. */
+	exif.setMake("libcamera");
+	exif.setModel("cameraModel");
+	exif.setOrientation(device->orientation());
+	exif.setSize(Size {compress_.image_width, compress_.image_height});
+	/*
+	 * We set the frame's EXIF timestamp as the time of encode.
+	 * Since the precision we need for EXIF timestamp is only one
+	 * second, it is good enough.
+	 */
+	exif.setTimestamp(std::time(nullptr));
+	if (exif.generate() != 0)
+		LOG(JPEG, Error) << "Failed to generate valid EXIF data";
+
+	int jpeg_size = encode(source, destination, exif.data());
+	if (jpeg_size < 0) {
+		LOG(JPEG, Error) << "Failed to encode stream image";
+		return jpeg_size;
+	}
+
+	/*
+	 * Fill in the JPEG blob header.
+	 *
+	 * The mapped size of the buffer is being returned as
+	 * substantially larger than the requested JPEG_MAX_SIZE
+	 * (which is referenced from maxJpegBufferSize_). Utilise
+	 * this static size to ensure the correct offset of the blob is
+	 * determined.
+	 *
+	 * \todo Investigate if the buffer size mismatch is an issue or
+	 * expected behaviour.
+	 */
+	uint8_t *resultPtr = destination.data() +
+			     device->maxJpegBufferSize() -
+			     sizeof(struct camera3_jpeg_blob);
+	auto *blob = reinterpret_cast<struct camera3_jpeg_blob *>(resultPtr);
+	blob->jpeg_blob_id = CAMERA3_JPEG_BLOB_ID;
+	blob->jpeg_size = jpeg_size;
+
+	/* Update the JPEG result Metadata. */
+	metadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1);
+
+	const uint32_t jpeg_quality = 95;
+	metadata->addEntry(ANDROID_JPEG_QUALITY, &jpeg_quality, 1);
+
+	const uint32_t jpeg_orientation = 0;
+	metadata->addEntry(ANDROID_JPEG_ORIENTATION, &jpeg_orientation, 1);
+
+	return 0;
+}
+
+void PostProcessorJpeg::compressRGB(const libcamera::MappedBuffer *frame)
 {
 	unsigned char *src = static_cast<unsigned char *>(frame->maps()[0].data());
 	/* \todo Stride information should come from buffer configuration. */
@@ -122,7 +194,7 @@ void EncoderLibJpeg::compressRGB(const libcamera::MappedBuffer *frame)
  * Compress the incoming buffer from a supported NV format.
  * This naively unpacks the semi-planar NV12 to a YUV888 format for libjpeg.
  */
-void EncoderLibJpeg::compressNV(const libcamera::MappedBuffer *frame)
+void PostProcessorJpeg::compressNV(const libcamera::MappedBuffer *frame)
 {
 	uint8_t tmprowbuf[compress_.image_width * 3];
 
@@ -179,9 +251,9 @@ void EncoderLibJpeg::compressNV(const libcamera::MappedBuffer *frame)
 	}
 }
 
-int EncoderLibJpeg::encode(const FrameBuffer *source,
-			   const libcamera::Span<uint8_t> &dest,
-			   const libcamera::Span<const uint8_t> &exifData)
+int PostProcessorJpeg::encode(const FrameBuffer *source,
+			      const libcamera::Span<uint8_t> &dest,
+			      const libcamera::Span<const uint8_t> &exifData)
 {
 	MappedFrameBuffer frame(source, PROT_READ);
 	if (!frame.isValid()) {
diff --git a/src/android/jpeg/encoder_libjpeg.h b/src/android/jpeg/post_processor_jpeg.h
similarity index 55%
rename from src/android/jpeg/encoder_libjpeg.h
rename to src/android/jpeg/post_processor_jpeg.h
index 1e8df05..7f9ce70 100644
--- a/src/android/jpeg/encoder_libjpeg.h
+++ b/src/android/jpeg/post_processor_jpeg.h
@@ -2,30 +2,37 @@
 /*
  * Copyright (C) 2020, Google Inc.
  *
- * encoder_libjpeg.h - JPEG encoding using libjpeg
+ * post_processor_jpeg.h - JPEG Post Processor
  */
-#ifndef __ANDROID_JPEG_ENCODER_LIBJPEG_H__
-#define __ANDROID_JPEG_ENCODER_LIBJPEG_H__
+#ifndef __ANDROID_POST_PROCESSOR_JPEG_H__
+#define __ANDROID_POST_PROCESSOR_JPEG_H__
 
-#include "encoder.h"
+#include "../post_processor.h"
+#include "../camera_metadata.h"
 
 #include "libcamera/internal/buffer.h"
 #include "libcamera/internal/formats.h"
 
 #include <jpeglib.h>
 
-class EncoderLibJpeg : public Encoder
+class PostProcessorJpeg : public PostProcessor
 {
 public:
-	EncoderLibJpeg();
-	~EncoderLibJpeg();
+	PostProcessorJpeg();
+	~PostProcessorJpeg();
 
 	int configure(const libcamera::StreamConfiguration &cfg) override;
+	int process(const libcamera::FrameBuffer *source,
+		    const libcamera::Span<uint8_t> &destination,
+		    CameraMetadata *metadata,
+		    ...) override;
+
+
+private:
 	int encode(const libcamera::FrameBuffer *source,
 		   const libcamera::Span<uint8_t> &destination,
-		   const libcamera::Span<const uint8_t> &exifData) override;
+		   const libcamera::Span<const uint8_t> &exifData);
 
-private:
 	void compressRGB(const libcamera::MappedBuffer *frame);
 	void compressNV(const libcamera::MappedBuffer *frame);
 
@@ -40,4 +47,4 @@ private:
 	bool nvSwap_;
 };
 
-#endif /* __ANDROID_JPEG_ENCODER_LIBJPEG_H__ */
+#endif /* __ANDROID_POST_PROCESSOR_JPEG_H__ */
diff --git a/src/android/meson.build b/src/android/meson.build
index 802bb89..02b3b47 100644
--- a/src/android/meson.build
+++ b/src/android/meson.build
@@ -21,8 +21,8 @@ android_hal_sources = files([
     'camera_metadata.cpp',
     'camera_ops.cpp',
     'camera_stream.cpp',
-    'jpeg/encoder_libjpeg.cpp',
     'jpeg/exif.cpp',
+    'jpeg/post_processor_jpeg.cpp',
 ])
 
 android_camera_metadata_sources = files([
-- 
2.26.2



More information about the libcamera-devel mailing list