[libcamera-devel,v2,04/20] libcamera: ipu3: cio2: Report format and sizes
diff mbox series

Message ID 20200709084128.5316-5-jacopo@jmondi.org
State Accepted
Headers show
Series
  • libcamera: ipu3: Rework configuration
Related show

Commit Message

Jacopo Mondi July 9, 2020, 8:41 a.m. UTC 
Add two methods to the CIO2Device class to retrieve all the supported
PixelFormats and sizes.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/pipeline/ipu3/cio2.cpp | 40 ++++++++++++++++++++++++++++
 src/libcamera/pipeline/ipu3/cio2.h   |  5 ++++
 2 files changed, 45 insertions(+)

Comments

Niklas Söderlund July 9, 2020, 1:14 p.m. UTC | #1 
Hi Jacopo,

Thanks for your work.

On 2020-07-09 10:41:12 +0200, Jacopo Mondi wrote:
> Add two methods to the CIO2Device class to retrieve all the supported
> PixelFormats and sizes.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/libcamera/pipeline/ipu3/cio2.cpp | 40 ++++++++++++++++++++++++++++
>  src/libcamera/pipeline/ipu3/cio2.h   |  5 ++++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
> index f5a01dd3ec1f..525513317604 100644
> --- a/src/libcamera/pipeline/ipu3/cio2.cpp
> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp
> @@ -9,6 +9,7 @@
>  
>  #include <linux/media-bus-format.h>
>  
> +#include <libcamera/geometry.h>
>  #include <libcamera/formats.h>
>  #include <libcamera/stream.h>
>  
> @@ -43,6 +44,45 @@ CIO2Device::~CIO2Device()
>  	delete sensor_;
>  }
>  
> +/**
> + * \brief Retrieve the list of supported PixelFormats
> + *
> + * Retrieve the list of supported pixel formats by matching the sensor produced
> + * media bus codes with the formats supported by the CIO2 unit.
> + *
> + * \return The list of supported PixelFormat
> + */
> +std::vector<PixelFormat> CIO2Device::formats() const

In practice would this function not always return a vector with a single 
element? As the CIO2 only supports the IPU3 specific memory layout that 
uses 10-bits samples. That means the only difference between the 
supported formats is the RGB mask of the sensor. The sensor can not 
change its RGB mask (right?).

