[libcamera-devel] [PATCH v3 2/6] libcamera: Initialise the SensorOutputSize property

[Jacopo Mondi]

Hi Kieran,

On Wed, Sep 30, 2020 at 09:31:22AM +0100, Kieran Bingham wrote:
>
>
> On 30/09/2020 09:32, Jacopo Mondi wrote:
> > Hi Kieran,
> >
> > On Tue, Sep 29, 2020 at 08:53:55PM +0100, Kieran Bingham wrote:
> >> Hi David,
> >>
> >> On 29/09/2020 17:39, David Plowman wrote:
> >>> Add a default initialisation according to the sensor resolution,
> >>> though it will need updating when the camera mode changes.
> >>>
> >>> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> >>> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> >>> ---
> >>>  src/libcamera/camera_sensor.cpp | 6 ++++++
> >>>  1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> >>> index d2679a4b..c9a19b02 100644
> >>> --- a/src/libcamera/camera_sensor.cpp
> >>> +++ b/src/libcamera/camera_sensor.cpp
> >>> @@ -277,6 +277,12 @@ int CameraSensor::init()
> >>>  	 */
> >>>  	resolution_ = sizes_.back();
> >>>
> >>> +	/*
> >>> +	 * Set a default value for the SensorOutputSize, though it will have to
> >>> +	 * be updated when new camera modes are chosen.
> >>> +	 */
> >>> +	properties_.set(properties::SensorOutputSize, resolution_);
> >>
> >> I guess here, this is just an initialisation, but doesn't reflect what's
> >> actually set on the sensor yet.
> >>
> >> Should we also set this during setFormat() ?
> >
> > Before the Camera is configured the value of the property is not
> > relevant. I think the property documentation reports that.
>
> My point/question was - is the 'real' sensor output size modified (or
> potentially modified) when ever a call is made through setFormat, and
> thus - should the SensorOutputSize be updated to reflect that.

Assuming the discussion on the other patch is settled (have the
property in Camera and not in CameraConfiguration and thus the
property is updated at Camera::configure() time), the way the
property is updated is up to the pipeline handler. RPi in example does
not set the sensor format on the CameraSensor at all, as it happens as
a consequence of a setFormat on the CSI-2 video device node and the
format is propagated in kernel space.

>
> Or is SensorOutputSize a fixed immutable thing that never changes?

It does indeed change here, some pipeline might decide they always go
full size. Anyway applications are meant to re-query the property
after a Camera::configure()

>
> Maybe that's my real question.
>
> *when* and *where* does this get updated when things change.

On pipeline handler side. Some pipeline handlers might want to
calculate it themselves, other will fetch the value directly from
CameraSensor after a CameraSensor::setFormat() (in which case the
operation should update the property, but at the moment it is not
required for the RPi specificities described here above).

Thanks
  j

> --
> Kieran
>
>
> >
> >>
> >>> +
> >>>  	return 0;
> >>>  }
> >>>
> >>>
> >>
> >> --
> >> Regards
> >> --
> >> Kieran
> >> _______________________________________________
> >> libcamera-devel mailing list
> >> libcamera-devel at lists.libcamera.org
> >> https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards
> --
> Kieran