[libcamera-devel] [PATCH 02/15] android: camera_stream: Add CameraStream::Type

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Mon, Oct 05, 2020 at 01:46:36PM +0300, Laurent Pinchart wrote:
> From: Jacopo Mondi <jacopo at jmondi.org>
> 
> Define the CameraStream::Type enumeration and assign it to
> each CameraStream instance at construction time.
> 
> The CameraStream type will be used to decide if memory needs to be
> allocated on its behalf or if the stream is backed by memory externally
> allocated by the Android framework.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Reviewed-by: Hirokazu Honda <hiroh at chromium.org>
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/android/camera_device.cpp |  8 +++-
>  src/android/camera_stream.cpp |  4 +-
>  src/android/camera_stream.h   | 86 ++++++++++++++++++++++++++++++++++-
>  3 files changed, 93 insertions(+), 5 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index bbc692fe109f..0600ebc81c64 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1216,12 +1216,14 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>  
>  		config_->addConfiguration(streamConfiguration);
>  		unsigned int index = config_->size() - 1;
> -		streams_.emplace_back(format, size, index);
> +		streams_.emplace_back(format, size, CameraStream::Type::Direct,
> +				      index);
>  		stream->priv = static_cast<void *>(&streams_.back());
>  	}
>  
>  	/* Now handle the MJPEG streams, adding a new stream if required. */
>  	if (jpegStream) {
> +		CameraStream::Type type;
>  		int index = -1;
>  
>  		/* Search for a compatible stream in the non-JPEG ones. */
> @@ -1239,6 +1241,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>  			LOG(HAL, Info)
>  				<< "Android JPEG stream mapped to libcamera stream " << i;
>  
> +			type = CameraStream::Type::Mapped;
>  			index = i;
>  			break;
>  		}
> @@ -1263,6 +1266,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>  			LOG(HAL, Info) << "Adding " << streamConfiguration.toString()
>  				       << " for MJPEG support";
>  
> +			type = CameraStream::Type::Internal;
>  			config_->addConfiguration(streamConfiguration);
>  			index = config_->size() - 1;
>  		}
> @@ -1281,7 +1285,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
>  			return ret;
>  		}
>  
> -		streams_.emplace_back(formats::MJPEG, cfg.size, index, encoder);
> +		streams_.emplace_back(formats::MJPEG, cfg.size, type, index, encoder);
>  		jpegStream->priv = static_cast<void *>(&streams_.back());
>  	}
>  
> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> index 01c62978ca3a..585bf2b68f4f 100644
> --- a/src/android/camera_stream.cpp
> +++ b/src/android/camera_stream.cpp
> @@ -10,7 +10,7 @@
>  using namespace libcamera;
>  
>  CameraStream::CameraStream(PixelFormat format, Size size,
> -			   unsigned int index, Encoder *encoder)
> -	: format_(format), size_(size), index_(index), encoder_(encoder)
> +			   Type type, unsigned int index, Encoder *encoder)
> +	: format_(format), size_(size), type_(type), index_(index), encoder_(encoder)
>  {
>  }
> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h
> index 10dece7beb69..07c714e3a365 100644
> --- a/src/android/camera_stream.h
> +++ b/src/android/camera_stream.h
> @@ -17,17 +17,101 @@
>  class CameraStream
>  {
>  public:
> +	/*
> +	 * Enumeration of CameraStream types.
> +	 *
> +	 * A camera stream associates an Android stream to a libcamera stream.
> +	 * This enumeration describes how the two streams are associated and how
> +	 * and where data produced from libcamera are delivered to the
> +	 * Android framework.
> +	 *
> +	 * Direct:
> +	 *
> +	 * The Android stream is directly mapped onto a libcamera stream: frames
> +	 * are delivered by the library directly in the memory location
> +	 * specified by the Android stream (buffer_handle_t->data) and provided
> +	 * to the framework as they are. The Android stream characteristics are
> +	 * directly translated to the libcamera stream configuration.
> +	 *
> +	 * +-----+                +-----+
> +	 * |  A  |                |  L  |
> +	 * +-----+                +-----+
> +	 *    |                      |
> +	 *    V                      V
> +	 * +-----+                +------+
> +	 * |  B  |<---------------|  FB  |
> +	 * +-----+                +------+
> +	 *
> +	 *
> +	 * Internal:
> +	 *
> +	 * Data for the Android stream is produced by processing a libcamera
> +	 * stream created by the HAL for that purpose. The libcamera stream
> +	 * needs to be supplied with intermediate buffers where the library
> +	 * delivers frames to be processed and then provided to the framework.
> +	 * The libcamera stream configuration is not a direct translation of the
> +	 * Android stream characteristics, but it describes the format and size
> +	 * required for the processing procedure to produce frames in the
> +	 * Android required format.
> +	 *
> +	 * +-----+                +-----+
> +	 * |  A  |                |  L  |
> +	 * +-----+                +-----+
> +	 *    |                      |
> +	 *    V                      V
> +	 * +-----+                +------+
> +	 * |  B  |                |  FB  |
> +	 * +-----+                +------+
> +	 *   ^                       |
> +	 *   |-------Processing------|
> +	 *
> +	 *
> +	 * Mapped:
> +	 *
> +	 * Data for the Android stream is produced by processing a libcamera
> +	 * stream associated with another CameraStream. Mapped camera streams do
> +	 * not need any memory to be reserved for them as they process data
> +	 * produced by libcamera for a different stream whose format and size
> +	 * is compatible with the processing procedure requirements to produce

s/is/are/

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> +	 * frames in the Android required format.
> +	 *
> +	 * +-----+      +-----+          +-----+
> +	 * |  A  |      |  A' |          |  L  |
> +	 * +-----+      +-----+          +-----+
> +	 *    |            |                |
> +	 *    V            V                V
> +	 * +-----+      +-----+          +------+
> +	 * |  B  |      |  B' |<---------|  FB  |
> +	 * +-----+      +-----+          +------+
> +	 *   ^              |
> +	 *   |--Processing--|
> +	 *
> +	 *
> +	 * --------------------------------------------------------------------
> +	 * A  = Android stream
> +	 * L  = libcamera stream
> +	 * B  = memory buffer
> +	 * FB = libcamera FrameBuffer
> +	 * "Processing" = Frame processing procedure (Encoding, scaling etc)
> +	 */
> +	enum class Type {
> +		Direct,
> +		Internal,
> +		Mapped,
> +	};
>  	CameraStream(libcamera::PixelFormat format, libcamera::Size size,
> -		     unsigned int index, Encoder *encoder = nullptr);
> +		     Type type, unsigned int index, Encoder *encoder = nullptr);
>  
>  	const libcamera::PixelFormat &format() const { return format_; }
>  	const libcamera::Size &size() const { return size_; }
> +	Type type() const { return type_; }
>  	unsigned int index() const { return index_; }
>  	Encoder *encoder() const { return encoder_.get(); }
>  
>  private:
>  	libcamera::PixelFormat format_;
>  	libcamera::Size size_;
> +	Type type_;
>  	/*
>  	 * The index of the libcamera StreamConfiguration as added during
>  	 * configureStreams(). A single libcamera Stream may be used to deliver

-- 
Regards,

Laurent Pinchart