[libcamera-devel,3/5] libcamera: imx8-isi: Automatically select media bus code
diff mbox series

Message ID 20230129135830.27490-4-jacopo.mondi@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: imx8-isi: Remove pixelformat-2-media-bus map
Related show

Commit Message

Jacopo Mondi Jan. 29, 2023, 1:58 p.m. UTC 
The ISICameraConfiguration::validate() function selects which media
bus format to configure the sensor with based on the pixel format
of the first configured stream using the media bus code associated to it
in the formatsMap_ map.

In order to remove the PixelFormamt-to-mbus-code association in
formatsMap_ provide a wrapper function for the newly introduced
getRawMediaBusFormat() and getYubMediaBusFormat() that automatically
selects what media bus format to use based on the first stream pixel
format.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

Comments

Paul Elder March 2, 2023, 11:36 a.m. UTC | #1 
On Sun, Jan 29, 2023 at 02:58:28PM +0100, Jacopo Mondi via libcamera-devel wrote:
> The ISICameraConfiguration::validate() function selects which media
> bus format to configure the sensor with based on the pixel format
> of the first configured stream using the media bus code associated to it
> in the formatsMap_ map.
> 
> In order to remove the PixelFormamt-to-mbus-code association in
> formatsMap_ provide a wrapper function for the newly introduced
> getRawMediaBusFormat() and getYubMediaBusFormat() that automatically

s/Yub/Yub/

> selects what media bus format to use based on the first stream pixel
> format.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> index 445fad32656c..5976a63d27dd 100644
> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> @@ -61,6 +61,7 @@ public:
>  
>  	unsigned int getRawMediaBusFormat(PixelFormat *pixelFormat) const;
>  	unsigned int getYuvMediaBusFormat(PixelFormat *pixelFormat) const;
> +	unsigned int getMediaBusFormat(PixelFormat *pixelFormat) const;
>  
>  	std::unique_ptr<CameraSensor> sensor_;
>  	std::unique_ptr<V4L2Subdevice> csis_;
> @@ -315,6 +316,15 @@ unsigned int ISICameraData::getYuvMediaBusFormat(PixelFormat *pixelFormat) const
>  	return supportedCodes[0];
>  }
>  
> +unsigned int ISICameraData::getMediaBusFormat(PixelFormat *pixelFormat) const
> +{
> +	if (PixelFormatInfo::info(*pixelFormat).colourEncoding ==
> +	    PixelFormatInfo::ColourEncodingRAW)
> +		return getRawMediaBusFormat(pixelFormat);
> +
> +	return getYuvMediaBusFormat(pixelFormat);
> +}
> +
>  /* -----------------------------------------------------------------------------
>   * Camera Configuration
>   */
> @@ -628,16 +638,16 @@ CameraConfiguration::Status ISICameraConfiguration::validate()
>  	 * image quality in exchange of a usually slower frame rate.
>  	 * Usage of the STILL_CAPTURE role could be consider for this.
>  	 */
> -	const PipeFormat &pipeFmt = formatsMap_.at(config_[0].pixelFormat);
> -
>  	Size maxSize;
>  	for (const auto &cfg : config_) {
>  		if (cfg.size > maxSize)
>  			maxSize = cfg.size;
>  	}
>  
> +	PixelFormat pixelFormat = config_[0].pixelFormat;
> +
>  	V4L2SubdeviceFormat sensorFormat{};
> -	sensorFormat.mbus_code = pipeFmt.sensorCode;
> +	sensorFormat.mbus_code = data_->getMediaBusFormat(&pixelFormat);
>  	sensorFormat.size = maxSize;
>  
>  	LOG(ISI, Debug) << "Computed sensor configuration: " << sensorFormat;
> -- 
> 2.39.0
>
Dan Scally March 7, 2023, 2:09 p.m. UTC | #2 
Hi Jacopo

