[libcamera-devel,3/4] ipu3: cio2: Change sensor size selection policy
diff mbox series

Message ID 20210803133205.6599-4-umang.jain@ideasonboard.com
State Superseded
Delegated to: Umang Jain
Headers show
Series
  • ipu3: Change sensor size selection policy
Related show

Commit Message

Umang Jain Aug. 3, 2021, 1:32 p.m. UTC 
From: Jacopo Mondi <jacopo@jmondi.org>

The current implementation of getSensorFormat() prioritizes sensor
sizes that match the output size FOV ratio.

Modify the frame size selection procedure to prioritize resolutions
with the same FOV as the sensor's native one, to avoid cropping in the
sensor pixel array.

In example, with the current implementation :

- on a Soraka device equipped with ov13858 as back sensor, with a
  native resolution of 4224x3136 (4:3), when requested to select the
  sensor output size to produce 1080p (16:9) a frame size of 2112x1188
  (16:9) is selected causing the ImgU configuration procedure to fail.
  If a resolution with the same FOV as the sensor's native size, such
  as 2112x1568 (4:3), is selected the pipeline works correctly.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 src/libcamera/pipeline/ipu3/cio2.cpp | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Jacopo Mondi Aug. 9, 2021, 3:59 p.m. UTC | #1 
Hi Umang,

On Tue, Aug 03, 2021 at 07:02:04PM +0530, Umang Jain wrote:
> From: Jacopo Mondi <jacopo@jmondi.org>

well, you wrote these bits, so you should get the authorship.
Maybe a Suggested-by: tag if you wish to, but it's not that necessary
:)

>
> The current implementation of getSensorFormat() prioritizes sensor
> sizes that match the output size FOV ratio.
>
> Modify the frame size selection procedure to prioritize resolutions
> with the same FOV as the sensor's native one, to avoid cropping in the
> sensor pixel array.
>
> In example, with the current implementation :

