[libcamera-devel] [RFC PATCH 2/5] android: camera_stream: Pass FrameBuffer pointer instead of reference

Umang Jain umang.jain at ideasonboard.com
Thu Aug 26 23:30:13 CEST 2021


Pass the libcamera::FrameBuffer pointer to the post-processor instead
of passing it by reference. Passing by reference is fine as long as
the post processing is done synchronously.

However in subsequent commits, the post processing is planned to be
moved to a separate thread. This will conflict as the reference
argument (in current case 'source') is copied when we will try to
invoke a method on the post-processor using Object::invokeMethod().
The cause of conflict is the LIBCAMERA_DISABLE_COPY_AND_MOVE rule
applied on the FrameBuffer class.

To resolve the conflict, pass in the FrameBuffer pointer instead of
reference. This requires changes to the existing PostProcessor interface
and all its implemented classes.

Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
---
 src/android/camera_device.cpp            |  2 +-
 src/android/camera_stream.cpp            |  2 +-
 src/android/camera_stream.h              |  2 +-
 src/android/jpeg/encoder.h               |  2 +-
 src/android/jpeg/encoder_libjpeg.cpp     |  4 ++--
 src/android/jpeg/encoder_libjpeg.h       |  2 +-
 src/android/jpeg/post_processor_jpeg.cpp |  4 ++--
 src/android/jpeg/post_processor_jpeg.h   |  4 ++--
 src/android/jpeg/thumbnailer.cpp         |  4 ++--
 src/android/jpeg/thumbnailer.h           |  2 +-
 src/android/post_processor.h             |  2 +-
 src/android/yuv/post_processor_yuv.cpp   | 18 +++++++++---------
 src/android/yuv/post_processor_yuv.h     |  4 ++--
 13 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index a69b687a..fea59ce6 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -1149,7 +1149,7 @@ void CameraDevice::requestComplete(Request *request)
 			continue;
 		}
 
