[libcamera-devel] [PATCH 4/5] libcamera: imx8-isi: Split Bayer/YUV config generation
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sun Mar 12 17:50:44 CET 2023
Hi Jacopo,
Thank you for the patch.
On Sun, Jan 29, 2023 at 02:58:29PM +0100, 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().
I agree with Paul, please split the new function to a separate patch.
> Reported-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo.mondi 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 = {
static const
> + { 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() } };
The ISI can't scale raw formats, so I don't think kMinISISize is right
here.
> + }
> +
> + 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);
> }
>
> - 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);
> }
>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list