[libcamera-devel] [PATCH 1/5] libcamera: imx8-isi: Break-out RAW format selection
Jacopo Mondi
jacopo.mondi at ideasonboard.com
Thu Mar 2 13:47:22 CET 2023
Hi Paul
On Thu, Mar 02, 2023 at 08:34:47PM +0900, Paul Elder via libcamera-devel wrote:
> On Sun, Jan 29, 2023 at 02:58:26PM +0100, Jacopo Mondi via libcamera-devel wrote:
> > The current implementation of the ISI pipeline handler handles
> > translation of PixelFormat to media bus formats from the sensor
> > through a centralized map.
> >
> > As the criteria to select the correct media bus code depend if the
> > output PixelFormat is a RAW Bayer format or not, start by splitting
> > the RAW media bus code procedure selection out by adding a method
> > for such purpose to the ISICameraData class.
> >
> > Add the function to the ISICameraData and not to the
> > ISICameraConfiguration because:
> > - The sensor is a property of CameraData
> > - The same function will be re-used by the ISIPipelineHandler
> > during CameraConfiguration generation.
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
>
> In the title, s/Break-out/Break out/
>
> > ---
> > src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 130 +++++++++++++------
> > 1 file changed, 90 insertions(+), 40 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> > index 0c67e35dde73..72bc310d80ec 100644
> > --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> > +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> > @@ -59,6 +59,8 @@ public:
> > return stream - &*streams_.begin();
> > }
> >
> > + unsigned int getRawMediaBusFormat(PixelFormat *pixelFormat) const;
> > +
> > std::unique_ptr<CameraSensor> sensor_;
> > std::unique_ptr<V4L2Subdevice> csis_;
> >
> > @@ -174,6 +176,85 @@ int ISICameraData::init()
> > return 0;
> > }
> >
> > +/*
> > + * Get a RAW Bayer media bus format compatible with the requested pixelFormat.
> > + *
> > + * If the requested pixelFormat cannot be produced by the sensor adjust it to
> > + * the one corresponding to the media bus format with the largest bit-depth.
> > + */
> > +unsigned int ISICameraData::getRawMediaBusFormat(PixelFormat *pixelFormat) const
>
> I'm wondering why you decided to use a pointer instead of a reference. I
pixelFormat is an output parameter and by convention we use references
for const input parameters and pointers for parameters that can be
modified afaik
> suppose since we control all the callers it's fine either way, so just
> wondering.
>
> Otherwise, looks good to me.
>
> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
Thanks
j
>
> > +{
> > + std::vector<unsigned int> mbusCodes = sensor_->mbusCodes();
> > +
> > + const std::map<PixelFormat, unsigned int> rawFormats = {
> > + { 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 },
> > + };
> > +
> > + /*
> > + * Make sure the requested PixelFormat is supported in the above
> > + * map and the sensor can produce the compatible mbus code.
> > + */
> > + auto it = rawFormats.find(*pixelFormat);
> > + if (it != rawFormats.end() &&
> > + std::count(mbusCodes.begin(), mbusCodes.end(), it->second))
> > + return it->second;
> > +
> > + if (it == rawFormats.end())
> > + LOG(ISI, Warning) << pixelFormat
> > + << " not supported in ISI formats map.";
> > +
> > + /*
> > + * The desired pixel format cannot be produced. Adjust it to the one
> > + * corresponding to the raw media bus format with the largest bit-depth
> > + * the sensor provides.
> > + */
> > + unsigned int sensorCode = 0;
> > + unsigned int maxDepth = 0;
> > + *pixelFormat = {};
> > +
> > + for (unsigned int code : mbusCodes) {
> > + /* Make sure the media bus format is RAW Bayer. */
> > + const BayerFormat &bayerFormat = BayerFormat::fromMbusCode(code);
> > + if (!bayerFormat.isValid())
> > + continue;
> > +
> > + /* Make sure the media format is supported. */
> > + it = std::find_if(rawFormats.begin(), rawFormats.end(),
> > + [code](auto &rawFormat) {
> > + return rawFormat.second == code;
> > + });
> > +
> > + if (it == rawFormats.end()) {
> > + LOG(ISI, Warning) << bayerFormat
> > + << " not supported in ISI formats map.";
> > + continue;
> > + }
> > +
> > + /* Pick the one with the largest bit depth. */
> > + if (bayerFormat.bitDepth > maxDepth) {
> > + maxDepth = bayerFormat.bitDepth;
> > + *pixelFormat = it->first;
> > + sensorCode = code;
> > + }
> > + }
> > +
> > + if (!pixelFormat->isValid())
> > + LOG(ISI, Error) << "Cannot find a supported RAW format";
> > +
> > + return sensorCode;
> > +}
> > +
> > /* -----------------------------------------------------------------------------
> > * Camera Configuration
> > */
> > @@ -311,50 +392,19 @@ ISICameraConfiguration::validateRaw(std::set<Stream *> &availableStreams,
> > */
> > std::vector<unsigned int> mbusCodes = data_->sensor_->mbusCodes();
> > StreamConfiguration &rawConfig = config_[0];
> > + PixelFormat rawFormat = rawConfig.pixelFormat;
> >
> > - bool supported = false;
> > - auto it = formatsMap_.find(rawConfig.pixelFormat);
> > - if (it != formatsMap_.end()) {
> > - unsigned int mbusCode = it->second.sensorCode;
> > -
> > - if (std::count(mbusCodes.begin(), mbusCodes.end(), mbusCode))
> > - supported = true;
> > + unsigned int sensorCode = data_->getRawMediaBusFormat(&rawFormat);
> > + if (!sensorCode) {
> > + LOG(ISI, Error) << "Cannot adjust RAW pixelformat "
> > + << rawConfig.pixelFormat;
> > + return Invalid;
> > }
> >
> > - if (!supported) {
> > - /*
> > - * Adjust to the first mbus code supported by both the
> > - * sensor and the pipeline.
> > - */
> > - const FormatMap::value_type *pipeConfig = nullptr;
> > - for (unsigned int code : mbusCodes) {
> > - const BayerFormat &bayerFormat = BayerFormat::fromMbusCode(code);
> > - if (!bayerFormat.isValid())
> > - continue;
> > -
> > - auto fmt = std::find_if(ISICameraConfiguration::formatsMap_.begin(),
> > - ISICameraConfiguration::formatsMap_.end(),
> > - [code](const auto &isiFormat) {
> > - const auto &pipe = isiFormat.second;
> > - return pipe.sensorCode == code;
> > - });
> > -
> > - if (fmt == ISICameraConfiguration::formatsMap_.end())
> > - continue;
> > -
> > - pipeConfig = &(*fmt);
> > - break;
> > - }
> > -
> > - if (!pipeConfig) {
> > - LOG(ISI, Error) << "Cannot adjust RAW format "
> > - << rawConfig.pixelFormat;
> > - return Invalid;
> > - }
> > -
> > - rawConfig.pixelFormat = pipeConfig->first;
> > + if (rawFormat != rawConfig.pixelFormat) {
> > LOG(ISI, Debug) << "RAW pixelformat adjusted to "
> > - << pipeConfig->first;
> > + << rawFormat;
> > + rawConfig.pixelFormat = rawFormat;
> > status = Adjusted;
> > }
> >
> > --
> > 2.39.0
> >
More information about the libcamera-devel
mailing list