-		int ret = cameraStream->process(*src, *buffer.buffer,
+		int ret = cameraStream->process(src, *buffer.buffer,
 						descriptor.settings_,
 						resultMetadata.get());
 		/*
diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
index 61b44183..59b1df6e 100644
--- a/src/android/camera_stream.cpp
+++ b/src/android/camera_stream.cpp
@@ -98,7 +98,7 @@ int CameraStream::configure()
 	return 0;
 }
 
-int CameraStream::process(const libcamera::FrameBuffer &source,
+int CameraStream::process(const libcamera::FrameBuffer *source,
 			  buffer_handle_t camera3Dest,
 			  const CameraMetadata &requestMetadata,
 			  CameraMetadata *resultMetadata)
diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h
index 2dab6c3a..5c232cb6 100644
--- a/src/android/camera_stream.h
+++ b/src/android/camera_stream.h
@@ -118,7 +118,7 @@ public:
 	libcamera::Stream *stream() const;
 
 	int configure();
-	int process(const libcamera::FrameBuffer &source,
+	int process(const libcamera::FrameBuffer *source,
 		    buffer_handle_t camera3Dest,
 		    const CameraMetadata &requestMetadata,
 		    CameraMetadata *resultMetadata);
diff --git a/src/android/jpeg/encoder.h b/src/android/jpeg/encoder.h
index a28522f4..7b6140e7 100644
--- a/src/android/jpeg/encoder.h
+++ b/src/android/jpeg/encoder.h
@@ -18,7 +18,7 @@ public:
 	virtual ~Encoder() = default;
 
 	virtual int configure(const libcamera::StreamConfiguration &cfg) = 0;
-	virtual int encode(const libcamera::FrameBuffer &source,
+	virtual int encode(const libcamera::FrameBuffer *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..1c5ec592 100644
--- a/src/android/jpeg/encoder_libjpeg.cpp
+++ b/src/android/jpeg/encoder_libjpeg.cpp
@@ -178,10 +178,10 @@ void EncoderLibJpeg::compressNV(Span<const uint8_t> frame)
 	}
 }
 
-int EncoderLibJpeg::encode(const FrameBuffer &source, Span<uint8_t> dest,
+int EncoderLibJpeg::encode(const FrameBuffer *source, Span<uint8_t> dest,
 			   Span<const uint8_t> exifData, unsigned int quality)
 {
-	MappedFrameBuffer frame(&source, MappedFrameBuffer::MapFlag::Read);
+	MappedFrameBuffer frame(source, MappedFrameBuffer::MapFlag::Read);
 	if (!frame.isValid()) {
 		LOG(JPEG, Error) << "Failed to map FrameBuffer : "
 				 << strerror(frame.error());
diff --git a/src/android/jpeg/encoder_libjpeg.h b/src/android/jpeg/encoder_libjpeg.h
index 61fbd1a6..92e57b75 100644
--- a/src/android/jpeg/encoder_libjpeg.h
+++ b/src/android/jpeg/encoder_libjpeg.h
@@ -20,7 +20,7 @@ public:
 	~EncoderLibJpeg();
 
 	int configure(const libcamera::StreamConfiguration &cfg) override;
-	int encode(const libcamera::FrameBuffer &source,
+	int encode(const libcamera::FrameBuffer *source,
 		   libcamera::Span<uint8_t> destination,
 		   libcamera::Span<const uint8_t> exifData,
 		   unsigned int quality) override;
diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp
index 3160a784..723e8d18 100644
--- a/src/android/jpeg/post_processor_jpeg.cpp
+++ b/src/android/jpeg/post_processor_jpeg.cpp
@@ -50,7 +50,7 @@ int PostProcessorJpeg::configure(const StreamConfiguration &inCfg,
 	return encoder_->configure(inCfg);
 }
 
-void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source,
+void PostProcessorJpeg::generateThumbnail(const FrameBuffer *source,
 					  const Size &targetSize,
 					  unsigned int quality,
 					  std::vector<unsigned char> *thumbnail)
@@ -82,7 +82,7 @@ void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source,
 	}
 }
 
-int PostProcessorJpeg::process(const FrameBuffer &source,
+int PostProcessorJpeg::process(const FrameBuffer *source,
 			       CameraBuffer *destination,
 			       const CameraMetadata &requestMetadata,
 			       CameraMetadata *resultMetadata)
diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h
index 6fd31022..c4b2e9ef 100644
--- a/src/android/jpeg/post_processor_jpeg.h
+++ b/src/android/jpeg/post_processor_jpeg.h
@@ -22,13 +22,13 @@ public:
 
 	int configure(const libcamera::StreamConfiguration &incfg,
 		      const libcamera::StreamConfiguration &outcfg) override;
-	int process(const libcamera::FrameBuffer &source,
+	int process(const libcamera::FrameBuffer *source,
 		    CameraBuffer *destination,
 		    const CameraMetadata &requestMetadata,
 		    CameraMetadata *resultMetadata) override;
 
 private:
-	void generateThumbnail(const libcamera::FrameBuffer &source,
+	void generateThumbnail(const libcamera::FrameBuffer *source,
 			       const libcamera::Size &targetSize,
 			       unsigned int quality,
 			       std::vector<unsigned char> *thumbnail);
diff --git a/src/android/jpeg/thumbnailer.cpp b/src/android/jpeg/thumbnailer.cpp
index 79d83926..46575e4e 100644
--- a/src/android/jpeg/thumbnailer.cpp
+++ b/src/android/jpeg/thumbnailer.cpp
@@ -37,11 +37,11 @@ void Thumbnailer::configure(const Size &sourceSize, PixelFormat pixelFormat)
 	valid_ = true;
 }
 
-void Thumbnailer::createThumbnail(const FrameBuffer &source,
+void Thumbnailer::createThumbnail(const FrameBuffer *source,
 				  const Size &targetSize,
 				  std::vector<unsigned char> *destination)
 {
-	MappedFrameBuffer frame(&source, MappedFrameBuffer::MapFlag::Read);
+	MappedFrameBuffer frame(source, MappedFrameBuffer::MapFlag::Read);
 	if (!frame.isValid()) {
 		LOG(Thumbnailer, Error)
 			<< "Failed to map FrameBuffer : "
diff --git a/src/android/jpeg/thumbnailer.h b/src/android/jpeg/thumbnailer.h
index 4d086c49..0f3caf40 100644
--- a/src/android/jpeg/thumbnailer.h
+++ b/src/android/jpeg/thumbnailer.h
@@ -19,7 +19,7 @@ public:
 
 	void configure(const libcamera::Size &sourceSize,
 		       libcamera::PixelFormat pixelFormat);
-	void createThumbnail(const libcamera::FrameBuffer &source,
+	void createThumbnail(const libcamera::FrameBuffer *source,
 			     const libcamera::Size &targetSize,
 			     std::vector<unsigned char> *dest);
 	const libcamera::PixelFormat &pixelFormat() const { return pixelFormat_; }
diff --git a/src/android/post_processor.h b/src/android/post_processor.h
index b2b5bd55..5b8f1ab8 100644
--- a/src/android/post_processor.h
+++ b/src/android/post_processor.h
@@ -23,7 +23,7 @@ public:
 
 	virtual int configure(const libcamera::StreamConfiguration &inCfg,
 			      const libcamera::StreamConfiguration &outCfg) = 0;
-	virtual int process(const libcamera::FrameBuffer &source,
+	virtual int process(const libcamera::FrameBuffer *source,
 			    CameraBuffer *destination,
 			    const CameraMetadata &requestMetadata,
 			    CameraMetadata *resultMetadata) = 0;
diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp
index 3e793a58..4b507d33 100644
--- a/src/android/yuv/post_processor_yuv.cpp
+++ b/src/android/yuv/post_processor_yuv.cpp
@@ -49,7 +49,7 @@ int PostProcessorYuv::configure(const StreamConfiguration &inCfg,
 	return 0;
 }
 
-int PostProcessorYuv::process(const FrameBuffer &source,
+int PostProcessorYuv::process(const FrameBuffer *source,
 			      CameraBuffer *destination,
 			      [[maybe_unused]] const CameraMetadata &requestMetadata,
 			      [[maybe_unused]] CameraMetadata *metadata)
@@ -57,7 +57,7 @@ int PostProcessorYuv::process(const FrameBuffer &source,
 	if (!isValidBuffers(source, *destination))
 		return -EINVAL;
 
-	const MappedFrameBuffer sourceMapped(&source, MappedFrameBuffer::MapFlag::Read);
+	const MappedFrameBuffer sourceMapped(source, MappedFrameBuffer::MapFlag::Read);
 	if (!sourceMapped.isValid()) {
 		LOG(YUV, Error) << "Failed to mmap camera frame buffer";
 		return -EINVAL;
@@ -83,12 +83,12 @@ int PostProcessorYuv::process(const FrameBuffer &source,
 	return 0;
 }
 
-bool PostProcessorYuv::isValidBuffers(const FrameBuffer &source,
+bool PostProcessorYuv::isValidBuffers(const FrameBuffer *source,
 				      const CameraBuffer &destination) const
 {
-	if (source.planes().size() != 2) {
+	if (source->planes().size() != 2) {
 		LOG(YUV, Error) << "Invalid number of source planes: "
-				<< source.planes().size();
+				<< source->planes().size();
 		return false;
 	}
 	if (destination.numPlanes() != 2) {
@@ -97,12 +97,12 @@ bool PostProcessorYuv::isValidBuffers(const FrameBuffer &source,
 		return false;
 	}
 
-	if (source.planes()[0].length < sourceLength_[0] ||
-	    source.planes()[1].length < sourceLength_[1]) {
+	if (source->planes()[0].length < sourceLength_[0] ||
+	    source->planes()[1].length < sourceLength_[1]) {
 		LOG(YUV, Error)
 			<< "The source planes lengths are too small, actual size: {"
-			<< source.planes()[0].length << ", "
-			<< source.planes()[1].length
+			<< source->planes()[0].length << ", "
+			<< source->planes()[1].length
 			<< "}, expected size: {"
 			<< sourceLength_[0] << ", "
 			<< sourceLength_[1] << "}";
diff --git a/src/android/yuv/post_processor_yuv.h b/src/android/yuv/post_processor_yuv.h
index f8b1ba23..44a04113 100644
--- a/src/android/yuv/post_processor_yuv.h
+++ b/src/android/yuv/post_processor_yuv.h
@@ -20,13 +20,13 @@ public:
 
 	int configure(const libcamera::StreamConfiguration &incfg,
 		      const libcamera::StreamConfiguration &outcfg) override;
-	int process(const libcamera::FrameBuffer &source,
+	int process(const libcamera::FrameBuffer *source,
 		    CameraBuffer *destination,
 		    const CameraMetadata &requestMetadata,
 		    CameraMetadata *metadata) override;
 
 private:
-	bool isValidBuffers(const libcamera::FrameBuffer &source,
+	bool isValidBuffers(const libcamera::FrameBuffer *source,
 			    const CameraBuffer &destination) const;
 	void calculateLengths(const libcamera::StreamConfiguration &inCfg,
 			      const libcamera::StreamConfiguration &outCfg);
-- 
2.31.1



More information about the libcamera-devel mailing list