camera_sensor: Expand on computeTransform() documentation

23655 diff mbox series

The description for computeTransform() when the desired orientation
cannot be achieved, can be expanded a further bit, to clearly report
that orientation will be adjusted to native camera sensor mounting
orientation.

Signed-off-by: Umang Jain <uajain@igalia.com>
---
 src/libcamera/sensor/camera_sensor.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Hi Umang

On Wed, Jun 25, 2025 at 09:44:58PM +0530, Umang Jain wrote:
> The description for computeTransform() when the desired orientation
> cannot be achieved, can be expanded a further bit, to clearly report
> that orientation will be adjusted to native camera sensor mounting
> orientation.
>
> Signed-off-by: Umang Jain <uajain@igalia.com>

> ---
>  src/libcamera/sensor/camera_sensor.cpp | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp
> index d19b5e2e..6f4a13fe 100644
> --- a/src/libcamera/sensor/camera_sensor.cpp
> +++ b/src/libcamera/sensor/camera_sensor.cpp
> @@ -302,7 +302,8 @@ int CameraSensor::setEmbeddedDataEnabled(bool enable)
>   * camera sensor, likely at configure() time.
>   *
>   * If the requested \a orientation cannot be obtained, the \a orientation
> - * parameter is adjusted to report the current image orientation and
> + * parameter is adjusted to report the native image orientation (i.e.
> + * physical mounting orientation of the camera sensor) and

I would say "rotation" for being consistent with the fact we use
rotation for camera sensor, and orientation for images.

nit apart:
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

>   * Transform::Identity is returned.
>   *
>   * If the requested \a orientation can be obtained, the function computes a
> --
> 2.49.0
>

Hi Jacopo

On 25 June 2025 11:02:38 pm IST, Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote:
>Hi Umang
>
>On Wed, Jun 25, 2025 at 09:44:58PM +0530, Umang Jain wrote:
>> The description for computeTransform() when the desired orientation
>> cannot be achieved, can be expanded a further bit, to clearly report
>> that orientation will be adjusted to native camera sensor mounting
>> orientation.
>>
>> Signed-off-by: Umang Jain <uajain@igalia.com>
>
>> ---
>>  src/libcamera/sensor/camera_sensor.cpp | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp
>> index d19b5e2e..6f4a13fe 100644
>> --- a/src/libcamera/sensor/camera_sensor.cpp
>> +++ b/src/libcamera/sensor/camera_sensor.cpp
>> @@ -302,7 +302,8 @@ int CameraSensor::setEmbeddedDataEnabled(bool enable)
>>   * camera sensor, likely at configure() time.
>>   *
>>   * If the requested \a orientation cannot be obtained, the \a orientation
>> - * parameter is adjusted to report the current image orientation and
>> + * parameter is adjusted to report the native image orientation (i.e.
>> + * physical mounting orientation of the camera sensor) and
>
>I would say "rotation" for being consistent with the fact we use
>rotation for camera sensor, and orientation for images.

I had written 'rotation' first, however changed it to 'orientation' after looking at mountingOrientation_ in computeTransform().

Perhaps that can be changed to mountingRotation_ as well. 

>
>nit apart:
>Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>
>>   * Transform::Identity is returned.
>>   *
>>   * If the requested \a orientation can be obtained, the function computes a
>> --
>> 2.49.0
>>

Regards,
Umang

On Thu, Jun 26, 2025 at 02:35:04AM +0530, Umang Jain wrote:
> On 25 June 2025 11:02:38 pm IST, Jacopo Mondi wrote:
> >On Wed, Jun 25, 2025 at 09:44:58PM +0530, Umang Jain wrote:
> >> The description for computeTransform() when the desired orientation
> >> cannot be achieved, can be expanded a further bit, to clearly report
> >> that orientation will be adjusted to native camera sensor mounting
> >> orientation.
> >>
> >> Signed-off-by: Umang Jain <uajain@igalia.com>
> >
> >> ---
> >>  src/libcamera/sensor/camera_sensor.cpp | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp
> >> index d19b5e2e..6f4a13fe 100644
> >> --- a/src/libcamera/sensor/camera_sensor.cpp
> >> +++ b/src/libcamera/sensor/camera_sensor.cpp
> >> @@ -302,7 +302,8 @@ int CameraSensor::setEmbeddedDataEnabled(bool enable)
> >>   * camera sensor, likely at configure() time.
> >>   *
> >>   * If the requested \a orientation cannot be obtained, the \a orientation
> >> - * parameter is adjusted to report the current image orientation and
> >> + * parameter is adjusted to report the native image orientation (i.e.
> >> + * physical mounting orientation of the camera sensor) and
> >
> > I would say "rotation" for being consistent with the fact we use
> > rotation for camera sensor, and orientation for images.
> 
> I had written 'rotation' first, however changed it to 'orientation'
> after looking at mountingOrientation_ in computeTransform().
> 
> Perhaps that can be changed to mountingRotation_ as well.

mountingOrientation_ is the orientation of the image resulting from the
physical rotation of the camera sensor:

	int32_t propertyValue = rotationControl->second.def().get<int32_t>();

	/*
	 * Cache the Transform associated with the camera mounting
	 * rotation for later use in computeTransform().
	 */
	bool success;
	mountingOrientation_ = orientationFromRotation(propertyValue, &success);

The variable name therefore seems correct to me.

Would the following be clearer ?

 * If the requested \a orientation cannot be obtained, the \a orientation
 * parameter is adjusted to report the native image orientation (i.e. resulting
 * from the physical mounting rotation of the camera sensor, without any
 * transformation) and Transform::Identity is returned.

> > nit apart:
> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> >
> >>   * Transform::Identity is returned.
> >>   *
> >>   * If the requested \a orientation can be obtained, the function computes a