> +{
> +	if (!sensor_)
> +		return {};
> +
> +	std::vector<PixelFormat> formats;
> +	for (unsigned int code : sensor_->mbusCodes()) {
> +		auto it = mbusCodesToPixelFormat.find(code);
> +		if (it != mbusCodesToPixelFormat.end())
> +			formats.push_back(it->second);
> +	}
> +
> +	return formats;
> +}
> +
> +/**
> + * \brief Retrieve the list of supported size ranges
> + * \return The list of supported SizeRange
> + */
> +std::vector<SizeRange> CIO2Device::sizes() const
> +{
> +	if (!sensor_)
> +		return {};
> +
> +	std::vector<SizeRange> sizes;
> +	for (const Size &size : sensor_->sizes())
> +		sizes.emplace_back(size, size);
> +
> +	return sizes;
> +}
> +
>  /**
>   * \brief Initialize components of the CIO2 device with \a index
>   * \param[in] media The CIO2 media device
> diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h
> index 4fd949f8e513..f905d97fa79d 100644
> --- a/src/libcamera/pipeline/ipu3/cio2.h
> +++ b/src/libcamera/pipeline/ipu3/cio2.h
> @@ -20,7 +20,9 @@ namespace libcamera {
>  class CameraSensor;
>  class FrameBuffer;
>  class MediaDevice;
> +class PixelFormat;
>  class Request;
> +class SizeRange;
>  class V4L2Subdevice;
>  struct Size;
>  struct StreamConfiguration;
> @@ -33,6 +35,9 @@ public:
>  	CIO2Device();
>  	~CIO2Device();
>  
> +	std::vector<PixelFormat> formats() const;
> +	std::vector<SizeRange> sizes() const;
> +
>  	int init(const MediaDevice *media, unsigned int index);
>  	int configure(const Size &size, V4L2DeviceFormat *outputFormat);
>  
> -- 
> 2.27.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Jacopo Mondi July 10, 2020, 7:06 a.m. UTC | #2 
Hi Niklas,

On Thu, Jul 09, 2020 at 03:14:48PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2020-07-09 10:41:12 +0200, Jacopo Mondi wrote:
> > Add two methods to the CIO2Device class to retrieve all the supported
> > PixelFormats and sizes.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  src/libcamera/pipeline/ipu3/cio2.cpp | 40 ++++++++++++++++++++++++++++
> >  src/libcamera/pipeline/ipu3/cio2.h   |  5 ++++
> >  2 files changed, 45 insertions(+)
> >
> > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
> > index f5a01dd3ec1f..525513317604 100644
> > --- a/src/libcamera/pipeline/ipu3/cio2.cpp
> > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp
> > @@ -9,6 +9,7 @@
> >
> >  #include <linux/media-bus-format.h>
> >
> > +#include <libcamera/geometry.h>
> >  #include <libcamera/formats.h>
> >  #include <libcamera/stream.h>
> >
> > @@ -43,6 +44,45 @@ CIO2Device::~CIO2Device()
> >  	delete sensor_;
> >  }
> >
> > +/**
> > + * \brief Retrieve the list of supported PixelFormats
> > + *
> > + * Retrieve the list of supported pixel formats by matching the sensor produced
> > + * media bus codes with the formats supported by the CIO2 unit.
> > + *
> > + * \return The list of supported PixelFormat
> > + */
> > +std::vector<PixelFormat> CIO2Device::formats() const
>
> In practice would this function not always return a vector with a single
> element? As the CIO2 only supports the IPU3 specific memory layout that
> uses 10-bits samples. That means the only difference between the
> supported formats is the RGB mask of the sensor. The sensor can not
> change its RGB mask (right?).

With RGB mask do you mean the color components order ? ie. SGBRG vs
SBGGR ?

In that case, the components order might be changed by applying a
vertical (horizontal too?) flip. Some sensor driver 'move' the
selection area one colum to the left/top to maintain the field of view
causing the bayer patter order to be changed.

In example, from imx219 driver
        /*
         * The supported formats.
         * This table MUST contain 4 entries per format, to cover the various flip
         * combinations in the order
         * - no flip
         * - h flip
         * - v flip
         * - h&v flips
         */
        static const u32 codes[] = {
                MEDIA_BUS_FMT_SRGGB10_1X10,
                MEDIA_BUS_FMT_SGRBG10_1X10,
                MEDIA_BUS_FMT_SGBRG10_1X10,
                MEDIA_BUS_FMT_SBGGR10_1X10,

                MEDIA_BUS_FMT_SRGGB8_1X8,
                MEDIA_BUS_FMT_SGRBG8_1X8,
                MEDIA_BUS_FMT_SGBRG8_1X8,
                MEDIA_BUS_FMT_SBGGR8_1X8,
        };

I also presume some sensor allows you to contol the bayer components
ordering by moving the selection rectangle, without having to flip.

In any case, multiple RAW bayer formats might be reported by the
sensor, so there's no guarantee the vector will have a single entry.

Thanks
  j