I would s/, with..//
>
> - on a Soraka device equipped with ov13858 as back sensor, with a
>   native resolution of 4224x3136 (4:3), when requested to select the
>   sensor output size to produce 1080p (16:9) a frame size of 2112x1188
>   (16:9) is selected causing the ImgU configuration procedure to fail.
>   If a resolution with the same FOV as the sensor's native size, such
>   as 2112x1568 (4:3), is selected the pipeline works correctly.
>
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>  src/libcamera/pipeline/ipu3/cio2.cpp | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
> index be063613..9980b8bd 100644
> --- a/src/libcamera/pipeline/ipu3/cio2.cpp
> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp
> @@ -239,8 +239,8 @@ StreamConfiguration CIO2Device::generateConfiguration(Size size) const
>   *
>   * - The desired \a size shall fit in the sensor output size to avoid the need
>   *   to up-scale.
> - * - The sensor output size shall match the desired aspect ratio to avoid the
> - *   need to crop the field of view.
> + * - The aspect ratio of sensor output size shall be as close as possible to
> + *   the sensor's native resolution field of view.
>   * - The sensor output size shall be as small as possible to lower the required
>   *   bandwidth.
>   * - The desired \a size shall be supported by one of the media bus code listed
> @@ -264,7 +264,9 @@ V4L2SubdeviceFormat CIO2Device::getSensorFormat(const std::vector<unsigned int>
>  {
>  	unsigned int desiredArea = size.width * size.height;
>  	unsigned int bestArea = UINT_MAX;
> -	float desiredRatio = static_cast<float>(size.width) / size.height;
> +	const Size &resolution = sensor_->resolution();
> +	float desiredRatio = static_cast<float>(resolution.width) /
> +			     resolution.height;

Does this fix your issues with Nautilus ?

The patch looks good!
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
   j
>  	float bestRatio = FLT_MAX;
>  	Size bestSize;
>  	uint32_t bestCode = 0;
> --
> 2.31.0
>
Umang Jain Aug. 9, 2021, 4:07 p.m. UTC | #2 
Hi Jacopo

On 8/9/21 9:29 PM, Jacopo Mondi wrote:
> Hi Umang,
>
> On Tue, Aug 03, 2021 at 07:02:04PM +0530, Umang Jain wrote:
>> From: Jacopo Mondi <jacopo@jmondi.org>
> well, you wrote these bits, so you should get the authorship.
> Maybe a Suggested-by: tag if you wish to, but it's not that necessary
> :)
>
>> The current implementation of getSensorFormat() prioritizes sensor
>> sizes that match the output size FOV ratio.
>>
>> Modify the frame size selection procedure to prioritize resolutions
>> with the same FOV as the sensor's native one, to avoid cropping in the
>> sensor pixel array.
>>
>> In example, with the current implementation :
> I would s/, with..//
>> - on a Soraka device equipped with ov13858 as back sensor, with a
>>    native resolution of 4224x3136 (4:3), when requested to select the
>>    sensor output size to produce 1080p (16:9) a frame size of 2112x1188
>>    (16:9) is selected causing the ImgU configuration procedure to fail.
>>    If a resolution with the same FOV as the sensor's native size, such
>>    as 2112x1568 (4:3), is selected the pipeline works correctly.
>>
>> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
>> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
>> ---
>>   src/libcamera/pipeline/ipu3/cio2.cpp | 8 +++++---
>>   1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
>> index be063613..9980b8bd 100644
>> --- a/src/libcamera/pipeline/ipu3/cio2.cpp
>> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp
>> @@ -239,8 +239,8 @@ StreamConfiguration CIO2Device::generateConfiguration(Size size) const
>>    *
>>    * - The desired \a size shall fit in the sensor output size to avoid the need
>>    *   to up-scale.
>> - * - The sensor output size shall match the desired aspect ratio to avoid the
>> - *   need to crop the field of view.
>> + * - The aspect ratio of sensor output size shall be as close as possible to
>> + *   the sensor's native resolution field of view.
>>    * - The sensor output size shall be as small as possible to lower the required
>>    *   bandwidth.
>>    * - The desired \a size shall be supported by one of the media bus code listed
>> @@ -264,7 +264,9 @@ V4L2SubdeviceFormat CIO2Device::getSensorFormat(const std::vector<unsigned int>
>>   {
>>   	unsigned int desiredArea = size.width * size.height;
>>   	unsigned int bestArea = UINT_MAX;
>> -	float desiredRatio = static_cast<float>(size.width) / size.height;
>> +	const Size &resolution = sensor_->resolution();
>> +	float desiredRatio = static_cast<float>(resolution.width) /
>> +			     resolution.height;
> Does this fix your issues with Nautilus ?

4/4 is the fix for nautilus. This one is mainly for soraka. It 
(standalone) doesn't change status quo on nautilus as far as I remember 
in my testing.

Thanks!

>
> The patch looks good!
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
>
> Thanks
>     j
>>   	float bestRatio = FLT_MAX;
>>   	Size bestSize;
>>   	uint32_t bestCode = 0;
>> --
>> 2.31.0
>>

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
index be063613..9980b8bd 100644
--- a/src/libcamera/pipeline/ipu3/cio2.cpp
+++ b/src/libcamera/pipeline/ipu3/cio2.cpp
@@ -239,8 +239,8 @@  StreamConfiguration CIO2Device::generateConfiguration(Size size) const
  *
  * - The desired \a size shall fit in the sensor output size to avoid the need
  *   to up-scale.
- * - The sensor output size shall match the desired aspect ratio to avoid the
- *   need to crop the field of view.
+ * - The aspect ratio of sensor output size shall be as close as possible to
+ *   the sensor's native resolution field of view.
  * - The sensor output size shall be as small as possible to lower the required
  *   bandwidth.
  * - The desired \a size shall be supported by one of the media bus code listed
@@ -264,7 +264,9 @@  V4L2SubdeviceFormat CIO2Device::getSensorFormat(const std::vector<unsigned int>
 {
 	unsigned int desiredArea = size.width * size.height;
 	unsigned int bestArea = UINT_MAX;
-	float desiredRatio = static_cast<float>(size.width) / size.height;
+	const Size &resolution = sensor_->resolution();
+	float desiredRatio = static_cast<float>(resolution.width) /
+			     resolution.height;
 	float bestRatio = FLT_MAX;
 	Size bestSize;
 	uint32_t bestCode = 0;