On 29/01/2023 13:58, Jacopo Mondi wrote:
> The ISICameraConfiguration::validate() function selects which media
> bus format to configure the sensor with based on the pixel format
> of the first configured stream using the media bus code associated to it
> in the formatsMap_ map.
>
> In order to remove the PixelFormamt-to-mbus-code association in
> formatsMap_ provide a wrapper function for the newly introduced
> getRawMediaBusFormat() and getYubMediaBusFormat() that automatically


s/getYubMediaBusFormat/getYuvMediaBusFormat here

> selects what media bus format to use based on the first stream pixel
> format.
>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>   src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 16 +++++++++++++---
>   1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> index 445fad32656c..5976a63d27dd 100644
> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> @@ -61,6 +61,7 @@ public:
>   
>   	unsigned int getRawMediaBusFormat(PixelFormat *pixelFormat) const;
>   	unsigned int getYuvMediaBusFormat(PixelFormat *pixelFormat) const;
> +	unsigned int getMediaBusFormat(PixelFormat *pixelFormat) const;
>   
>   	std::unique_ptr<CameraSensor> sensor_;
>   	std::unique_ptr<V4L2Subdevice> csis_;
> @@ -315,6 +316,15 @@ unsigned int ISICameraData::getYuvMediaBusFormat(PixelFormat *pixelFormat) const
>   	return supportedCodes[0];
>   }
>   
> +unsigned int ISICameraData::getMediaBusFormat(PixelFormat *pixelFormat) const
> +{
> +	if (PixelFormatInfo::info(*pixelFormat).colourEncoding ==
> +	    PixelFormatInfo::ColourEncodingRAW)
> +		return getRawMediaBusFormat(pixelFormat);
> +
> +	return getYuvMediaBusFormat(pixelFormat);
> +}
> +
>   /* -----------------------------------------------------------------------------
>    * Camera Configuration
>    */
> @@ -628,16 +638,16 @@ CameraConfiguration::Status ISICameraConfiguration::validate()
>   	 * image quality in exchange of a usually slower frame rate.
>   	 * Usage of the STILL_CAPTURE role could be consider for this.
>   	 */
> -	const PipeFormat &pipeFmt = formatsMap_.at(config_[0].pixelFormat);
> -
>   	Size maxSize;
>   	for (const auto &cfg : config_) {
>   		if (cfg.size > maxSize)
>   			maxSize = cfg.size;
>   	}
>   
> +	PixelFormat pixelFormat = config_[0].pixelFormat;
> +
>   	V4L2SubdeviceFormat sensorFormat{};
> -	sensorFormat.mbus_code = pipeFmt.sensorCode;
> +	sensorFormat.mbus_code = data_->getMediaBusFormat(&pixelFormat);
>   	sensorFormat.size = maxSize;
>   
>   	LOG(ISI, Debug) << "Computed sensor configuration: " << sensorFormat;
Laurent Pinchart March 12, 2023, 4:35 p.m. UTC | #3 
On Tue, Mar 07, 2023 at 02:09:33PM +0000, Dan Scally via libcamera-devel wrote:
> On 29/01/2023 13:58, Jacopo Mondi wrote:
> > The ISICameraConfiguration::validate() function selects which media
> > bus format to configure the sensor with based on the pixel format
> > of the first configured stream using the media bus code associated to it
> > in the formatsMap_ map.
> >
> > In order to remove the PixelFormamt-to-mbus-code association in
> > formatsMap_ provide a wrapper function for the newly introduced
> > getRawMediaBusFormat() and getYubMediaBusFormat() that automatically
> 
> s/getYubMediaBusFormat/getYuvMediaBusFormat here

