[libcamera-devel,v7,4/8] libcamera: ipu3: Use roles in stream configuration

1077 diff mbox series

Use and inspect the stream roles provided by the application to
streamConfiguration() to assign streams to their intended roles and
return a default configuration associated with them.

Support a limited number of usages, with the viewfinder stream able to
capture both continuous video streams and still images, and the main
output stream supporting still images images. This is an artificial
limitation until we figure out the exact capabilities of the hardware.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/pipeline/ipu3/ipu3.cpp | 113 ++++++++++++++++++++-------
 1 file changed, 83 insertions(+), 30 deletions(-)

Hi Jacopo,

Thank you for the patch.

On Fri, Apr 19, 2019 at 12:18:35PM +0200, Jacopo Mondi wrote:
> Use and inspect the stream roles provided by the application to
> streamConfiguration() to assign streams to their intended roles and
> return a default configuration associated with them.
> 
> Support a limited number of usages, with the viewfinder stream able to
> capture both continuous video streams and still images, and the main
> output stream supporting still images images. This is an artificial

s/images images/images/

> limitation until we figure out the exact capabilities of the hardware.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 113 ++++++++++++++++++++-------
>  1 file changed, 83 insertions(+), 30 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 9b2d96f22b2b..45edc24b5d4c 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -5,6 +5,7 @@
>   * ipu3.cpp - Pipeline handler for Intel IPU3
>   */
>  
> +#include <algorithm>
>  #include <iomanip>
>  #include <memory>
>  #include <vector>
> @@ -222,38 +223,90 @@ CameraConfiguration
>  PipelineHandlerIPU3::streamConfiguration(Camera *camera,
>  					 const std::vector<StreamUsage> &usages)
>  {
> -	CameraConfiguration configs;
>  	IPU3CameraData *data = cameraData(camera);
> -	StreamConfiguration config = {};
> +	CameraConfiguration cameraConfig;
> +	std::set<IPU3Stream *> streams = {
> +		&data->outStream_,
> +		&data->vfStream_,
> +	};
>  
> -	/*
> -	 * FIXME: Soraka: the maximum resolution reported by both sensors
> -	 * (2592x1944 for ov5670 and 4224x3136 for ov13858) are returned as
> -	 * default configurations but they're not correctly processed by the
> -	 * ImgU. Resolutions up tp 2560x1920 have been validated.
> -	 *
> -	 * \todo Clarify ImgU alignement requirements.
> -	 */
> -	config.width = 2560;
> -	config.height = 1920;
> -	config.pixelFormat = V4L2_PIX_FMT_NV12;
> -	config.bufferCount = IPU3_BUFFER_COUNT;
> -
> -	configs[&data->outStream_] = config;
> -	LOG(IPU3, Debug)
> -		<< "Stream '" << data->outStream_.name_ << "' set to "
> -		<< config.width << "x" << config.height << "-0x"
> -		<< std::hex << std::setfill('0') << std::setw(8)
> -		<< config.pixelFormat;
> -
> -	configs[&data->vfStream_] = config;
> -	LOG(IPU3, Debug)
> -		<< "Stream '" << data->vfStream_.name_ << "' set to "
> -		<< config.width << "x" << config.height << "-0x"
> -		<< std::hex << std::setfill('0') << std::setw(8)
> -		<< config.pixelFormat;
> -
> -	return configs;
> +	for (const StreamUsage &usage : usages) {
> +		std::vector<IPU3Stream *>::iterator found;

This variable isn't used anymore. No warning from the compiler ?

> +		StreamUsage::Role role = usage.role();
> +		StreamConfiguration streamConfig = {};
> +		IPU3Stream *stream = nullptr;
> +
> +		if (role == StreamUsage::Role::StillCapture) {
> +			/*
> +			 * Don't allow viewfinder or video capture on the
> +			 * 'output stream. This is an artificial limitation
> +			 * until we figure out the capabilities of the
> +			 * hardware.
> +			 */
> +			if (streams.find(&data->outStream_) != streams.end())
> +				stream = &data->outStream_;
> +			else if (streams.find(&data->vfStream_) != streams.end())
> +				stream = &data->vfStream_;
> +			else
> +				goto error;
> +
> +			/*
> +			 * FIXME: Soraka: the maximum resolution reported by
> +			 * both sensors (2592x1944 for ov5670 and 4224x3136 for
> +			 * ov13858) are returned as default configurations but
> +			 * they're not correctly processed by the ImgU.
> +			 * Resolutions up tp 2560x1920 have been validated.
> +			 *
> +			 * \todo Clarify ImgU alignment requirements.
> +			 */
> +			streamConfig.width = 2560;
> +			streamConfig.height = 1920;
> +		} else if (role == StreamUsage::Role::Viewfinder ||
> +			   role == StreamUsage::Role::VideoRecording) {
> +			/*
> +			 * We can't use the 'output' stream for viewfinder or
> +			 * video capture usages.
> +			 */

I'd duplicate here the information from the commit message explaining
that the limitation is artificial, maybe with a \todo

> +			if (streams.find(&data->vfStream_) == streams.end())
> +				goto error;
> +
> +			stream = &data->vfStream_;
> +
> +

Extra blank line.

> +			/*
> +			 * Align the requested viewfinder size to the
> +			 * maximum available sensor resolution and to the
> +			 * IPU3 alignment constraints.
> +			 */
> +			const Size &res = data->cio2_.sensor_->resolution();
> +			unsigned int width = std::min(usage.size().width,
> +						      res.width);
> +			unsigned int height = std::min(usage.size().height,
> +						       res.height);

Should we set a minimum size too ?

We need to figure out a policy for the streamsConfiguration() operation.
It doesn't have to be solved as part of this series, but we need to
document it.

> +			streamConfig.width = width & ~7;
> +			streamConfig.height = height & ~3;
> +		}
> +
> +		streams.erase(stream);
> +
> +		streamConfig.pixelFormat = V4L2_PIX_FMT_NV12;
> +		streamConfig.bufferCount = IPU3_BUFFER_COUNT;
> +
> +		cameraConfig[stream] = streamConfig;
> +
> +		LOG(IPU3, Debug)
> +			<< "Stream '" << stream->name_ << "' format set to "
> +			<< streamConfig.width << "x" << streamConfig.height
> +			<< "-0x" << std::hex << std::setfill('0')
> +			<< std::setw(8) << streamConfig.pixelFormat;
> +	}
> +
> +	return cameraConfig;
> +
> +error:
> +	LOG(IPU3, Error) << "Requested stream roles not supported";
> +
> +	return CameraConfiguration{};
>  }
>  
>  int PipelineHandlerIPU3::configureStreams(Camera *camera,