[libcamera-devel] [PATCH v4 11/12] libcamera: ipu3: Use roles in stream configuration

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2019-04-09 21:25:47 +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, as the viewfinder stream can
> optionally be used for capturing still images, but the main output
> stream cannot be used as viewfinder or for video recording purposes.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 105 ++++++++++++++++++++-------
>  1 file changed, 79 insertions(+), 26 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 75ffdc56d157..70a92783076f 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -227,36 +227,89 @@ CameraConfiguration
>  PipelineHandlerIPU3::streamConfiguration(Camera *camera,
>  					 const std::vector<StreamUsage> &usages)
>  {
> -	CameraConfiguration configs;
>  	IPU3CameraData *data = cameraData(camera);
> -	StreamConfiguration config = {};
> +	CameraConfiguration configs;
> +	std::vector<IPU3Stream *> availableStreams = {

Using a std::set instead of std::vector here would be useful. As you 
can't have the same stream in the set twice there is no limitation 
moving from a vector to a set.

> +		&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 'output' format 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 'viewfinder' format set to " << config.width << "x"
> -		<< config.height << "-0x" << std::hex << std::setfill('0')
> -		<< std::setw(8) << config.pixelFormat;
> +	for (const StreamUsage &usage : usages) {
> +		std::vector<IPU3Stream *>::iterator found;

Can be refactored away if availableStreams is a std::set, see example 
bellow.

> +		enum StreamUsage::Role r = usage.role();

s/enum//

> +		StreamConfiguration config = {};
> +		IPU3Stream *stream = nullptr;
> +
> +		std::vector<IPU3Stream *>::iterator s = availableStreams.begin();
> +		if (r == StreamUsage::Role::StillCapture) {
> +			for (; s < availableStreams.end(); ++s) {
> +				/*
> +				 * We can use the viewfinder stream in case
> +				 * the 'StillCapture' usage is required
> +				 * multiple times.
> +				 */
> +				if (*s == &data->outStream_) {
> +					stream = &data->outStream_;
> +					found = s;
> +					break;
> +				} else {
> +					stream = &data->vfStream_;
> +					found = s;
> +				}
> +			}

The for() loop can be replaced with this if availableStreams is std::set


    if (availableStreams.find(&data->outStream_) != availableStreams.end())
        stream = &data->outStream_;
    else
        stream = &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 alignment requirements.
> +			 */
> +			config.width = 2560;
> +			config.height = 1920;
> +		} else if (r == StreamUsage::Role::Viewfinder ||
> +			   r == StreamUsage::Role::VideoRecording) {
> +			for (; s < availableStreams.end(); ++s) {
> +				/*
> +				 * We can't use the 'output' stream for
> +				 * viewfinder or video capture usages.
> +				 */
> +				if (*s != &data->vfStream_)
> +					continue;
> +
> +				stream = &data->vfStream_;
> +				found = s;
> +				break;
> +			}
> +
> +			config.width = 640;
> +			config.height = 480;
> +		}
> +
> +		if (stream == nullptr)
> +			goto error;
> +
> +		availableStreams.erase(found);

With std::set

    availableStreams.erase(stream);

> +
> +		config.pixelFormat = V4L2_PIX_FMT_NV12;
> +		config.bufferCount = IPU3_BUFFER_COUNT;
> +
> +		configs[stream] = config;
> +
> +		LOG(IPU3, Debug)
> +			<< "Stream " << stream->name_ << " format set to "
> +			<< config.width << "x" << config.height
> +			<< "-0x" << std::hex << std::setfill('0')
> +			<< std::setw(8) << config.pixelFormat;
> +	}
>  
>  	return configs;
> +
> +error:
> +	LOG(IPU3, Error) << "Requested stream roles not supported";
> +	return CameraConfiguration{};
>  }
>  
>  int PipelineHandlerIPU3::configureStreams(Camera *camera,
> -- 
> 2.21.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund