[libcamera-devel] [PATCH 1/6] libcamera: camera_sensor: Add dev() operation

[Laurent Pinchart]

Hi Kieran,

On Sun, Jun 09, 2019 at 01:05:13AM +0100, Kieran Bingham wrote:
> On 03/06/2019 12:31, Laurent Pinchart wrote:
> > On Sun, Jun 02, 2019 at 03:04:30PM +0200, Jacopo Mondi wrote:
> >> Add dev() operation to the CameraSensor class to access the
> >> V4L2Subdevice backing the camera sensor.
> >>
> >> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> >> ---
> >>  src/libcamera/camera_sensor.cpp       | 6 ++++++
> >>  src/libcamera/include/camera_sensor.h | 1 +
> >>  2 files changed, 7 insertions(+)
> >>
> >> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> >> index 2b9d8fa593c1..8cbef8bccbef 100644
> >> --- a/src/libcamera/camera_sensor.cpp
> >> +++ b/src/libcamera/camera_sensor.cpp
> >> @@ -130,6 +130,12 @@ int CameraSensor::init()
> >>   * \return The sensor media entity
> >>   */
> >>  
> >> +/**
> >> + * \fn CameraSensor::dev()
> > 
> > Shouldn't this be called device() ?
> 
> Or perhaps even ::subdev() as that is the field which is returned.
> 
> > What if the camera sensor is exposed
> > through multiple subdevs ?
> 
> CameraSensor only supports a single subdev currently ... so I guess that
> would then be considered if CameraSensor were to ever be extended?
> 
> Can you imagine a CameraSensor exposing more than one subdev in the near
> future?

The smiapp driver exposes sensors through multiple subdevs, so that's
certainly something we'll need to support at some point.

> (In the context of needing to set controls on each subdev specifically?)

I'm thinking it could be a good idea to abstract sensor controls in the
CameraSensor class, and avoiding accessing the subdevs directly.

> We might have separate subdevs for handling say, flash-light controls,
> but I think perhaps we would handle that in a separate class?

Yes, that would be handled separately.

> >> + * \brief Retrieve the sensor V4L2 subdevice
> >> + * \return The sensor V4L2 subdevice
> >> + */
> >> +
> >>  /**
> >>   * \fn CameraSensor::mbusCodes()
> >>   * \brief Retrieve the media bus codes supported by the camera sensor
> >> diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h
> >> index b823480241a7..6cdf833a27bf 100644
> >> --- a/src/libcamera/include/camera_sensor.h
> >> +++ b/src/libcamera/include/camera_sensor.h
> >> @@ -33,6 +33,7 @@ public:
> >>  	int init();
> >>  
> >>  	const MediaEntity *entity() const { return entity_; }
> >> +	V4L2Subdevice *dev() const { return subdev_; }
> >>  	const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; }
> >>  	const std::vector<Size> &sizes() const { return sizes_; }
> >>  	const Size &resolution() const;

-- 
Regards,

Laurent Pinchart