[libcamera-devel] Constraints on the sensor subdevice

[Naushir Patuck]

HI Jacopo,


On Thu, 6 Feb 2020 at 19:03, Jacopo Mondi <jacopo at jmondi.org> wrote:
>
> Hello Naush,
>
> On Thu, Feb 06, 2020 at 10:21:06AM +0000, Naushir Patuck wrote:
> > Hi all,
> >
> > I just wanted to query a couple of constraints on the sensor subdevice
> > that we have come across (in camera._sensor.cpp -
> > CameraSensor::init()):
> >
>
> Thanks, both your points are very appropriate. Limitations where known
> https://git.linuxtv.org/libcamera.git/tree/src/libcamera/camera_sensor.cpp#n37
> but I think it's now the right time to address them \o/
>
> I will reply to only the first point here, but won't forget the
> second, I pinky-promise that :)

I will remember this :)

>
> > 1) Sensor subdevice must have only one pad.
> > As you know, right now we are overloading the subdevice pad to
> > advertise sensor embedded data is available.  Eventually, we intend to
> > add a "stream" property to the subdevice that can be used instead, but
> > that may be some time before that's ready.  Would it be possible to
> > remove this constraint?
>
> yes indeed. I just submitted a series that introduces a factory of
> camera sensor handler, as the idea here is that management of more
> complex devices (hopefully all of them :) should be delegated to a
> CameraSensor sub-class.
>
> Sensor-specific handlers are meant to precisely interface with
> sensor and their associated drivers, including handling more complex
> topologies exposed to userspace such as in the case you have here
> described.
>
> The question I now would have is which one, among the available entites
> exposed by a sensor driver, should be used to 'identify' the sensor
> when asking the factory to create an handler (have a look at
> CameraSensorFactory::createSensor(MediaEntity *entity) in my series).
>
> I presume it is fair to consider the sensor's subdevice most close to
> the ISP the one that is used to identify the sensor. In most cases, if
> not all, it will be the CSI-2 transmitter entity. Once the specialized
> sensor handler class receives this at creation time, it can walk the
> links directed to its sink pads backward, to discover all the other
> subdevices incrementally.
>
> Would something like that work for you ?
>

I need to have a look at the patch-set to see what is available, but
it sounds like this could replace the code that I was talking about
earlier.  I'll look at this again early next week and let me get back
to you then.

Regards,
Naush


> Thanks
>    j
>
> >
> > 3) Sensor subdevices must advertise the same frame sizes for all MBUS codes.
> > There are sensors that we currently use (e.g. ov5647) that advertise a
> > number of 10-bit modes (preferred) as well as a single 8-bit more
> > (legacy use).  Other sensors may require lower bit depth formats for
> > fast frame rate modes as another example.  With these configurations,
> > this constraint seems too restrictive.
> >
> > From the code in camera_sensor.cpp, I can see why this is needed
> > (mbusCodes_ and sizes_ are associated std::vectors.  Switching to
> > using something like class ImageFormats (std::map with MBUS code as a
> > key) would seem to solve this problem.  What do you think?
> >
> > Regards,
> > Naush
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel