[libcamera-devel] [PATCH v3 7/9] ipu3: Assign |outCaptureStream| to StillCapture configuration

[Umang Jain]

Hi Harvey,

On 6/29/22 16:00, Harvey Yang via libcamera-devel wrote:
> When StillCapture and other non-raw configurations are requested,
> assigns |outCaptureStream| instead of |outStream| to the StillCapture
> configuration.
>
> Signed-off-by: Harvey Yang <chenghaoyang at chromium.org>
> ---
>   src/libcamera/pipeline/ipu3/ipu3.cpp | 50 +++++++++++++++++++++++-----
>   1 file changed, 42 insertions(+), 8 deletions(-)
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index ec9d14d1..e26c2736 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -254,8 +254,10 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
>   	 * \todo Clarify the IF and BDS margins requirements.
>   	 */
>   	unsigned int rawCount = 0;
> -	unsigned int yuvCount = 0;
> +	unsigned int videoCount = 0;
> +	unsigned int stillCount = 0;
>   	Size maxYuvSize;
> +	Size maxVideoSize;
>   	Size rawSize;
>   
>   	for (const StreamConfiguration &cfg : config_) {
> @@ -264,16 +266,29 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
>   		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {
>   			rawCount++;
>   			rawSize.expandTo(cfg.size);
> +		} else if (cfg.streamRole == StreamRole::StillCapture) {
> +			if (stillCount != 0)
> +				return Invalid;
> +
> +			stillCount++;
> +			maxYuvSize.expandTo(cfg.size);
>   		} else {
> -			yuvCount++;
> +			videoCount++;
>   			maxYuvSize.expandTo(cfg.size);
> +			maxVideoSize.expandTo(cfg.size);
>   		}
>   	}
>   
> -	if (rawCount > 1 || yuvCount > 2) {
> +	if (videoCount == 0 && stillCount == 1) {
> +		stillCount = 0;
> +		videoCount = 1;
> +		maxVideoSize.expandTo(maxYuvSize);
> +	}


Maybe you can explain this if block with a brief comment on the case why 
we invert stillCount and videoCount.

> +
> +	if (rawCount > 1 || videoCount > 2) {
>   		LOG(IPU3, Debug) << "Camera configuration not supported";
>   		return Invalid;
> -	} else if (rawCount && !yuvCount) {
> +	} else if (rawCount && !stillCount && !videoCount) {
>   		/*
>   		 * Disallow raw-only camera configuration. Currently, ImgU does
>   		 * not get configured for raw-only streams and has early return
> @@ -316,6 +331,9 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
>   	ImgUDevice::Pipe pipe{};
>   	pipe.input = cio2Configuration_.size;
>   
> +	ImgUDevice::Pipe pipe1{};
> +	pipe1.input = cio2Configuration_.size;
> +
>   	/*
>   	 * Adjust the configurations if needed and assign streams while
>   	 * iterating them.
> @@ -380,18 +398,34 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
>   			cfg->stride = info.stride(cfg->size.width, 0, 1);
>   			cfg->frameSize = info.frameSize(cfg->size, 1);
>   
> +			if (stillCount == 1 && cfg->streamRole == StreamRole::StillCapture) {


Maybe check both is redundant? Can stillCount be converted to 
ASSERT(stillCount != 0) inside the block?

> +				LOG(IPU3, Debug) << "Assigned "
> +						 << cfg->toString()
> +						 << " to the imgu1 main output";
> +				cfg->setStream(const_cast<Stream *>(&data_->outCaptureStream_));
> +
> +				pipe1.main = cfg->size;
> +				pipe1.viewfinder = pipe1.main;
> +
> +				pipeConfig1_ = data_->imgu1_->calculatePipeConfig(&pipe1);
> +				if (pipeConfig1_.isNull()) {
> +					LOG(IPU3, Error) << "Failed to calculate pipe configuration: "
> +							 << "unsupported resolutions.";
> +					return Invalid;
> +				}
>   			/*
>   			 * Use the main output stream in case only one stream is
>   			 * requested or if the current configuration is the one
>   			 * with the maximum YUV output size.
>   			 */
> -			if (mainOutputAvailable &&
> -			    (originalCfg.size == maxYuvSize || yuvCount == 1)) {
> +			} else if (mainOutputAvailable &&
> +				   (originalCfg.size == maxVideoSize ||
> +				    videoCount == 1)) {
>   				cfg->setStream(const_cast<Stream *>(&data_->outStream_));
>   				mainOutputAvailable = false;
>   
>   				pipe.main = cfg->size;
> -				if (yuvCount == 1)
> +				if (videoCount == 1)
>   					pipe.viewfinder = pipe.main;
>   
>   				LOG(IPU3, Debug) << "Assigned " << cfg->toString()
> @@ -415,7 +449,7 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
>   	}
>   
>   	/* Only compute the ImgU configuration if a YUV stream has been requested. */
> -	if (yuvCount) {
> +	if (videoCount) {
>   		pipeConfig0_ = data_->imgu0_->calculatePipeConfig(&pipe);
>   		if (pipeConfig0_.isNull()) {
>   			LOG(IPU3, Error) << "Failed to calculate pipe configuration: "