[libcamera-devel,v4,09/10] android: camera_device: Set Encoder at construction

Message ID 20200912101129.12625-10-jacopo@jmondi.org
State Accepted
Headers show
Series
  • android: camera_device: Fix JPEG/RAW sizes
Related show

Commit Message

Jacopo Mondi Sept. 12, 2020, 10:11 a.m. UTC
Make the CameraStream encoder a private unique pointer and require its
initialization at construction time. This ties the encoder lifetime to
the CameraStream it has been created with, allowing to remove the
CameraStream destructor.

This change dis-allow creating a CameraStream and set the Encoder later,
which shall not happen now that we create CameraStream once we have all
the required information in place.

No functional changes intended but this change aims to make the code
more robust enforcing a stricter CameraStream interface.

Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/android/camera_device.cpp | 22 +++++++++-------------
 src/android/camera_device.h   |  8 ++++----
 2 files changed, 13 insertions(+), 17 deletions(-)

Patch

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index 59acfd762a89..4d8847e72db5 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -169,16 +169,11 @@  MappedCamera3Buffer::MappedCamera3Buffer(const buffer_handle_t camera3buffer,
 	}
 }
 
-CameraStream::CameraStream(PixelFormat f, Size s, unsigned int i)
-	: format(f), size(s), jpeg(nullptr), index_(i)
+CameraStream::CameraStream(PixelFormat f, Size s, unsigned int i, Encoder *e)
+	: format(f), size(s), index_(i), encoder_(e)
 {
 }
 
-CameraStream::~CameraStream()
-{
-	delete jpeg;
-};
-
 /*
  * \struct Camera3RequestDescriptor
  *
@@ -1272,20 +1267,21 @@  int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
 		}
 
 		StreamConfiguration &cfg = config_->at(index);
-		CameraStream &cameraStream =
-			streams_.emplace_back(formats::MJPEG, cfg.size, index);
-		jpegStream->priv = static_cast<void *>(&cameraStream);
 
 		/*
 		 * Construct a software encoder for MJPEG streams from the
 		 * chosen libcamera source stream.
 		 */
-		cameraStream.jpeg = new EncoderLibJpeg();
-		int ret = cameraStream.jpeg->configure(cfg);
+		Encoder *encoder = new EncoderLibJpeg();
+		int ret = encoder->configure(cfg);
 		if (ret) {
 			LOG(HAL, Error) << "Failed to configure encoder";
+			delete encoder;
 			return ret;
 		}
+
+		streams_.emplace_back(formats::MJPEG, cfg.size, index, encoder);
+		jpegStream->priv = static_cast<void *>(&streams_.back());
 	}
 
 	switch (config_->validate()) {
@@ -1474,7 +1470,7 @@  void CameraDevice::requestComplete(Request *request)
 		if (cameraStream->format != formats::MJPEG)
 			continue;
 
-		Encoder *encoder = cameraStream->jpeg;
+		Encoder *encoder = cameraStream->encoder();
 		if (!encoder) {
 			LOG(HAL, Error) << "Failed to identify encoder";
 			continue;
diff --git a/src/android/camera_device.h b/src/android/camera_device.h
index 376d001ea7d7..49a2e7f3f8bc 100644
--- a/src/android/camera_device.h
+++ b/src/android/camera_device.h
@@ -29,16 +29,15 @@  class CameraMetadata;
 
 struct CameraStream {
 public:
-	CameraStream(libcamera::PixelFormat, libcamera::Size, unsigned int i);
-	~CameraStream();
+	CameraStream(libcamera::PixelFormat, libcamera::Size, unsigned int i,
+		     Encoder *e = nullptr);
 
 	unsigned int index() const { return index_; }
+	Encoder *encoder() const { return encoder_.get(); }
 
 	libcamera::PixelFormat format;
 	libcamera::Size size;
 
-	Encoder *jpeg;
-
 private:
 	/*
 	 * The index of the libcamera StreamConfiguration as added during
@@ -46,6 +45,7 @@  private:
 	 * one or more streams to the Android framework.
 	 */
 	unsigned int index_;
+	std::unique_ptr<Encoder> encoder_;
 };
 
 class CameraDevice : protected libcamera::Loggable