Message ID | 20230129135830.27490-6-jacopo.mondi@ideasonboard.com |
---|---|
State | Accepted |
Headers | show |
Series |
|
Related | show |
On Sun, Jan 29, 2023 at 02:58:30PM +0100, Jacopo Mondi via libcamera-devel wrote: > Now that the media bus code selection procedure does not depend > on the ISICameraConfiguration::formatsMap_ remove the association > between PixelFormat supported by the ISI and the media bus code produced > by the sensor. > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Looks good to me. Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 159 ++++--------------- > 1 file changed, 29 insertions(+), 130 deletions(-) > > diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > index 0e1e87c7a2aa..f754f5d77f90 100644 > --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > @@ -76,18 +76,6 @@ public: > class ISICameraConfiguration : public CameraConfiguration > { > public: > - /* > - * formatsMap_ records the association between an output pixel format > - * and the combination of V4L2 pixel format and media bus codes that have > - * to be applied to the pipeline. > - */ > - struct PipeFormat { > - unsigned int isiCode; > - unsigned int sensorCode; > - }; > - > - using FormatMap = std::map<PixelFormat, PipeFormat>; > - > ISICameraConfiguration(ISICameraData *data) > : data_(data) > { > @@ -95,7 +83,7 @@ public: > > Status validate() override; > > - static const FormatMap formatsMap_; > + static const std::map<PixelFormat, unsigned int> formatsMap_; > > V4L2SubdeviceFormat sensorFormat_; > > @@ -333,121 +321,33 @@ unsigned int ISICameraData::getMediaBusFormat(PixelFormat *pixelFormat) const > * Camera Configuration > */ > > -/** > - * \todo Do not associate the sensor format to non-RAW pixelformats, as > - * the ISI can do colorspace conversion. > +/* > + * ISICameraConfiguration::formatsMap_ records the association between an output > + * pixel format and the ISI source pixel format to be applied to the pipeline. > */ > -const ISICameraConfiguration::FormatMap ISICameraConfiguration::formatsMap_ = { > - { > - formats::YUYV, > - { MEDIA_BUS_FMT_YUV8_1X24, > - MEDIA_BUS_FMT_UYVY8_1X16 }, > - }, > - { > - formats::AVUY8888, > - { MEDIA_BUS_FMT_YUV8_1X24, > - MEDIA_BUS_FMT_UYVY8_1X16 }, > - }, > - { > - formats::NV12, > - { MEDIA_BUS_FMT_YUV8_1X24, > - MEDIA_BUS_FMT_UYVY8_1X16 }, > - }, > - { > - formats::NV16, > - { MEDIA_BUS_FMT_YUV8_1X24, > - MEDIA_BUS_FMT_UYVY8_1X16 }, > - }, > - { > - formats::YUV444, > - { MEDIA_BUS_FMT_YUV8_1X24, > - MEDIA_BUS_FMT_UYVY8_1X16 }, > - }, > - { > - formats::RGB565, > - { MEDIA_BUS_FMT_RGB888_1X24, > - MEDIA_BUS_FMT_RGB565_1X16 }, > - }, > - { > - formats::BGR888, > - { MEDIA_BUS_FMT_RGB888_1X24, > - MEDIA_BUS_FMT_RGB565_1X16 }, > - }, > - { > - formats::RGB888, > - { MEDIA_BUS_FMT_RGB888_1X24, > - MEDIA_BUS_FMT_RGB565_1X16 }, > - }, > - { > - formats::XRGB8888, > - { MEDIA_BUS_FMT_RGB888_1X24, > - MEDIA_BUS_FMT_RGB565_1X16 }, > - }, > - { > - formats::ABGR8888, > - { MEDIA_BUS_FMT_RGB888_1X24, > - MEDIA_BUS_FMT_RGB565_1X16 }, > - }, > - { > - formats::SBGGR8, > - { MEDIA_BUS_FMT_SBGGR8_1X8, > - MEDIA_BUS_FMT_SBGGR8_1X8 }, > - }, > - { > - formats::SGBRG8, > - { MEDIA_BUS_FMT_SGBRG8_1X8, > - MEDIA_BUS_FMT_SGBRG8_1X8 }, > - }, > - { > - formats::SGRBG8, > - { MEDIA_BUS_FMT_SGRBG8_1X8, > - MEDIA_BUS_FMT_SGRBG8_1X8 }, > - }, > - { > - formats::SRGGB8, > - { MEDIA_BUS_FMT_SRGGB8_1X8, > - MEDIA_BUS_FMT_SRGGB8_1X8 }, > - }, > - { > - formats::SBGGR10, > - { MEDIA_BUS_FMT_SBGGR10_1X10, > - MEDIA_BUS_FMT_SBGGR10_1X10 }, > - }, > - { > - formats::SGBRG10, > - { MEDIA_BUS_FMT_SGBRG10_1X10, > - MEDIA_BUS_FMT_SGBRG10_1X10 }, > - }, > - { > - formats::SGRBG10, > - { MEDIA_BUS_FMT_SGRBG10_1X10, > - MEDIA_BUS_FMT_SGRBG10_1X10 }, > - }, > - { > - formats::SRGGB10, > - { MEDIA_BUS_FMT_SRGGB10_1X10, > - MEDIA_BUS_FMT_SRGGB10_1X10 }, > - }, > - { > - formats::SBGGR12, > - { MEDIA_BUS_FMT_SBGGR12_1X12, > - MEDIA_BUS_FMT_SBGGR12_1X12 }, > - }, > - { > - formats::SGBRG12, > - { MEDIA_BUS_FMT_SGBRG12_1X12, > - MEDIA_BUS_FMT_SGBRG12_1X12 }, > - }, > - { > - formats::SGRBG12, > - { MEDIA_BUS_FMT_SGRBG12_1X12, > - MEDIA_BUS_FMT_SGRBG12_1X12 }, > - }, > - { > - formats::SRGGB12, > - { MEDIA_BUS_FMT_SRGGB12_1X12, > - MEDIA_BUS_FMT_SRGGB12_1X12 }, > - }, > +const std::map<PixelFormat, unsigned int> ISICameraConfiguration::formatsMap_ = { > + { formats::YUYV, MEDIA_BUS_FMT_UYVY8_1X16 }, > + { formats::AVUY8888, MEDIA_BUS_FMT_UYVY8_1X16 }, > + { formats::NV12, MEDIA_BUS_FMT_UYVY8_1X16 }, > + { formats::NV16, MEDIA_BUS_FMT_UYVY8_1X16 }, > + { formats::YUV444, MEDIA_BUS_FMT_UYVY8_1X16 }, > + { formats::RGB565, MEDIA_BUS_FMT_RGB565_1X16 }, > + { formats::BGR888, MEDIA_BUS_FMT_RGB565_1X16 }, > + { formats::RGB888, MEDIA_BUS_FMT_RGB565_1X16 }, > + { formats::XRGB8888, MEDIA_BUS_FMT_RGB565_1X16 }, > + { formats::ABGR8888, MEDIA_BUS_FMT_RGB565_1X16 }, > + { formats::SBGGR8, MEDIA_BUS_FMT_SBGGR8_1X8 }, > + { formats::SGBRG8, MEDIA_BUS_FMT_SGBRG8_1X8 }, > + { formats::SGRBG8, MEDIA_BUS_FMT_SGRBG8_1X8 }, > + { formats::SRGGB8, MEDIA_BUS_FMT_SRGGB8_1X8 }, > + { formats::SBGGR10, MEDIA_BUS_FMT_SBGGR10_1X10 }, > + { formats::SGBRG10, MEDIA_BUS_FMT_SGBRG10_1X10 }, > + { formats::SGRBG10, MEDIA_BUS_FMT_SGRBG10_1X10 }, > + { formats::SRGGB10, MEDIA_BUS_FMT_SRGGB10_1X10 }, > + { formats::SBGGR12, MEDIA_BUS_FMT_SBGGR12_1X12 }, > + { formats::SGBRG12, MEDIA_BUS_FMT_SGBRG12_1X12 }, > + { formats::SGRBG12, MEDIA_BUS_FMT_SGRBG12_1X12 }, > + { formats::SRGGB12, MEDIA_BUS_FMT_SRGGB12_1X12 }, > }; > > /* > @@ -991,11 +891,10 @@ int PipelineHandlerISI::configure(Camera *camera, CameraConfiguration *c) > * size is taken from the sink's COMPOSE (or source's CROP, > * if any) rectangles. > */ > - const ISICameraConfiguration::PipeFormat &pipeFormat = > - ISICameraConfiguration::formatsMap_.at(config.pixelFormat); > + unsigned int isiCode = ISICameraConfiguration::formatsMap_.at(config.pixelFormat); > > V4L2SubdeviceFormat isiFormat{}; > - isiFormat.mbus_code = pipeFormat.isiCode; > + isiFormat.mbus_code = isiCode; > isiFormat.size = config.size; > > ret = pipe->isi->setFormat(1, &isiFormat); > -- > 2.39.0 >
Hi Jacopo, Thank you for the patch. On Sun, Jan 29, 2023 at 02:58:30PM +0100, Jacopo Mondi via libcamera-devel wrote: > Now that the media bus code selection procedure does not depend > on the ISICameraConfiguration::formatsMap_ remove the association > between PixelFormat supported by the ISI and the media bus code produced > by the sensor. > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > --- > src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 159 ++++--------------- > 1 file changed, 29 insertions(+), 130 deletions(-) > > diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > index 0e1e87c7a2aa..f754f5d77f90 100644 > --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > @@ -76,18 +76,6 @@ public: > class ISICameraConfiguration : public CameraConfiguration > { > public: > - /* > - * formatsMap_ records the association between an output pixel format > - * and the combination of V4L2 pixel format and media bus codes that have > - * to be applied to the pipeline. > - */ > - struct PipeFormat { > - unsigned int isiCode; > - unsigned int sensorCode; > - }; > - > - using FormatMap = std::map<PixelFormat, PipeFormat>; > - > ISICameraConfiguration(ISICameraData *data) > : data_(data) > { > @@ -95,7 +83,7 @@ public: > > Status validate() override; > > - static const FormatMap formatsMap_; > + static const std::map<PixelFormat, unsigned int> formatsMap_; > > V4L2SubdeviceFormat sensorFormat_; > > @@ -333,121 +321,33 @@ unsigned int ISICameraData::getMediaBusFormat(PixelFormat *pixelFormat) const > * Camera Configuration > */ > > -/** > - * \todo Do not associate the sensor format to non-RAW pixelformats, as > - * the ISI can do colorspace conversion. > +/* > + * ISICameraConfiguration::formatsMap_ records the association between an output > + * pixel format and the ISI source pixel format to be applied to the pipeline. > */ > -const ISICameraConfiguration::FormatMap ISICameraConfiguration::formatsMap_ = { > - { > - formats::YUYV, > - { MEDIA_BUS_FMT_YUV8_1X24, > - MEDIA_BUS_FMT_UYVY8_1X16 }, > - }, > - { > - formats::AVUY8888, > - { MEDIA_BUS_FMT_YUV8_1X24, > - MEDIA_BUS_FMT_UYVY8_1X16 }, > - }, > - { > - formats::NV12, > - { MEDIA_BUS_FMT_YUV8_1X24, > - MEDIA_BUS_FMT_UYVY8_1X16 }, > - }, > - { > - formats::NV16, > - { MEDIA_BUS_FMT_YUV8_1X24, > - MEDIA_BUS_FMT_UYVY8_1X16 }, > - }, > - { > - formats::YUV444, > - { MEDIA_BUS_FMT_YUV8_1X24, > - MEDIA_BUS_FMT_UYVY8_1X16 }, > - }, > - { > - formats::RGB565, > - { MEDIA_BUS_FMT_RGB888_1X24, > - MEDIA_BUS_FMT_RGB565_1X16 }, > - }, > - { > - formats::BGR888, > - { MEDIA_BUS_FMT_RGB888_1X24, > - MEDIA_BUS_FMT_RGB565_1X16 }, > - }, > - { > - formats::RGB888, > - { MEDIA_BUS_FMT_RGB888_1X24, > - MEDIA_BUS_FMT_RGB565_1X16 }, > - }, > - { > - formats::XRGB8888, > - { MEDIA_BUS_FMT_RGB888_1X24, > - MEDIA_BUS_FMT_RGB565_1X16 }, > - }, > - { > - formats::ABGR8888, > - { MEDIA_BUS_FMT_RGB888_1X24, > - MEDIA_BUS_FMT_RGB565_1X16 }, > - }, > - { > - formats::SBGGR8, > - { MEDIA_BUS_FMT_SBGGR8_1X8, > - MEDIA_BUS_FMT_SBGGR8_1X8 }, > - }, > - { > - formats::SGBRG8, > - { MEDIA_BUS_FMT_SGBRG8_1X8, > - MEDIA_BUS_FMT_SGBRG8_1X8 }, > - }, > - { > - formats::SGRBG8, > - { MEDIA_BUS_FMT_SGRBG8_1X8, > - MEDIA_BUS_FMT_SGRBG8_1X8 }, > - }, > - { > - formats::SRGGB8, > - { MEDIA_BUS_FMT_SRGGB8_1X8, > - MEDIA_BUS_FMT_SRGGB8_1X8 }, > - }, > - { > - formats::SBGGR10, > - { MEDIA_BUS_FMT_SBGGR10_1X10, > - MEDIA_BUS_FMT_SBGGR10_1X10 }, > - }, > - { > - formats::SGBRG10, > - { MEDIA_BUS_FMT_SGBRG10_1X10, > - MEDIA_BUS_FMT_SGBRG10_1X10 }, > - }, > - { > - formats::SGRBG10, > - { MEDIA_BUS_FMT_SGRBG10_1X10, > - MEDIA_BUS_FMT_SGRBG10_1X10 }, > - }, > - { > - formats::SRGGB10, > - { MEDIA_BUS_FMT_SRGGB10_1X10, > - MEDIA_BUS_FMT_SRGGB10_1X10 }, > - }, > - { > - formats::SBGGR12, > - { MEDIA_BUS_FMT_SBGGR12_1X12, > - MEDIA_BUS_FMT_SBGGR12_1X12 }, > - }, > - { > - formats::SGBRG12, > - { MEDIA_BUS_FMT_SGBRG12_1X12, > - MEDIA_BUS_FMT_SGBRG12_1X12 }, > - }, > - { > - formats::SGRBG12, > - { MEDIA_BUS_FMT_SGRBG12_1X12, > - MEDIA_BUS_FMT_SGRBG12_1X12 }, > - }, > - { > - formats::SRGGB12, > - { MEDIA_BUS_FMT_SRGGB12_1X12, > - MEDIA_BUS_FMT_SRGGB12_1X12 }, > - }, > +const std::map<PixelFormat, unsigned int> ISICameraConfiguration::formatsMap_ = { > + { formats::YUYV, MEDIA_BUS_FMT_UYVY8_1X16 }, > + { formats::AVUY8888, MEDIA_BUS_FMT_UYVY8_1X16 }, > + { formats::NV12, MEDIA_BUS_FMT_UYVY8_1X16 }, > + { formats::NV16, MEDIA_BUS_FMT_UYVY8_1X16 }, > + { formats::YUV444, MEDIA_BUS_FMT_UYVY8_1X16 }, > + { formats::RGB565, MEDIA_BUS_FMT_RGB565_1X16 }, > + { formats::BGR888, MEDIA_BUS_FMT_RGB565_1X16 }, > + { formats::RGB888, MEDIA_BUS_FMT_RGB565_1X16 }, > + { formats::XRGB8888, MEDIA_BUS_FMT_RGB565_1X16 }, > + { formats::ABGR8888, MEDIA_BUS_FMT_RGB565_1X16 }, > + { formats::SBGGR8, MEDIA_BUS_FMT_SBGGR8_1X8 }, > + { formats::SGBRG8, MEDIA_BUS_FMT_SGBRG8_1X8 }, > + { formats::SGRBG8, MEDIA_BUS_FMT_SGRBG8_1X8 }, > + { formats::SRGGB8, MEDIA_BUS_FMT_SRGGB8_1X8 }, > + { formats::SBGGR10, MEDIA_BUS_FMT_SBGGR10_1X10 }, > + { formats::SGBRG10, MEDIA_BUS_FMT_SGBRG10_1X10 }, > + { formats::SGRBG10, MEDIA_BUS_FMT_SGRBG10_1X10 }, > + { formats::SRGGB10, MEDIA_BUS_FMT_SRGGB10_1X10 }, > + { formats::SBGGR12, MEDIA_BUS_FMT_SBGGR12_1X12 }, > + { formats::SGBRG12, MEDIA_BUS_FMT_SGBRG12_1X12 }, > + { formats::SGRBG12, MEDIA_BUS_FMT_SGRBG12_1X12 }, > + { formats::SRGGB12, MEDIA_BUS_FMT_SRGGB12_1X12 }, > }; > > /* > @@ -991,11 +891,10 @@ int PipelineHandlerISI::configure(Camera *camera, CameraConfiguration *c) > * size is taken from the sink's COMPOSE (or source's CROP, > * if any) rectangles. > */ > - const ISICameraConfiguration::PipeFormat &pipeFormat = > - ISICameraConfiguration::formatsMap_.at(config.pixelFormat); > + unsigned int isiCode = ISICameraConfiguration::formatsMap_.at(config.pixelFormat); > > V4L2SubdeviceFormat isiFormat{}; > - isiFormat.mbus_code = pipeFormat.isiCode; > + isiFormat.mbus_code = isiCode; Unless I'm mistaken, you've kept the sensorCore, not the isiCode, in the formatsMap_. > isiFormat.size = config.size; > > ret = pipe->isi->setFormat(1, &isiFormat);
Hi Laurent On Sun, Mar 12, 2023 at 06:51:56PM +0200, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patch. > > On Sun, Jan 29, 2023 at 02:58:30PM +0100, Jacopo Mondi via libcamera-devel wrote: > > Now that the media bus code selection procedure does not depend > > on the ISICameraConfiguration::formatsMap_ remove the association > > between PixelFormat supported by the ISI and the media bus code produced > > by the sensor. > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > --- > > src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 159 ++++--------------- > > 1 file changed, 29 insertions(+), 130 deletions(-) > > > > diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > > index 0e1e87c7a2aa..f754f5d77f90 100644 > > --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > > +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > > @@ -76,18 +76,6 @@ public: > > class ISICameraConfiguration : public CameraConfiguration > > { > > public: > > - /* > > - * formatsMap_ records the association between an output pixel format > > - * and the combination of V4L2 pixel format and media bus codes that have > > - * to be applied to the pipeline. > > - */ > > - struct PipeFormat { > > - unsigned int isiCode; > > - unsigned int sensorCode; > > - }; > > - > > - using FormatMap = std::map<PixelFormat, PipeFormat>; > > - > > ISICameraConfiguration(ISICameraData *data) > > : data_(data) > > { > > @@ -95,7 +83,7 @@ public: > > > > Status validate() override; > > > > - static const FormatMap formatsMap_; > > + static const std::map<PixelFormat, unsigned int> formatsMap_; > > > > V4L2SubdeviceFormat sensorFormat_; > > > > @@ -333,121 +321,33 @@ unsigned int ISICameraData::getMediaBusFormat(PixelFormat *pixelFormat) const > > * Camera Configuration > > */ > > > > -/** > > - * \todo Do not associate the sensor format to non-RAW pixelformats, as > > - * the ISI can do colorspace conversion. > > +/* > > + * ISICameraConfiguration::formatsMap_ records the association between an output > > + * pixel format and the ISI source pixel format to be applied to the pipeline. > > */ > > -const ISICameraConfiguration::FormatMap ISICameraConfiguration::formatsMap_ = { > > - { > > - formats::YUYV, > > - { MEDIA_BUS_FMT_YUV8_1X24, > > - MEDIA_BUS_FMT_UYVY8_1X16 }, > > - }, > > - { > > - formats::AVUY8888, > > - { MEDIA_BUS_FMT_YUV8_1X24, > > - MEDIA_BUS_FMT_UYVY8_1X16 }, > > - }, > > - { > > - formats::NV12, > > - { MEDIA_BUS_FMT_YUV8_1X24, > > - MEDIA_BUS_FMT_UYVY8_1X16 }, > > - }, > > - { > > - formats::NV16, > > - { MEDIA_BUS_FMT_YUV8_1X24, > > - MEDIA_BUS_FMT_UYVY8_1X16 }, > > - }, > > - { > > - formats::YUV444, > > - { MEDIA_BUS_FMT_YUV8_1X24, > > - MEDIA_BUS_FMT_UYVY8_1X16 }, > > - }, > > - { > > - formats::RGB565, > > - { MEDIA_BUS_FMT_RGB888_1X24, > > - MEDIA_BUS_FMT_RGB565_1X16 }, > > - }, > > - { > > - formats::BGR888, > > - { MEDIA_BUS_FMT_RGB888_1X24, > > - MEDIA_BUS_FMT_RGB565_1X16 }, > > - }, > > - { > > - formats::RGB888, > > - { MEDIA_BUS_FMT_RGB888_1X24, > > - MEDIA_BUS_FMT_RGB565_1X16 }, > > - }, > > - { > > - formats::XRGB8888, > > - { MEDIA_BUS_FMT_RGB888_1X24, > > - MEDIA_BUS_FMT_RGB565_1X16 }, > > - }, > > - { > > - formats::ABGR8888, > > - { MEDIA_BUS_FMT_RGB888_1X24, > > - MEDIA_BUS_FMT_RGB565_1X16 }, > > - }, > > - { > > - formats::SBGGR8, > > - { MEDIA_BUS_FMT_SBGGR8_1X8, > > - MEDIA_BUS_FMT_SBGGR8_1X8 }, > > - }, > > - { > > - formats::SGBRG8, > > - { MEDIA_BUS_FMT_SGBRG8_1X8, > > - MEDIA_BUS_FMT_SGBRG8_1X8 }, > > - }, > > - { > > - formats::SGRBG8, > > - { MEDIA_BUS_FMT_SGRBG8_1X8, > > - MEDIA_BUS_FMT_SGRBG8_1X8 }, > > - }, > > - { > > - formats::SRGGB8, > > - { MEDIA_BUS_FMT_SRGGB8_1X8, > > - MEDIA_BUS_FMT_SRGGB8_1X8 }, > > - }, > > - { > > - formats::SBGGR10, > > - { MEDIA_BUS_FMT_SBGGR10_1X10, > > - MEDIA_BUS_FMT_SBGGR10_1X10 }, > > - }, > > - { > > - formats::SGBRG10, > > - { MEDIA_BUS_FMT_SGBRG10_1X10, > > - MEDIA_BUS_FMT_SGBRG10_1X10 }, > > - }, > > - { > > - formats::SGRBG10, > > - { MEDIA_BUS_FMT_SGRBG10_1X10, > > - MEDIA_BUS_FMT_SGRBG10_1X10 }, > > - }, > > - { > > - formats::SRGGB10, > > - { MEDIA_BUS_FMT_SRGGB10_1X10, > > - MEDIA_BUS_FMT_SRGGB10_1X10 }, > > - }, > > - { > > - formats::SBGGR12, > > - { MEDIA_BUS_FMT_SBGGR12_1X12, > > - MEDIA_BUS_FMT_SBGGR12_1X12 }, > > - }, > > - { > > - formats::SGBRG12, > > - { MEDIA_BUS_FMT_SGBRG12_1X12, > > - MEDIA_BUS_FMT_SGBRG12_1X12 }, > > - }, > > - { > > - formats::SGRBG12, > > - { MEDIA_BUS_FMT_SGRBG12_1X12, > > - MEDIA_BUS_FMT_SGRBG12_1X12 }, > > - }, > > - { > > - formats::SRGGB12, > > - { MEDIA_BUS_FMT_SRGGB12_1X12, > > - MEDIA_BUS_FMT_SRGGB12_1X12 }, > > - }, > > +const std::map<PixelFormat, unsigned int> ISICameraConfiguration::formatsMap_ = { > > + { formats::YUYV, MEDIA_BUS_FMT_UYVY8_1X16 }, > > + { formats::AVUY8888, MEDIA_BUS_FMT_UYVY8_1X16 }, > > + { formats::NV12, MEDIA_BUS_FMT_UYVY8_1X16 }, > > + { formats::NV16, MEDIA_BUS_FMT_UYVY8_1X16 }, > > + { formats::YUV444, MEDIA_BUS_FMT_UYVY8_1X16 }, > > + { formats::RGB565, MEDIA_BUS_FMT_RGB565_1X16 }, > > + { formats::BGR888, MEDIA_BUS_FMT_RGB565_1X16 }, > > + { formats::RGB888, MEDIA_BUS_FMT_RGB565_1X16 }, > > + { formats::XRGB8888, MEDIA_BUS_FMT_RGB565_1X16 }, > > + { formats::ABGR8888, MEDIA_BUS_FMT_RGB565_1X16 }, > > + { formats::SBGGR8, MEDIA_BUS_FMT_SBGGR8_1X8 }, > > + { formats::SGBRG8, MEDIA_BUS_FMT_SGBRG8_1X8 }, > > + { formats::SGRBG8, MEDIA_BUS_FMT_SGRBG8_1X8 }, > > + { formats::SRGGB8, MEDIA_BUS_FMT_SRGGB8_1X8 }, > > + { formats::SBGGR10, MEDIA_BUS_FMT_SBGGR10_1X10 }, > > + { formats::SGBRG10, MEDIA_BUS_FMT_SGBRG10_1X10 }, > > + { formats::SGRBG10, MEDIA_BUS_FMT_SGRBG10_1X10 }, > > + { formats::SRGGB10, MEDIA_BUS_FMT_SRGGB10_1X10 }, > > + { formats::SBGGR12, MEDIA_BUS_FMT_SBGGR12_1X12 }, > > + { formats::SGBRG12, MEDIA_BUS_FMT_SGBRG12_1X12 }, > > + { formats::SGRBG12, MEDIA_BUS_FMT_SGRBG12_1X12 }, > > + { formats::SRGGB12, MEDIA_BUS_FMT_SRGGB12_1X12 }, > > }; > > > > /* > > @@ -991,11 +891,10 @@ int PipelineHandlerISI::configure(Camera *camera, CameraConfiguration *c) > > * size is taken from the sink's COMPOSE (or source's CROP, > > * if any) rectangles. > > */ > > - const ISICameraConfiguration::PipeFormat &pipeFormat = > > - ISICameraConfiguration::formatsMap_.at(config.pixelFormat); > > + unsigned int isiCode = ISICameraConfiguration::formatsMap_.at(config.pixelFormat); > > > > V4L2SubdeviceFormat isiFormat{}; > > - isiFormat.mbus_code = pipeFormat.isiCode; > > + isiFormat.mbus_code = isiCode; > > Unless I'm mistaken, you've kept the sensorCore, not the isiCode, in the > formatsMap_. > Mmmm, not really, the formatsMap_ collects the association between an output pixelformat and the ISI source pad format. The sensor format is dynamically computed by ISICameraData::getMediaBusFormat() and propagated from the sensor to the ISI sink pad during configure(). Does this answer your question ? > > isiFormat.size = config.size; > > > > ret = pipe->isi->setFormat(1, &isiFormat); > > -- > Regards, > > Laurent Pinchart
Hi Jacopo, On Mon, Mar 13, 2023 at 09:59:09AM +0100, Jacopo Mondi wrote: > On Sun, Mar 12, 2023 at 06:51:56PM +0200, Laurent Pinchart wrote: > > On Sun, Jan 29, 2023 at 02:58:30PM +0100, Jacopo Mondi via libcamera-devel wrote: > > > Now that the media bus code selection procedure does not depend > > > on the ISICameraConfiguration::formatsMap_ remove the association > > > between PixelFormat supported by the ISI and the media bus code produced > > > by the sensor. > > > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > --- > > > src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 159 ++++--------------- > > > 1 file changed, 29 insertions(+), 130 deletions(-) > > > > > > diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > > > index 0e1e87c7a2aa..f754f5d77f90 100644 > > > --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > > > +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp > > > @@ -76,18 +76,6 @@ public: > > > class ISICameraConfiguration : public CameraConfiguration > > > { > > > public: > > > - /* > > > - * formatsMap_ records the association between an output pixel format > > > - * and the combination of V4L2 pixel format and media bus codes that have > > > - * to be applied to the pipeline. > > > - */ > > > - struct PipeFormat { > > > - unsigned int isiCode; > > > - unsigned int sensorCode; > > > - }; > > > - > > > - using FormatMap = std::map<PixelFormat, PipeFormat>; > > > - > > > ISICameraConfiguration(ISICameraData *data) > > > : data_(data) > > > { > > > @@ -95,7 +83,7 @@ public: > > > > > > Status validate() override; > > > > > > - static const FormatMap formatsMap_; > > > + static const std::map<PixelFormat, unsigned int> formatsMap_; > > > > > > V4L2SubdeviceFormat sensorFormat_; > > > > > > @@ -333,121 +321,33 @@ unsigned int ISICameraData::getMediaBusFormat(PixelFormat *pixelFormat) const > > > * Camera Configuration > > > */ > > > > > > -/** > > > - * \todo Do not associate the sensor format to non-RAW pixelformats, as > > > - * the ISI can do colorspace conversion. > > > +/* > > > + * ISICameraConfiguration::formatsMap_ records the association between an output > > > + * pixel format and the ISI source pixel format to be applied to the pipeline. > > > */ > > > -const ISICameraConfiguration::FormatMap ISICameraConfiguration::formatsMap_ = { > > > - { > > > - formats::YUYV, > > > - { MEDIA_BUS_FMT_YUV8_1X24, > > > - MEDIA_BUS_FMT_UYVY8_1X16 }, > > > - }, > > > - { > > > - formats::AVUY8888, > > > - { MEDIA_BUS_FMT_YUV8_1X24, > > > - MEDIA_BUS_FMT_UYVY8_1X16 }, > > > - }, > > > - { > > > - formats::NV12, > > > - { MEDIA_BUS_FMT_YUV8_1X24, > > > - MEDIA_BUS_FMT_UYVY8_1X16 }, > > > - }, > > > - { > > > - formats::NV16, > > > - { MEDIA_BUS_FMT_YUV8_1X24, > > > - MEDIA_BUS_FMT_UYVY8_1X16 }, > > > - }, > > > - { > > > - formats::YUV444, > > > - { MEDIA_BUS_FMT_YUV8_1X24, > > > - MEDIA_BUS_FMT_UYVY8_1X16 }, > > > - }, > > > - { > > > - formats::RGB565, > > > - { MEDIA_BUS_FMT_RGB888_1X24, > > > - MEDIA_BUS_FMT_RGB565_1X16 }, > > > - }, > > > - { > > > - formats::BGR888, > > > - { MEDIA_BUS_FMT_RGB888_1X24, > > > - MEDIA_BUS_FMT_RGB565_1X16 }, > > > - }, > > > - { > > > - formats::RGB888, > > > - { MEDIA_BUS_FMT_RGB888_1X24, > > > - MEDIA_BUS_FMT_RGB565_1X16 }, > > > - }, > > > - { > > > - formats::XRGB8888, > > > - { MEDIA_BUS_FMT_RGB888_1X24, > > > - MEDIA_BUS_FMT_RGB565_1X16 }, > > > - }, > > > - { > > > - formats::ABGR8888, > > > - { MEDIA_BUS_FMT_RGB888_1X24, > > > - MEDIA_BUS_FMT_RGB565_1X16 }, > > > - }, > > > - { > > > - formats::SBGGR8, > > > - { MEDIA_BUS_FMT_SBGGR8_1X8, > > > - MEDIA_BUS_FMT_SBGGR8_1X8 }, > > > - }, > > > - { > > > - formats::SGBRG8, > > > - { MEDIA_BUS_FMT_SGBRG8_1X8, > > > - MEDIA_BUS_FMT_SGBRG8_1X8 }, > > > - }, > > > - { > > > - formats::SGRBG8, > > > - { MEDIA_BUS_FMT_SGRBG8_1X8, > > > - MEDIA_BUS_FMT_SGRBG8_1X8 }, > > > - }, > > > - { > > > - formats::SRGGB8, > > > - { MEDIA_BUS_FMT_SRGGB8_1X8, > > > - MEDIA_BUS_FMT_SRGGB8_1X8 }, > > > - }, > > > - { > > > - formats::SBGGR10, > > > - { MEDIA_BUS_FMT_SBGGR10_1X10, > > > - MEDIA_BUS_FMT_SBGGR10_1X10 }, > > > - }, > > > - { > > > - formats::SGBRG10, > > > - { MEDIA_BUS_FMT_SGBRG10_1X10, > > > - MEDIA_BUS_FMT_SGBRG10_1X10 }, > > > - }, > > > - { > > > - formats::SGRBG10, > > > - { MEDIA_BUS_FMT_SGRBG10_1X10, > > > - MEDIA_BUS_FMT_SGRBG10_1X10 }, > > > - }, > > > - { > > > - formats::SRGGB10, > > > - { MEDIA_BUS_FMT_SRGGB10_1X10, > > > - MEDIA_BUS_FMT_SRGGB10_1X10 }, > > > - }, > > > - { > > > - formats::SBGGR12, > > > - { MEDIA_BUS_FMT_SBGGR12_1X12, > > > - MEDIA_BUS_FMT_SBGGR12_1X12 }, > > > - }, > > > - { > > > - formats::SGBRG12, > > > - { MEDIA_BUS_FMT_SGBRG12_1X12, > > > - MEDIA_BUS_FMT_SGBRG12_1X12 }, > > > - }, > > > - { > > > - formats::SGRBG12, > > > - { MEDIA_BUS_FMT_SGRBG12_1X12, > > > - MEDIA_BUS_FMT_SGRBG12_1X12 }, > > > - }, > > > - { > > > - formats::SRGGB12, > > > - { MEDIA_BUS_FMT_SRGGB12_1X12, > > > - MEDIA_BUS_FMT_SRGGB12_1X12 }, > > > - }, > > > +const std::map<PixelFormat, unsigned int> ISICameraConfiguration::formatsMap_ = { > > > + { formats::YUYV, MEDIA_BUS_FMT_UYVY8_1X16 }, > > > + { formats::AVUY8888, MEDIA_BUS_FMT_UYVY8_1X16 }, > > > + { formats::NV12, MEDIA_BUS_FMT_UYVY8_1X16 }, > > > + { formats::NV16, MEDIA_BUS_FMT_UYVY8_1X16 }, > > > + { formats::YUV444, MEDIA_BUS_FMT_UYVY8_1X16 }, > > > + { formats::RGB565, MEDIA_BUS_FMT_RGB565_1X16 }, > > > + { formats::BGR888, MEDIA_BUS_FMT_RGB565_1X16 }, > > > + { formats::RGB888, MEDIA_BUS_FMT_RGB565_1X16 }, > > > + { formats::XRGB8888, MEDIA_BUS_FMT_RGB565_1X16 }, > > > + { formats::ABGR8888, MEDIA_BUS_FMT_RGB565_1X16 }, > > > + { formats::SBGGR8, MEDIA_BUS_FMT_SBGGR8_1X8 }, > > > + { formats::SGBRG8, MEDIA_BUS_FMT_SGBRG8_1X8 }, > > > + { formats::SGRBG8, MEDIA_BUS_FMT_SGRBG8_1X8 }, > > > + { formats::SRGGB8, MEDIA_BUS_FMT_SRGGB8_1X8 }, > > > + { formats::SBGGR10, MEDIA_BUS_FMT_SBGGR10_1X10 }, > > > + { formats::SGBRG10, MEDIA_BUS_FMT_SGBRG10_1X10 }, > > > + { formats::SGRBG10, MEDIA_BUS_FMT_SGRBG10_1X10 }, > > > + { formats::SRGGB10, MEDIA_BUS_FMT_SRGGB10_1X10 }, > > > + { formats::SBGGR12, MEDIA_BUS_FMT_SBGGR12_1X12 }, > > > + { formats::SGBRG12, MEDIA_BUS_FMT_SGBRG12_1X12 }, > > > + { formats::SGRBG12, MEDIA_BUS_FMT_SGRBG12_1X12 }, > > > + { formats::SRGGB12, MEDIA_BUS_FMT_SRGGB12_1X12 }, > > > }; > > > > > > /* > > > @@ -991,11 +891,10 @@ int PipelineHandlerISI::configure(Camera *camera, CameraConfiguration *c) > > > * size is taken from the sink's COMPOSE (or source's CROP, > > > * if any) rectangles. > > > */ > > > - const ISICameraConfiguration::PipeFormat &pipeFormat = > > > - ISICameraConfiguration::formatsMap_.at(config.pixelFormat); > > > + unsigned int isiCode = ISICameraConfiguration::formatsMap_.at(config.pixelFormat); > > > > > > V4L2SubdeviceFormat isiFormat{}; > > > - isiFormat.mbus_code = pipeFormat.isiCode; > > > + isiFormat.mbus_code = isiCode; > > > > Unless I'm mistaken, you've kept the sensorCore, not the isiCode, in the > > formatsMap_. > > Mmmm, not really, the formatsMap_ collects the association between an > output pixelformat and the ISI source pad format. > > The sensor format is dynamically computed by > ISICameraData::getMediaBusFormat() and propagated from the sensor to the > ISI sink pad during configure(). > > Does this answer your question ? Looking at the first entry of the map, you have - formats::YUYV, - { MEDIA_BUS_FMT_YUV8_1X24, - MEDIA_BUS_FMT_UYVY8_1X16 }, + { formats::YUYV, MEDIA_BUS_FMT_UYVY8_1X16 }, and the map used to store a struct PipeFormat { unsigned int isiCode; unsigned int sensorCode; }; You have thus kept the sensorCode value, not the isiCode value, while you're modifying the code that uses to map as follows: + unsigned int isiCode = ISICameraConfiguration::formatsMap_.at(config.pixelFormat); V4L2SubdeviceFormat isiFormat{}; - isiFormat.mbus_code = pipeFormat.isiCode; + isiFormat.mbus_code = isiCode; I would thus have expected the map to now store { formats::YUYV, MEDIA_BUS_FMT_YUV8_1X24 }, > > > isiFormat.size = config.size; > > > > > > ret = pipe->isi->setFormat(1, &isiFormat);
diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp index 0e1e87c7a2aa..f754f5d77f90 100644 --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp @@ -76,18 +76,6 @@ public: class ISICameraConfiguration : public CameraConfiguration { public: - /* - * formatsMap_ records the association between an output pixel format - * and the combination of V4L2 pixel format and media bus codes that have - * to be applied to the pipeline. - */ - struct PipeFormat { - unsigned int isiCode; - unsigned int sensorCode; - }; - - using FormatMap = std::map<PixelFormat, PipeFormat>; - ISICameraConfiguration(ISICameraData *data) : data_(data) { @@ -95,7 +83,7 @@ public: Status validate() override; - static const FormatMap formatsMap_; + static const std::map<PixelFormat, unsigned int> formatsMap_; V4L2SubdeviceFormat sensorFormat_; @@ -333,121 +321,33 @@ unsigned int ISICameraData::getMediaBusFormat(PixelFormat *pixelFormat) const * Camera Configuration */ -/** - * \todo Do not associate the sensor format to non-RAW pixelformats, as - * the ISI can do colorspace conversion. +/* + * ISICameraConfiguration::formatsMap_ records the association between an output + * pixel format and the ISI source pixel format to be applied to the pipeline. */ -const ISICameraConfiguration::FormatMap ISICameraConfiguration::formatsMap_ = { - { - formats::YUYV, - { MEDIA_BUS_FMT_YUV8_1X24, - MEDIA_BUS_FMT_UYVY8_1X16 }, - }, - { - formats::AVUY8888, - { MEDIA_BUS_FMT_YUV8_1X24, - MEDIA_BUS_FMT_UYVY8_1X16 }, - }, - { - formats::NV12, - { MEDIA_BUS_FMT_YUV8_1X24, - MEDIA_BUS_FMT_UYVY8_1X16 }, - }, - { - formats::NV16, - { MEDIA_BUS_FMT_YUV8_1X24, - MEDIA_BUS_FMT_UYVY8_1X16 }, - }, - { - formats::YUV444, - { MEDIA_BUS_FMT_YUV8_1X24, - MEDIA_BUS_FMT_UYVY8_1X16 }, - }, - { - formats::RGB565, - { MEDIA_BUS_FMT_RGB888_1X24, - MEDIA_BUS_FMT_RGB565_1X16 }, - }, - { - formats::BGR888, - { MEDIA_BUS_FMT_RGB888_1X24, - MEDIA_BUS_FMT_RGB565_1X16 }, - }, - { - formats::RGB888, - { MEDIA_BUS_FMT_RGB888_1X24, - MEDIA_BUS_FMT_RGB565_1X16 }, - }, - { - formats::XRGB8888, - { MEDIA_BUS_FMT_RGB888_1X24, - MEDIA_BUS_FMT_RGB565_1X16 }, - }, - { - formats::ABGR8888, - { MEDIA_BUS_FMT_RGB888_1X24, - MEDIA_BUS_FMT_RGB565_1X16 }, - }, - { - formats::SBGGR8, - { MEDIA_BUS_FMT_SBGGR8_1X8, - MEDIA_BUS_FMT_SBGGR8_1X8 }, - }, - { - formats::SGBRG8, - { MEDIA_BUS_FMT_SGBRG8_1X8, - MEDIA_BUS_FMT_SGBRG8_1X8 }, - }, - { - formats::SGRBG8, - { MEDIA_BUS_FMT_SGRBG8_1X8, - MEDIA_BUS_FMT_SGRBG8_1X8 }, - }, - { - formats::SRGGB8, - { MEDIA_BUS_FMT_SRGGB8_1X8, - MEDIA_BUS_FMT_SRGGB8_1X8 }, - }, - { - formats::SBGGR10, - { MEDIA_BUS_FMT_SBGGR10_1X10, - MEDIA_BUS_FMT_SBGGR10_1X10 }, - }, - { - formats::SGBRG10, - { MEDIA_BUS_FMT_SGBRG10_1X10, - MEDIA_BUS_FMT_SGBRG10_1X10 }, - }, - { - formats::SGRBG10, - { MEDIA_BUS_FMT_SGRBG10_1X10, - MEDIA_BUS_FMT_SGRBG10_1X10 }, - }, - { - formats::SRGGB10, - { MEDIA_BUS_FMT_SRGGB10_1X10, - MEDIA_BUS_FMT_SRGGB10_1X10 }, - }, - { - formats::SBGGR12, - { MEDIA_BUS_FMT_SBGGR12_1X12, - MEDIA_BUS_FMT_SBGGR12_1X12 }, - }, - { - formats::SGBRG12, - { MEDIA_BUS_FMT_SGBRG12_1X12, - MEDIA_BUS_FMT_SGBRG12_1X12 }, - }, - { - formats::SGRBG12, - { MEDIA_BUS_FMT_SGRBG12_1X12, - MEDIA_BUS_FMT_SGRBG12_1X12 }, - }, - { - formats::SRGGB12, - { MEDIA_BUS_FMT_SRGGB12_1X12, - MEDIA_BUS_FMT_SRGGB12_1X12 }, - }, +const std::map<PixelFormat, unsigned int> ISICameraConfiguration::formatsMap_ = { + { formats::YUYV, MEDIA_BUS_FMT_UYVY8_1X16 }, + { formats::AVUY8888, MEDIA_BUS_FMT_UYVY8_1X16 }, + { formats::NV12, MEDIA_BUS_FMT_UYVY8_1X16 }, + { formats::NV16, MEDIA_BUS_FMT_UYVY8_1X16 }, + { formats::YUV444, MEDIA_BUS_FMT_UYVY8_1X16 }, + { formats::RGB565, MEDIA_BUS_FMT_RGB565_1X16 }, + { formats::BGR888, MEDIA_BUS_FMT_RGB565_1X16 }, + { formats::RGB888, MEDIA_BUS_FMT_RGB565_1X16 }, + { formats::XRGB8888, MEDIA_BUS_FMT_RGB565_1X16 }, + { formats::ABGR8888, MEDIA_BUS_FMT_RGB565_1X16 }, + { formats::SBGGR8, MEDIA_BUS_FMT_SBGGR8_1X8 }, + { formats::SGBRG8, MEDIA_BUS_FMT_SGBRG8_1X8 }, + { formats::SGRBG8, MEDIA_BUS_FMT_SGRBG8_1X8 }, + { formats::SRGGB8, MEDIA_BUS_FMT_SRGGB8_1X8 }, + { formats::SBGGR10, MEDIA_BUS_FMT_SBGGR10_1X10 }, + { formats::SGBRG10, MEDIA_BUS_FMT_SGBRG10_1X10 }, + { formats::SGRBG10, MEDIA_BUS_FMT_SGRBG10_1X10 }, + { formats::SRGGB10, MEDIA_BUS_FMT_SRGGB10_1X10 }, + { formats::SBGGR12, MEDIA_BUS_FMT_SBGGR12_1X12 }, + { formats::SGBRG12, MEDIA_BUS_FMT_SGBRG12_1X12 }, + { formats::SGRBG12, MEDIA_BUS_FMT_SGRBG12_1X12 }, + { formats::SRGGB12, MEDIA_BUS_FMT_SRGGB12_1X12 }, }; /* @@ -991,11 +891,10 @@ int PipelineHandlerISI::configure(Camera *camera, CameraConfiguration *c) * size is taken from the sink's COMPOSE (or source's CROP, * if any) rectangles. */ - const ISICameraConfiguration::PipeFormat &pipeFormat = - ISICameraConfiguration::formatsMap_.at(config.pixelFormat); + unsigned int isiCode = ISICameraConfiguration::formatsMap_.at(config.pixelFormat); V4L2SubdeviceFormat isiFormat{}; - isiFormat.mbus_code = pipeFormat.isiCode; + isiFormat.mbus_code = isiCode; isiFormat.size = config.size; ret = pipe->isi->setFormat(1, &isiFormat);
Now that the media bus code selection procedure does not depend on the ISICameraConfiguration::formatsMap_ remove the association between PixelFormat supported by the ISI and the media bus code produced by the sensor. Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> --- src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 159 ++++--------------- 1 file changed, 29 insertions(+), 130 deletions(-)