[libcamera-devel] [PATCH 03/15] android: camera_stream: Delegate Encoder construction

[Umang Jain]

Hi Jacopo,

On 10/5/20 4:16 PM, Laurent Pinchart wrote:
> From: Jacopo Mondi <jacopo at jmondi.org>
>
> Delegate the construction of the encoder to the CameraStream class
> for streams that need post-processing.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>   src/android/camera_device.cpp | 23 ++++++++++-------------
>   src/android/camera_stream.cpp | 17 ++++++++++++++---
>   src/android/camera_stream.h   |  4 +++-
>   3 files changed, 27 insertions(+), 17 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 0600ebc81c64..9c9a5cfa3c2f 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1273,19 +1273,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>   
>   		StreamConfiguration &cfg = config_->at(index);
>   
> -		/*
> -		 * Construct a software encoder for the MJPEG streams from the
> -		 * chosen libcamera source stream.
> -		 */
> -		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, type, index, encoder);
> +		streams_.emplace_back(formats::MJPEG, cfg.size, type, index);
>   		jpegStream->priv = static_cast<void *>(&streams_.back());
>   	}
>   
> @@ -1306,11 +1294,20 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>   		return -EINVAL;
>   	}
>   
> +	/*
> +	 * Configure the HAL CameraStream instances using the associated
> +	 * StreamConfiguration and set the number of required buffers in
> +	 * the Android camera3_stream_t.
> +	 */
>   	for (unsigned int i = 0; i < stream_list->num_streams; ++i) {
>   		camera3_stream_t *stream = stream_list->streams[i];
>   		CameraStream *cameraStream = static_cast<CameraStream *>(stream->priv);
>   		StreamConfiguration &cfg = config_->at(cameraStream->index());
>   
> +		int ret = cameraStream->configure(cfg);
> +		if (ret)
> +			return ret;
> +
>   		/* Use the bufferCount confirmed by the validation process. */
>   		stream->max_buffers = cfg.bufferCount;
>   	}
> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> index 585bf2b68f4f..5b2b625c563b 100644
> --- a/src/android/camera_stream.cpp
> +++ b/src/android/camera_stream.cpp
> @@ -7,10 +7,21 @@
>   
>   #include "camera_stream.h"
>   
> +#include "jpeg/encoder_libjpeg.h"
> +
>   using namespace libcamera;
>   
> -CameraStream::CameraStream(PixelFormat format, Size size,
> -			   Type type, unsigned int index, Encoder *encoder)
> -	: format_(format), size_(size), type_(type), index_(index), encoder_(encoder)
> +CameraStream::CameraStream(PixelFormat format, Size size, Type type, unsigned int index)
> +	: format_(format), size_(size), type_(type), index_(index), encoder(nullptr)
I think this should be s/encoder(nullptr)/encoder_(nullptr)/

Reviewed-by: Umang Jain <email at uajain.com>
>   {
> +	if (type_ == Type::Internal || type_ == Type::Mapped)
> +		encoder_.reset(new EncoderLibJpeg);
> +}
> +
> +int CameraStream::configure(const libcamera::StreamConfiguration &cfg)
> +{
> +	if (encoder_)
> +		return encoder_->configure(cfg);
> +
> +	return 0;
>   }
> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h
> index 07c714e3a365..d0dc40d81151 100644
> --- a/src/android/camera_stream.h
> +++ b/src/android/camera_stream.h
> @@ -100,7 +100,7 @@ public:
>   		Mapped,
>   	};
>   	CameraStream(libcamera::PixelFormat format, libcamera::Size size,
> -		     Type type, unsigned int index, Encoder *encoder = nullptr);
> +		     Type type, unsigned int index);
>   
>   	const libcamera::PixelFormat &format() const { return format_; }
>   	const libcamera::Size &size() const { return size_; }
> @@ -108,6 +108,8 @@ public:
>   	unsigned int index() const { return index_; }
>   	Encoder *encoder() const { return encoder_.get(); }
>   
> +	int configure(const libcamera::StreamConfiguration &cfg);
> +
>   private:
>   	libcamera::PixelFormat format_;
>   	libcamera::Size size_;