[libcamera-devel] [RFC] libcamera: ipu3: Always use sensor full frame size

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2020-09-09 15:10:14 +0200, Jacopo Mondi wrote:
> When calculating the pipeline configuration for the IPU3 platform,
> libcamera tries to be smart and select the smaller sensor frame
> resolution larger enough to accommodate the stream sizes
> requested by the application.
> 
> While this seems to make a lot of sense, in practice optimizing the
> selected sensor resolution makes the pipeline configuration calculation
> process fail in multiple occasions, or results in stalls in the capture
> process.
> 
> As a trivial example, capturing with cam with the following command
> line result in a stall:
> $ cam -swidth=1280,height=720 -swidth=640,height=480 -c1 -C
> 
> Likewise, the Android HAL supported format enumeration fails in
> reporting smaller resolutions as supported, when used with the OV5670
> sensor.
> 
> 320x240:
> DEBUG IPU3 ipu3.cpp:192 CIO2 configuration: 648x486-SGRBG10_IPU3
> ERROR IPU3 imgu.cpp:408 Failed to calculate pipe configuration
> ERROR IPU3 ipu3.cpp:299 Failed to calculate pipe configuration: unsupported resolutions.
> 
> 640x480:
> DEBUG IPU3 ipu3.cpp:192 CIO2 configuration: 648x486-SGRBG10_IPU3
> ERROR IPU3 imgu.cpp:408 Failed to calculate pipe configuration
> ERROR IPU3 ipu3.cpp:299 Failed to calculate pipe configuration: unsupported resolutions.
> 
> Furthermore the reference .xml files used for the IPU3 camera
> configuration on the ChromeOS platform restrict the number of sensor
> resolution to be used for the OV5670 sensor to 2 from the 6 supported by
> the driver [1].
> 
> The selection criteria of the correct CIO2 mode are not specified, and
> for the time being, always use the sensor maximum resolution at the
> expense of frame rate and bus bandwidth allows the pipeline to successfully
> support smaller modes for the OV5670 sensor and solves pipeline stalls
> when capturing with both sensors.
> 
> [1] See the <sensor_modes> enumeration in:
> https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/master/baseboard-poppy/media-libs/cros-camera-hal-configs-poppy/files/gcss/graph_settings_ov5670.xml
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>

I love how the IPU3 configuration depends on a xml file ;-) Given that 
our current configuration depends on this and assumes the larger 
resolution I think this is the right move.

Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 2d881fe28f98..488e9fff299e 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -155,14 +155,12 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
>  	unsigned int rawCount = 0;
>  	unsigned int yuvCount = 0;
>  	Size maxYuvSize;
> -	Size maxRawSize;
>  
>  	for (const StreamConfiguration &cfg : config_) {
>  		const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);
>  
>  		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {
>  			rawCount++;
> -			maxRawSize.expandTo(cfg.size);
>  		} else {
>  			yuvCount++;
>  			maxYuvSize.expandTo(cfg.size);
> @@ -174,18 +172,13 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
>  		return Invalid;
>  	}
>  
> -	if (maxRawSize.isNull())
> -		maxRawSize = maxYuvSize.alignedUpTo(IMGU_OUTPUT_WIDTH_MARGIN,
> -						    IMGU_OUTPUT_HEIGHT_MARGIN)
> -				       .boundedTo(data_->cio2_.sensor()->resolution());
> -
>  	/*
>  	 * Generate raw configuration from CIO2.
>  	 *
>  	 * The output YUV streams will be limited in size to the maximum
>  	 * frame size requested for the RAW stream.
>  	 */
> -	cio2Configuration_ = data_->cio2_.generateConfiguration(maxRawSize);
> +	cio2Configuration_ = data_->cio2_.generateConfiguration({});
>  	if (!cio2Configuration_.pixelFormat.isValid())
>  		return Invalid;
>  
> -- 
> 2.28.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund