| Message ID | 20230129135830.27490-5-jacopo.mondi@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
On Sun, Jan 29, 2023 at 02:58:29PM +0100, Jacopo Mondi via libcamera-devel 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(). imo CameraSensor::tryFormat() should be split into a separate patch, but it's not that big of a deal. > > Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Jacopo Mondi <jacopo.mondi@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? Other than that, looks good to me. Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > + { 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); > } > > - 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); > } > > -- > 2.39.0 >
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@ideasonboard.com> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > Reviewed-by: Paul Elder <paul.elder@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. > > - 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); > } >
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@ideasonboard.com> > Signed-off-by: Jacopo Mondi <jacopo.mondi@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); > } >
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@ideasonboard.com> > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > Reviewed-by: Paul Elder <paul.elder@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); > > }
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); } - 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); }
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@ideasonboard.com> Signed-off-by: Jacopo Mondi <jacopo.mondi@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(-)