>
> > +{
> > +	if (!sensor_)
> > +		return {};
> > +
> > +	std::vector<PixelFormat> formats;
> > +	for (unsigned int code : sensor_->mbusCodes()) {
> > +		auto it = mbusCodesToPixelFormat.find(code);
> > +		if (it != mbusCodesToPixelFormat.end())
> > +			formats.push_back(it->second);
> > +	}
> > +
> > +	return formats;
> > +}
> > +
> > +/**
> > + * \brief Retrieve the list of supported size ranges
> > + * \return The list of supported SizeRange
> > + */
> > +std::vector<SizeRange> CIO2Device::sizes() const
> > +{
> > +	if (!sensor_)
> > +		return {};
> > +
> > +	std::vector<SizeRange> sizes;
> > +	for (const Size &size : sensor_->sizes())
> > +		sizes.emplace_back(size, size);
> > +
> > +	return sizes;
> > +}
> > +
> >  /**
> >   * \brief Initialize components of the CIO2 device with \a index
> >   * \param[in] media The CIO2 media device
> > diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h
> > index 4fd949f8e513..f905d97fa79d 100644
> > --- a/src/libcamera/pipeline/ipu3/cio2.h
> > +++ b/src/libcamera/pipeline/ipu3/cio2.h
> > @@ -20,7 +20,9 @@ namespace libcamera {
> >  class CameraSensor;
> >  class FrameBuffer;
> >  class MediaDevice;
> > +class PixelFormat;
> >  class Request;
> > +class SizeRange;
> >  class V4L2Subdevice;
> >  struct Size;
> >  struct StreamConfiguration;
> > @@ -33,6 +35,9 @@ public:
> >  	CIO2Device();
> >  	~CIO2Device();
> >
> > +	std::vector<PixelFormat> formats() const;
> > +	std::vector<SizeRange> sizes() const;
> > +
> >  	int init(const MediaDevice *media, unsigned int index);
> >  	int configure(const Size &size, V4L2DeviceFormat *outputFormat);
> >
> > --
> > 2.27.0
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards,
> Niklas Söderlund

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
index f5a01dd3ec1f..525513317604 100644
--- a/src/libcamera/pipeline/ipu3/cio2.cpp
+++ b/src/libcamera/pipeline/ipu3/cio2.cpp
@@ -9,6 +9,7 @@ 
 
 #include <linux/media-bus-format.h>
 
+#include <libcamera/geometry.h>
 #include <libcamera/formats.h>
 #include <libcamera/stream.h>
 
@@ -43,6 +44,45 @@  CIO2Device::~CIO2Device()
 	delete sensor_;
 }
 
+/**
+ * \brief Retrieve the list of supported PixelFormats
+ *
+ * Retrieve the list of supported pixel formats by matching the sensor produced
+ * media bus codes with the formats supported by the CIO2 unit.
+ *
+ * \return The list of supported PixelFormat
+ */
+std::vector<PixelFormat> CIO2Device::formats() const
+{
+	if (!sensor_)
+		return {};
+
+	std::vector<PixelFormat> formats;
+	for (unsigned int code : sensor_->mbusCodes()) {
+		auto it = mbusCodesToPixelFormat.find(code);
+		if (it != mbusCodesToPixelFormat.end())
+			formats.push_back(it->second);
+	}
+
+	return formats;
+}
+
+/**
+ * \brief Retrieve the list of supported size ranges
+ * \return The list of supported SizeRange
+ */
+std::vector<SizeRange> CIO2Device::sizes() const
+{
+	if (!sensor_)
+		return {};
+
+	std::vector<SizeRange> sizes;
+	for (const Size &size : sensor_->sizes())
+		sizes.emplace_back(size, size);
+
+	return sizes;
+}
+
 /**
  * \brief Initialize components of the CIO2 device with \a index
  * \param[in] media The CIO2 media device
diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h
index 4fd949f8e513..f905d97fa79d 100644
--- a/src/libcamera/pipeline/ipu3/cio2.h
+++ b/src/libcamera/pipeline/ipu3/cio2.h
@@ -20,7 +20,9 @@  namespace libcamera {
 class CameraSensor;
 class FrameBuffer;
 class MediaDevice;
+class PixelFormat;
 class Request;
+class SizeRange;
 class V4L2Subdevice;
 struct Size;
 struct StreamConfiguration;
@@ -33,6 +35,9 @@  public:
 	CIO2Device();
 	~CIO2Device();
 
+	std::vector<PixelFormat> formats() const;
+	std::vector<SizeRange> sizes() const;
+
 	int init(const MediaDevice *media, unsigned int index);
 	int configure(const Size &size, V4L2DeviceFormat *outputFormat);