[libcamera-devel] [PATCH 4/5] libcamera: imx8-isi: Split Bayer/YUV config generation
Jacopo Mondi
jacopo.mondi at ideasonboard.com
Mon Mar 13 09:51:17 CET 2023
Hi Dan
On Tue, Mar 07, 2023 at 02:14:31PM +0000, Dan Scally via libcamera-devel wrote:
> Hi Jacopo
>
> On 29/01/2023 13:58, Jacopo Mondi wrote:
> > At generateConfiguration() a YUV/RGB pixel format is preferred for the
> > StillCapture/VideoRecording/Viewfinder roles, but currently there are no
> > guarantees in place that the sensor provides a non-Bayer bus format from
> > which YUV/RGB can be generated.
> >
> > This makes the default configuration generated for those roles not to
> > work if the sensor is a RAW-only one.
> >
> > To improve the situation split the configuration generation in two,
> > one for YUV modes and one for Raw Bayer mode.
> >
> > StreamRoles assigned to a YUV mode will try to first generate a YUV
> > configuration and then fallback to RAW if that's what the sensor can
> > provide.
> >
> > As an additional requirement, for YUV streams, the generated mode has to
> > be validated with the sensor to confirm the desired sizes can be
> > generated. In order to test a format on the sensor introduce
> > CameraSensor::tryFormat().
> >
> > Reported-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > Signed-off-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> > Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> > ---
> > include/libcamera/internal/camera_sensor.h | 1 +
> > src/libcamera/camera_sensor.cpp | 14 ++
> > src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 231 +++++++++++++------
> > 3 files changed, 170 insertions(+), 76 deletions(-)
> >
> > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> > index b9f4d7867854..ce3a790f00ee 100644
> > --- a/include/libcamera/internal/camera_sensor.h
> > +++ b/include/libcamera/internal/camera_sensor.h
> > @@ -54,6 +54,7 @@ public:
> > V4L2SubdeviceFormat getFormat(const std::vector<unsigned int> &mbusCodes,
> > const Size &size) const;
> > int setFormat(V4L2SubdeviceFormat *format);
> > + int tryFormat(V4L2SubdeviceFormat *format) const;
> > const ControlInfoMap &controls() const;
> > ControlList getControls(const std::vector<uint32_t> &ids);
> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> > index a210aa4fa370..dfe593033def 100644
> > --- a/src/libcamera/camera_sensor.cpp
> > +++ b/src/libcamera/camera_sensor.cpp
> > @@ -757,6 +757,20 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat *format)
> > return 0;
> > }
> > +/**
> > + * \brief Try the sensor output format
> > + * \param[in] format The desired sensor output format
> > + *
> > + * The ranges of any controls associated with the sensor are not updated.
> > + *
> > + * \return 0 on success or a negative error code otherwise
> > + */
> > +int CameraSensor::tryFormat(V4L2SubdeviceFormat *format) const
> > +{
> > + return subdev_->setFormat(pad_, format,
> > + V4L2Subdevice::Whence::TryFormat);
> > +}
> > +
> > /**
> > * \brief Retrieve the supported V4L2 controls and their information
> > *
> > diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> > index 5976a63d27dd..0e1e87c7a2aa 100644
> > --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> > +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp
> > @@ -145,6 +145,10 @@ private:
> > Pipe *pipeFromStream(Camera *camera, const Stream *stream);
> > + StreamConfiguration generateYUVConfiguration(Camera *camera,
> > + const Size &size);
> > + StreamConfiguration generateRawConfiguration(Camera *camera);
> > +
> > void bufferReady(FrameBuffer *buffer);
> > MediaDevice *isiDev_;
> > @@ -705,6 +709,129 @@ PipelineHandlerISI::PipelineHandlerISI(CameraManager *manager)
> > {
> > }
> > +/*
> > + * Generate a StreamConfiguration for YUV/RGB use case.
> > + *
> > + * Verify it the sensor can produce a YUV/RGB media bus format and collect
> > + * all the processed pixel formats the ISI can generate as supported stream
> > + * configurations.
> > + */
> > +StreamConfiguration PipelineHandlerISI::generateYUVConfiguration(Camera *camera,
> > + const Size &size)
> > +{
> > + ISICameraData *data = cameraData(camera);
> > + PixelFormat pixelFormat = formats::YUYV;
> > + unsigned int mbusCode;
> > +
> > + mbusCode = data->getYuvMediaBusFormat(&pixelFormat);
> > + if (!mbusCode)
> > + return {};
> > +
> > + /* Adjust the requested size to the sensor's capabilities. */
> > + const CameraSensor *sensor = data->sensor_.get();
> > +
> > + V4L2SubdeviceFormat sensorFmt;
> > + sensorFmt.mbus_code = mbusCode;
> > + sensorFmt.size = size;
> > +
> > + int ret = sensor->tryFormat(&sensorFmt);
> > + if (ret) {
> > + LOG(ISI, Error) << "Failed to try sensor format.";
> > + return {};
> > + }
> > +
> > + Size sensorSize = sensorFmt.size;
> > +
> > + /*
> > + * Populate the StreamConfiguration.
> > + *
> > + * As the sensor supports at least one YUV/RGB media bus format all the
> > + * processed ones in formatsMap_ can be generated from it.
> > + */
> > + std::map<PixelFormat, std::vector<SizeRange>> streamFormats;
> > +
> > + for (const auto &[pixFmt, pipeFmt] : ISICameraConfiguration::formatsMap_) {
> > + const PixelFormatInfo &info = PixelFormatInfo::info(pixFmt);
> > + if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)
> > + continue;
> > +
> > + streamFormats[pixFmt] = { { kMinISISize, sensorSize } };
> > + }
> > +
> > + StreamFormats formats(streamFormats);
> > +
> > + StreamConfiguration cfg(formats);
> > + cfg.pixelFormat = pixelFormat;
> > + cfg.size = sensorSize;
> > + cfg.bufferCount = 4;
> > +
> > + return cfg;
> > +}
> > +
> > +/*
> > + * Generate a StreamConfiguration for Raw Bayer use case. Verify if the sensor
> > + * can produce the requested RAW bayer format and eventually adjust it to
> > + * the one with the largest bit-depth the sensor can produce.
> > + */
> > +StreamConfiguration PipelineHandlerISI::generateRawConfiguration(Camera *camera)
> > +{
> > + const std::map<unsigned int, PixelFormat> rawFormats = {
> > + { MEDIA_BUS_FMT_SBGGR8_1X8, formats::SBGGR8 },
> > + { MEDIA_BUS_FMT_SGBRG8_1X8, formats::SGBRG8 },
> > + { MEDIA_BUS_FMT_SGRBG8_1X8, formats::SGRBG8 },
> > + { MEDIA_BUS_FMT_SRGGB8_1X8, formats::SRGGB8 },
> > + { MEDIA_BUS_FMT_SBGGR10_1X10, formats::SBGGR10 },
> > + { MEDIA_BUS_FMT_SGBRG10_1X10, formats::SGBRG10 },
> > + { MEDIA_BUS_FMT_SGRBG10_1X10, formats::SGRBG10 },
> > + { MEDIA_BUS_FMT_SRGGB10_1X10, formats::SRGGB10 },
> > + { MEDIA_BUS_FMT_SBGGR12_1X12, formats::SBGGR12 },
> > + { MEDIA_BUS_FMT_SGBRG12_1X12, formats::SGBRG12 },
> > + { MEDIA_BUS_FMT_SGRBG12_1X12, formats::SGRBG12 },
> > + { MEDIA_BUS_FMT_SRGGB12_1X12, formats::SRGGB12 },
> > + };
> > +
> > + ISICameraData *data = cameraData(camera);
> > + PixelFormat pixelFormat = formats::SBGGR10;
> > + unsigned int mbusCode;
> > +
> > + /* pixelFormat will be adjusted, if the sensor can produce RAW. */
> > + mbusCode = data->getRawMediaBusFormat(&pixelFormat);
> > + if (!mbusCode)
> > + return {};
> > +
> > + /*
> > + * Populate the StreamConfiguration with all the supported Bayer
> > + * formats the sensor can produce.
> > + */
> > + std::map<PixelFormat, std::vector<SizeRange>> streamFormats;
> > + const CameraSensor *sensor = data->sensor_.get();
> > +
> > + for (unsigned int code : sensor->mbusCodes()) {
> > + /* Find a Bayer media bus code from the sensor. */
> > + const BayerFormat &bayerFormat = BayerFormat::fromMbusCode(code);
> > + if (!bayerFormat.isValid())
> > + continue;
> > +
> > + auto it = rawFormats.find(code);
> > + if (it == rawFormats.end()) {
> > + LOG(ISI, Warning) << bayerFormat
> > + << " not supported in ISI formats map.";
> > + continue;
> > + }
> > +
> > + streamFormats[it->second] = { { kMinISISize, sensor->resolution() } };
> > + }
> > +
> > + StreamFormats formats(streamFormats);
> > +
> > + StreamConfiguration cfg(formats);
> > + cfg.size = sensor->resolution();
> > + cfg.pixelFormat = pixelFormat;
> > + cfg.bufferCount = 4;
> > +
> > + return cfg;
> > +}
> > +
> > std::unique_ptr<CameraConfiguration>
> > PipelineHandlerISI::generateConfiguration(Camera *camera,
> > const StreamRoles &roles)
> > @@ -722,79 +849,45 @@ PipelineHandlerISI::generateConfiguration(Camera *camera,
> > return nullptr;
> > }
> > - bool isRaw = false;
> > for (const auto &role : roles) {
> > /*
> > - * Prefer the following formats
> > + * Prefer the following formats:
> > * - Still Capture: Full resolution YUYV
> > * - ViewFinder/VideoRecording: 1080p YUYV
> > - * - RAW: sensor's native format and resolution
> > + * - RAW: Full resolution Bayer
> > */
> > - std::map<PixelFormat, std::vector<SizeRange>> streamFormats;
> > - PixelFormat pixelFormat;
> > - Size size;
> > + StreamConfiguration cfg;
> > switch (role) {
> > case StreamRole::StillCapture:
> > - /*
> > - * \todo Make sure the sensor can produce non-RAW formats
> > - * compatible with the ones supported by the pipeline.
> > - */
> > - size = data->sensor_->resolution();
> > - pixelFormat = formats::YUYV;
> > + cfg = generateYUVConfiguration(camera,
> > + data->sensor_->resolution());
> > + if (!cfg.pixelFormat.isValid()) {
> > + /*
> > + * Fallback to use a Bayer format if that's what
> > + * the sensor supports.
> > + */
> > + cfg = generateRawConfiguration(camera);
> > + }
> > +
> > break;
> > case StreamRole::Viewfinder:
> > case StreamRole::VideoRecording:
> > - /*
> > - * \todo Make sure the sensor can produce non-RAW formats
> > - * compatible with the ones supported by the pipeline.
> > - */
> > - size = PipelineHandlerISI::kPreviewSize;
> > - pixelFormat = formats::YUYV;
> > - break;
> > -
> > - case StreamRole::Raw: {
> > - /*
> > - * Make sure the sensor can generate a RAW format and
> > - * prefer the ones with a larger bitdepth.
> > - */
> > - const ISICameraConfiguration::FormatMap::value_type *rawPipeFormat = nullptr;
> > - unsigned int maxDepth = 0;
> > -
> > - for (unsigned int code : data->sensor_->mbusCodes()) {
> > - const BayerFormat &bayerFormat = BayerFormat::fromMbusCode(code);
> > - if (!bayerFormat.isValid())
> > - continue;
> > -
> > - /* Make sure the format is supported by the pipeline handler. */
> > - auto it = std::find_if(ISICameraConfiguration::formatsMap_.begin(),
> > - ISICameraConfiguration::formatsMap_.end(),
> > - [code](auto &isiFormat) {
> > - auto &pipe = isiFormat.second;
> > - return pipe.sensorCode == code;
> > - });
> > - if (it == ISICameraConfiguration::formatsMap_.end())
> > - continue;
> > -
> > - if (bayerFormat.bitDepth > maxDepth) {
> > - maxDepth = bayerFormat.bitDepth;
> > - rawPipeFormat = &(*it);
> > - }
> > - }
> > -
> > - if (!rawPipeFormat) {
> > - LOG(ISI, Error)
> > - << "Cannot generate a configuration for RAW stream";
> > - return nullptr;
> > + cfg = generateYUVConfiguration(camera,
> > + PipelineHandlerISI::kPreviewSize);
> > + if (!cfg.pixelFormat.isValid()) {
> > + /*
> > + * Fallback to use a Bayer format if that's what
> > + * the sensor supports.
> > + */
> > + cfg = generateRawConfiguration(camera);
> > }
>
>
> Not a big deal at all so feel free to ignore me, but I think I'd have kept
> the resolution the same and just changed the pixel format - it threw me
> briefly in testing that it wasn't configuring 1080p as I expected in
> ViewFinder role.
>
Thing is that when switching to RAW formats we can only output what
the sensor produces and we always generate a raw configuration with
its full resolution. We could try to find a size closer to 1080p
produced by the sensor, but as there are no guarantees one exists, I
think it is more predictable to use full resolution unconditionally.
> > - size = data->sensor_->resolution();
> > - pixelFormat = rawPipeFormat->first;
> > -
> > - streamFormats[pixelFormat] = { { kMinISISize, size } };
> > - isRaw = true;
> > + break;
> > + case StreamRole::Raw: {
> > + cfg = generateRawConfiguration(camera);
> > break;
> > }
> > @@ -803,26 +896,12 @@ PipelineHandlerISI::generateConfiguration(Camera *camera,
> > return nullptr;
> > }
> > - /*
> > - * For non-RAW configurations the ISI can perform colorspace
> > - * conversion. List all the supported output formats here.
> > - */
> > - if (!isRaw) {
> > - for (const auto &[pixFmt, pipeFmt] : ISICameraConfiguration::formatsMap_) {
> > - const PixelFormatInfo &info = PixelFormatInfo::info(pixFmt);
> > - if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)
> > - continue;
> > -
> > - streamFormats[pixFmt] = { { kMinISISize, size } };
> > - }
> > + if (!cfg.pixelFormat.isValid()) {
> > + LOG(ISI, Error)
> > + << "Cannot generate configuration for role: " << role;
> > + return nullptr;
> > }
> > - StreamFormats formats(streamFormats);
> > -
> > - StreamConfiguration cfg(formats);
> > - cfg.pixelFormat = pixelFormat;
> > - cfg.size = size;
> > - cfg.bufferCount = 4;
> > config->addConfiguration(cfg);
> > }
More information about the libcamera-devel
mailing list