With that fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> > selects what media bus format to use based on the first stream pixel
> > format.
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> > ---
> >   src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 16 +++++++++++++---
> >   1 file changed, 13 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> > index 445fad32656c..5976a63d27dd 100644
> > --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> > +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> > @@ -61,6 +61,7 @@ public:
> >   
> >   	unsigned int getRawMediaBusFormat(PixelFormat *pixelFormat) const;
> >   	unsigned int getYuvMediaBusFormat(PixelFormat *pixelFormat) const;
> > +	unsigned int getMediaBusFormat(PixelFormat *pixelFormat) const;
> >   
> >   	std::unique_ptr<CameraSensor> sensor_;
> >   	std::unique_ptr<V4L2Subdevice> csis_;
> > @@ -315,6 +316,15 @@ unsigned int ISICameraData::getYuvMediaBusFormat(PixelFormat *pixelFormat) const
> >   	return supportedCodes[0];
> >   }
> >   
> > +unsigned int ISICameraData::getMediaBusFormat(PixelFormat *pixelFormat) const
> > +{
> > +	if (PixelFormatInfo::info(*pixelFormat).colourEncoding ==
> > +	    PixelFormatInfo::ColourEncodingRAW)
> > +		return getRawMediaBusFormat(pixelFormat);
> > +
> > +	return getYuvMediaBusFormat(pixelFormat);
> > +}
> > +
> >   /* -----------------------------------------------------------------------------
> >    * Camera Configuration
> >    */
> > @@ -628,16 +638,16 @@ CameraConfiguration::Status ISICameraConfiguration::validate()
> >   	 * image quality in exchange of a usually slower frame rate.
> >   	 * Usage of the STILL_CAPTURE role could be consider for this.
> >   	 */
> > -	const PipeFormat &pipeFmt = formatsMap_.at(config_[0].pixelFormat);
> > -
> >   	Size maxSize;
> >   	for (const auto &cfg : config_) {
> >   		if (cfg.size > maxSize)
> >   			maxSize = cfg.size;
> >   	}
> >   
> > +	PixelFormat pixelFormat = config_[0].pixelFormat;
> > +
> >   	V4L2SubdeviceFormat sensorFormat{};
> > -	sensorFormat.mbus_code = pipeFmt.sensorCode;
> > +	sensorFormat.mbus_code = data_->getMediaBusFormat(&pixelFormat);
> >   	sensorFormat.size = maxSize;
> >   
> >   	LOG(ISI, Debug) << "Computed sensor configuration: " << sensorFormat;

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
index 445fad32656c..5976a63d27dd 100644
--- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
+++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
@@ -61,6 +61,7 @@  public:
 
 	unsigned int getRawMediaBusFormat(PixelFormat *pixelFormat) const;
 	unsigned int getYuvMediaBusFormat(PixelFormat *pixelFormat) const;
+	unsigned int getMediaBusFormat(PixelFormat *pixelFormat) const;
 
 	std::unique_ptr<CameraSensor> sensor_;
 	std::unique_ptr<V4L2Subdevice> csis_;
@@ -315,6 +316,15 @@  unsigned int ISICameraData::getYuvMediaBusFormat(PixelFormat *pixelFormat) const
 	return supportedCodes[0];
 }
 
+unsigned int ISICameraData::getMediaBusFormat(PixelFormat *pixelFormat) const
+{
+	if (PixelFormatInfo::info(*pixelFormat).colourEncoding ==
+	    PixelFormatInfo::ColourEncodingRAW)
+		return getRawMediaBusFormat(pixelFormat);
+
+	return getYuvMediaBusFormat(pixelFormat);
+}
+
 /* -----------------------------------------------------------------------------
  * Camera Configuration
  */
@@ -628,16 +638,16 @@  CameraConfiguration::Status ISICameraConfiguration::validate()
 	 * image quality in exchange of a usually slower frame rate.
 	 * Usage of the STILL_CAPTURE role could be consider for this.
 	 */
-	const PipeFormat &pipeFmt = formatsMap_.at(config_[0].pixelFormat);
-
 	Size maxSize;
 	for (const auto &cfg : config_) {
 		if (cfg.size > maxSize)
 			maxSize = cfg.size;
 	}
 
+	PixelFormat pixelFormat = config_[0].pixelFormat;
+
 	V4L2SubdeviceFormat sensorFormat{};
-	sensorFormat.mbus_code = pipeFmt.sensorCode;
+	sensorFormat.mbus_code = data_->getMediaBusFormat(&pixelFormat);
 	sensorFormat.size = maxSize;
 
 	LOG(ISI, Debug) << "Computed sensor configuration: " << sensorFormat;