[libcamera-devel] [PATCH 5/5] libcamera: imx8-isi: Remove mbusCode from formatsMap_
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Mar 13 11:28:37 CET 2023
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 at 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);
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list