[libcamera-devel] [RFC PATCH 1/3] libcamera: base: camera_sensor: Expose sensor's formats map

[Jacopo Mondi]

Hello Umang,

On Fri, Jul 30, 2021 at 01:58:26PM +0530, Umang Jain wrote:
> Add a getter function formats() to retrieve the V4L2Subdevice::format
> map of all the formats that the sensor supports. This will probably be
> used by pipeline handlers to match against a custom list of formats
> internally while making a selection on sensor's format to be used for
> image capture.
>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
>  include/libcamera/internal/camera_sensor.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index db12b07e..d8826e8a 100644
> --- a/include/libcamera/internal/camera_sensor.h
> +++ b/include/libcamera/internal/camera_sensor.h
> @@ -37,6 +37,7 @@ public:
>  	const std::string &model() const { return model_; }
>  	const std::string &id() const { return id_; }
>  	const MediaEntity *entity() const { return entity_; }
> +	const V4L2Subdevice::Formats &formats() const { return formats_; }

We currently have

  	const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; }
  	const std::vector<Size> &sizes() const { return sizes_; }

sizes() is used by the CIO2 class only to enumerate the supported raw
stream resolution.

Honestly exposing the list of all resolution through sizes() without
context (the mbus code those sizes can be produced with) is not nice.
It only works so far as we know on IPU3 that all the produced raw formats
are good.

I would transform sizes() in sizes(unsigned int) and if one needs to
know all the sizes it will be required to

        std::vector<Size> allSizes;
        for (mbusCode : sensor->mbusCodes())
                for (size : sensor->sizes(mbusCode))
                        allSizes.push_back(size);

but this will have duplicates, so probably the caller should use an
std::set<>.

Anyway, my point is that having

	const V4L2Subdevice::Formats &formats() const { return formats_; }

Renders

  	const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; }
  	const std::vector<Size> &sizes() const { return sizes_; }

A bit useless and exposing the v4l2 subdev formats map is possibly
undesirable, as ph will have to use the V4L2Subdevice::Format which is
V4L2 specific.

I would then rather

  	const std::vector<Size> &sizes(unsigned int mbusCode) const;

What do you think ?



>  	Size resolution() const;
> --
> 2.31.0
>