[libcamera-devel] [PATCH v3 13/13] pipeline: rkisp1: Support raw Bayer capture configuration
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sun Oct 30 18:32:13 CET 2022
Hi Jacopo,
On Wed, Oct 26, 2022 at 07:31:44PM +0200, Jacopo Mondi wrote:
> On Mon, Oct 24, 2022 at 03:03:56AM +0300, Laurent Pinchart via libcamera-devel wrote:
> > From: Florian Sylvestre <fsylvestre at baylibre.com>
> >
> > Implement support for raw Bayer capture during configuration generation,
> > validation and camera configuration.
> >
> > While at it, fix a typo in a comment.
> >
> > Signed-off-by: Florian Sylvestre <fsylvestre at baylibre.com>
> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > Changes since v2:
> >
> > - Nearly complete rewrite
> > ---
> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 116 +++++++---
> > src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 209 +++++++++++++++---
> > src/libcamera/pipeline/rkisp1/rkisp1_path.h | 3 +-
> > 3 files changed, 273 insertions(+), 55 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index e57411544f7a..891fd2d50270 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -403,6 +403,30 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta
> > pipe()->tryCompleteRequest(info);
> > }
> >
> > +/* -----------------------------------------------------------------------------
> > + * Camera Configuration
> > + */
> > +
> > +namespace {
> > +
> > +/* Keep in sync with the supported raw formats in rkisp1_path.cpp. */
> > +const std::map<PixelFormat, uint32_t> 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 },
> > +};
> > +
> > +} /* namespace */
> > +
> > RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera,
> > RkISP1CameraData *data)
> > : CameraConfiguration()
> > @@ -449,6 +473,21 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
> > status = Adjusted;
> > }
> >
> > + /*
> > + * Simultaneous capture of raw and processed streams isn't possible. If
> > + * there is any raw stream, cap the number of streams to one.
> > + */
> > + if (config_.size() > 1) {
> > + for (const auto &cfg : config_) {
> > + if (PixelFormatInfo::info(cfg.pixelFormat).colourEncoding ==
> > + PixelFormatInfo::ColourEncodingRAW) {
> > + config_.resize(1);
> > + status = Adjusted;
> > + }
> > + break;
>
> Does this belong to the previous if() ?
It does, thanks for spotting it.
> > + }
> > + }
> > +
> > /*
> > * If there are more than one stream in the configuration figure out the
> > * order to evaluate the streams. The first stream has the highest
> > @@ -510,45 +549,51 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
> > }
> > }
> >
> > - /* All paths rejected configuraiton. */
> > + /* All paths rejected configuration. */
> > LOG(RkISP1, Debug) << "Camera configuration not supported "
> > << cfg.toString();
> > return Invalid;
> > }
> >
> > /* Select the sensor format. */
> > + PixelFormat rawFormat;
> > Size maxSize;
> > - for (const StreamConfiguration &cfg : config_)
> > +
> > + for (const StreamConfiguration &cfg : config_) {
> > + const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);
> > + if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)
> > + rawFormat = cfg.pixelFormat;
> > +
> > maxSize = std::max(maxSize, cfg.size);
> > + }
> > +
> > + std::vector<unsigned int> mbusCodes;
> > +
> > + if (rawFormat.isValid()) {
> > + mbusCodes = { rawFormats.at(rawFormat) };
> > + } else {
> > + std::transform(rawFormats.begin(), rawFormats.end(),
> > + std::back_inserter(mbusCodes),
> > + [](const auto &value) { return value.second; });
> > + }
> > +
> > + sensorFormat_ = sensor->getFormat(mbusCodes, maxSize);
> >
> > - sensorFormat_ = sensor->getFormat({ MEDIA_BUS_FMT_SBGGR12_1X12,
> > - MEDIA_BUS_FMT_SGBRG12_1X12,
> > - MEDIA_BUS_FMT_SGRBG12_1X12,
> > - MEDIA_BUS_FMT_SRGGB12_1X12,
> > - MEDIA_BUS_FMT_SBGGR10_1X10,
> > - MEDIA_BUS_FMT_SGBRG10_1X10,
> > - MEDIA_BUS_FMT_SGRBG10_1X10,
> > - MEDIA_BUS_FMT_SRGGB10_1X10,
> > - MEDIA_BUS_FMT_SBGGR8_1X8,
> > - MEDIA_BUS_FMT_SGBRG8_1X8,
> > - MEDIA_BUS_FMT_SGRBG8_1X8,
> > - MEDIA_BUS_FMT_SRGGB8_1X8 },
> > - maxSize);
> > if (sensorFormat_.size.isNull())
> > sensorFormat_.size = sensor->resolution();
> >
> > return status;
> > }
> >
> > +/* -----------------------------------------------------------------------------
> > + * Pipeline Operations
> > + */
> > +
> > PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)
> > : PipelineHandler(manager), hasSelfPath_(true)
> > {
> > }
> >
> > -/* -----------------------------------------------------------------------------
> > - * Pipeline Operations
> > - */
> > -
> > std::unique_ptr<CameraConfiguration>
> > PipelineHandlerRkISP1::generateConfiguration(Camera *camera,
> > const StreamRoles &roles)
> > @@ -604,23 +649,38 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera,
> > colorSpace = ColorSpace::Rec709;
> > break;
> >
> > + case StreamRole::Raw:
> > + if (roles.size() > 1) {
> > + LOG(RkISP1, Error)
> > + << "Can't capture both raw and processed streams";
> > + return nullptr;
> > + }
> > +
> > + useMainPath = true;
> > + colorSpace = ColorSpace::Raw;
> > + break;
> > +
> > default:
> > LOG(RkISP1, Warning)
> > << "Requested stream role not supported: " << role;
> > return nullptr;
> > }
> >
> > - StreamConfiguration cfg;
> > + RkISP1Path *path;
> > +
> > if (useMainPath) {
> > - cfg = data->mainPath_->generateConfiguration(
> > - data->sensor_.get());
> > + path = data->mainPath_;
> > mainPathAvailable = false;
> > } else {
> > - cfg = data->selfPath_->generateConfiguration(
> > - data->sensor_.get());
> > + path = data->selfPath_;
> > selfPathAvailable = false;
> > }
> >
> > + StreamConfiguration cfg =
> > + path->generateConfiguration(data->sensor_.get(), role);
> > + if (!cfg.pixelFormat.isValid())
> > + return nullptr;
> > +
> > cfg.colorSpace = colorSpace;
> > config->addConfiguration(cfg);
> > }
> > @@ -674,10 +734,14 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> > << "ISP input pad configured with " << format
> > << " crop " << rect;
> >
> > - isRaw_ = false;
> > + const PixelFormat &streamFormat = config->at(0).pixelFormat;
> > + const PixelFormatInfo &info = PixelFormatInfo::info(streamFormat);
> > + isRaw_ = info.colourEncoding == PixelFormatInfo::ColourEncodingRAW;
> >
> > /* YUYV8_2X8 is required on the ISP source path pad for YUV output. */
> > - format.mbus_code = MEDIA_BUS_FMT_YUYV8_2X8;
> > + if (!isRaw_)
> > + format.mbus_code = MEDIA_BUS_FMT_YUYV8_2X8;
> > +
> > LOG(RkISP1, Debug)
> > << "Configuring ISP output pad with " << format
> > << " crop " << rect;
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> > index cc2ac66e6939..2994bd665ebb 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> > @@ -21,6 +21,39 @@ namespace libcamera {
> >
> > LOG_DECLARE_CATEGORY(RkISP1)
> >
> > +namespace {
> > +
> > +/* Keep in sync with the supported raw formats in rkisp1.cpp. */
> > +const std::map<PixelFormat, uint32_t> formatToMediaBus = {
> > + { formats::UYVY, MEDIA_BUS_FMT_YUYV8_2X8 },
> > + { formats::YUYV, MEDIA_BUS_FMT_YUYV8_2X8 },
> > + { formats::NV12, MEDIA_BUS_FMT_YUYV8_1_5X8 },
> > + { formats::NV21, MEDIA_BUS_FMT_YUYV8_1_5X8 },
> > + { formats::NV16, MEDIA_BUS_FMT_YUYV8_2X8 },
> > + { formats::NV61, MEDIA_BUS_FMT_YUYV8_2X8 },
> > + { formats::YUV420, MEDIA_BUS_FMT_YUYV8_1_5X8 },
> > + { formats::YVU420, MEDIA_BUS_FMT_YUYV8_1_5X8 },
> > + { formats::YUV422, MEDIA_BUS_FMT_YUYV8_2X8 },
> > + { formats::YVU422, MEDIA_BUS_FMT_YUYV8_2X8 },
> > + { formats::R8, MEDIA_BUS_FMT_YUYV8_2X8 },
> > + { formats::RGB565, MEDIA_BUS_FMT_YUYV8_2X8 },
> > + { formats::XRGB8888, MEDIA_BUS_FMT_YUYV8_2X8 },
> > + { 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 },
> > +};
> > +
> > +} /* namespace */
> > +
> > RkISP1Path::RkISP1Path(const char *name, const Span<const PixelFormat> &formats,
> > const Size &minResolution, const Size &maxResolution)
> > : name_(name), running_(false), formats_(formats),
> > @@ -69,11 +102,18 @@ void RkISP1Path::populateFormats()
> > std::vector<PixelFormat> formats;
> > for (const auto &[format, sizes] : v4l2Formats) {
> > const PixelFormat pixelFormat = format.toPixelFormat();
> > - const PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);
> >
> > - /* \todo Add support for RAW formats. */
> > - if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)
> > + /*
> > + * As a defensive measure, skip any pixel format exposed by the
> > + * driver that we don't know about. This ensures that looking up
> > + * formats in formatToMediaBus using a key from streamFormats_
> > + * will never fail in any of the other functions.
> > + */
> > + if (!formatToMediaBus.count(pixelFormat)) {
> > + LOG(RkISP1, Warning)
> > + << "Unsupported pixel format " << pixelFormat;
> > continue;
> > + }
> >
> > streamFormats_.insert(pixelFormat);
> >
> > @@ -86,21 +126,68 @@ void RkISP1Path::populateFormats()
> > }
> > }
> >
> > -StreamConfiguration RkISP1Path::generateConfiguration(const CameraSensor *sensor)
> > +StreamConfiguration
> > +RkISP1Path::generateConfiguration(const CameraSensor *sensor, StreamRole role)
> > {
> > + const std::vector<unsigned int> &mbusCodes = sensor->mbusCodes();
> > const Size &resolution = sensor->resolution();
> >
> > Size maxResolution = maxResolution_.boundedToAspectRatio(resolution)
> > .boundedTo(resolution);
> > Size minResolution = minResolution_.expandedToAspectRatio(resolution);
> >
> > + /* Create the list of supported stream formats. */
> > std::map<PixelFormat, std::vector<SizeRange>> streamFormats;
> > - for (const auto &format : streamFormats_)
> > - streamFormats[format] = { { minResolution, maxResolution } };
> > + unsigned int rawBitsPerPixel = 0;
> > + PixelFormat rawFormat;
> > +
> > + for (const auto &format : streamFormats_) {
> > + const PixelFormatInfo &info = PixelFormatInfo::info(format);
> > +
> > + if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {
> > + /* Skip raw formats not supported by the sensor. */
> > + uint32_t mbusCode = formatToMediaBus.at(format);
> > + if (std::find(mbusCodes.begin(), mbusCodes.end(), mbusCode) ==
> > + mbusCodes.end())
> > + continue;
> > +
> > + streamFormats[format] = { { resolution, resolution } };
>
> Can't the ISP crop with RAW formats ? I guess not..
Not that I know of, it's completely bypassed.
> > +
> > + /*
> > + * Store the raw format with the higher bits per pixel
> > + * for later usage.
> > + */
> > + if (info.bitsPerPixel > rawBitsPerPixel) {
> > + rawBitsPerPixel = info.bitsPerPixel;
> > + rawFormat = format;
> > + }
> > + } else {
> > + streamFormats[format] = { { minResolution, maxResolution } };
> > + }
>
> You could save one indentation level by
>
> if (info.colourEncoding != PixelFormatInfo::ColourEncodingRAW) {
> streamFormats[format] = { { minResolution, maxResolution } };
> continue;
> }
OK.
> > + }
> > +
> > + /*
> > + * Pick a suitable pixel format for the role. Raw streams need to use a
> > + * raw format, processed streams use NV12 by default.
> > + */
> > + PixelFormat format;
> > +
> > + if (role == StreamRole::Raw) {
> > + if (!rawFormat.isValid()) {
> > + LOG(RkISP1, Error)
> > + << "Sensor " << sensor->model()
> > + << " doesn't support raw capture";
> > + return {};
> > + }
> > +
> > + format = rawFormat;
> > + } else {
> > + format = formats::NV12;
> > + }
> >
> > StreamFormats formats(streamFormats);
> > StreamConfiguration cfg(formats);
> > - cfg.pixelFormat = formats::NV12;
> > + cfg.pixelFormat = format;
> > cfg.size = maxResolution;
> > cfg.bufferCount = RKISP1_BUFFER_COUNT;
> >
> > @@ -110,26 +197,85 @@ StreamConfiguration RkISP1Path::generateConfiguration(const CameraSensor *sensor
> > CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor,
> > StreamConfiguration *cfg)
> > {
> > + const std::vector<unsigned int> &mbusCodes = sensor->mbusCodes();
> > const Size &resolution = sensor->resolution();
> >
> > const StreamConfiguration reqCfg = *cfg;
> > CameraConfiguration::Status status = CameraConfiguration::Valid;
> >
> > /*
> > - * Default to NV12 if the requested format is not supported. All
> > - * versions of the ISP are guaranteed to support NV12 on both the main
> > - * and self paths.
> > + * Validate the pixel format. If the requested format isn't supported,
> > + * default to either NV12 (all versions of the ISP are guaranteed to
> > + * support NV12 on both the main and self paths) if the requested format
> > + * is not a raw format, or to the supported raw format with the highest
> > + * bits per pixel otherwise.
> > */
> > - if (!streamFormats_.count(cfg->pixelFormat))
> > - cfg->pixelFormat = formats::NV12;
> > + unsigned int rawBitsPerPixel = 0;
> > + PixelFormat rawFormat;
> > + bool found = false;
> > +
> > + for (const auto &format : streamFormats_) {
> > + const PixelFormatInfo &info = PixelFormatInfo::info(format);
> > +
> > + if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {
> > + /* Skip raw formats not supported by the sensor. */
> > + uint32_t mbusCode = formatToMediaBus.at(format);
> > + if (std::find(mbusCodes.begin(), mbusCodes.end(), mbusCode) ==
> > + mbusCodes.end())
> > + continue;
> > +
> > + /*
> > + * Store the raw format with the higher bits per pixel
> > + * for later usage.
> > + */
> > + if (info.bitsPerPixel > rawBitsPerPixel) {
> > + rawBitsPerPixel = info.bitsPerPixel;
> > + rawFormat = format;
> > + }
> > + }
> > +
> > + if (cfg->pixelFormat == format) {
> > + found = true;
> > + break;
> > + }
> > + }
> > +
> > + bool isRaw = PixelFormatInfo::info(cfg->pixelFormat).colourEncoding ==
> > + PixelFormatInfo::ColourEncodingRAW;
> >
> > /*
> > - * Adjust the size based on the sensor resolution and absolute limits
> > - * of the ISP.
> > + * If no raw format supported by the sensor has been found, use a
> > + * processed format.
> > */
> > - Size maxResolution = maxResolution_.boundedToAspectRatio(resolution)
> > - .boundedTo(resolution);
> > - Size minResolution = minResolution_.expandedToAspectRatio(resolution);
> > + if (!rawFormat.isValid())
> > + isRaw = false;
> > +
> > + if (!found)
> > + cfg->pixelFormat = isRaw ? rawFormat : formats::NV12;
> > +
> > + Size minResolution;
> > + Size maxResolution;
> > +
> > + if (isRaw) {
> > + /*
> > + * Use the sensor output size closest to the requested stream
> > + * size.
> > + */
> > + uint32_t mbusCode = formatToMediaBus.at(cfg->pixelFormat);
> > + V4L2SubdeviceFormat sensorFormat =
> > + sensor->getFormat({ mbusCode }, cfg->size);
> > +
> > + minResolution = sensorFormat.size;
> > + maxResolution = sensorFormat.size;
> > + } else {
> > + /*
> > + * Adjust the size based on the sensor resolution and absolute
> > + * limits of the ISP.
> > + */
> > + minResolution = minResolution_.expandedToAspectRatio(resolution);
> > + maxResolution = maxResolution_.boundedToAspectRatio(resolution)
> > + .boundedTo(resolution);
> > + }
> >
> > cfg->size.boundTo(maxResolution);
> > cfg->size.expandTo(minResolution);
> > @@ -182,15 +328,11 @@ int RkISP1Path::configure(const StreamConfiguration &config,
> > << "Configuring " << name_ << " resizer output pad with "
> > << ispFormat;
> >
> > - switch (config.pixelFormat) {
> > - case formats::NV12:
> > - case formats::NV21:
> > - ispFormat.mbus_code = MEDIA_BUS_FMT_YUYV8_1_5X8;
> > - break;
> > - default:
> > - ispFormat.mbus_code = MEDIA_BUS_FMT_YUYV8_2X8;
> > - break;
> > - }
> > + /*
> > + * The configuration has been validated, the pixel format is guaranteed
> > + * to be supported and thus found in formatToMediaBus.
> > + */
> > + ispFormat.mbus_code = formatToMediaBus.at(config.pixelFormat);
>
> Uff, made to end of it and it seems all fine!
>
> Thanks
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
>
> > ret = resizer_->setFormat(1, &ispFormat);
> > if (ret < 0)
> > @@ -266,14 +408,25 @@ void RkISP1Path::stop()
> > namespace {
> > constexpr Size RKISP1_RSZ_MP_SRC_MIN{ 32, 16 };
> > constexpr Size RKISP1_RSZ_MP_SRC_MAX{ 4416, 3312 };
> > -constexpr std::array<PixelFormat, 6> RKISP1_RSZ_MP_FORMATS{
> > +constexpr std::array<PixelFormat, 18> RKISP1_RSZ_MP_FORMATS{
> > formats::YUYV,
> > formats::NV16,
> > formats::NV61,
> > formats::NV21,
> > formats::NV12,
> > formats::R8,
> > - /* \todo Add support for RAW formats. */
> > + formats::SBGGR8,
> > + formats::SGBRG8,
> > + formats::SGRBG8,
> > + formats::SRGGB8,
> > + formats::SBGGR10,
> > + formats::SGBRG10,
> > + formats::SGRBG10,
> > + formats::SRGGB10,
> > + formats::SBGGR12,
> > + formats::SGBRG12,
> > + formats::SGRBG12,
> > + formats::SRGGB12,
> > };
> >
> > constexpr Size RKISP1_RSZ_SP_SRC_MIN{ 32, 16 };
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> > index bf4ad18fbbf2..bdf3f95b95e1 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> > @@ -40,7 +40,8 @@ public:
> > int setEnabled(bool enable) { return link_->setEnabled(enable); }
> > bool isEnabled() const { return link_->flags() & MEDIA_LNK_FL_ENABLED; }
> >
> > - StreamConfiguration generateConfiguration(const CameraSensor *sensor);
> > + StreamConfiguration generateConfiguration(const CameraSensor *sensor,
> > + StreamRole role);
> > CameraConfiguration::Status validate(const CameraSensor *sensor,
> > StreamConfiguration *cfg);
> >
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list