[PATCH v3 12/16] libcamera: pipeline: simple: enable use of Soft ISP and Soft IPA
Kieran Bingham
kieran.bingham at ideasonboard.com
Fri Feb 16 16:25:36 CET 2024
Quoting Milan Zamazal (2024-02-15 16:47:54)
> Hans de Goede <hdegoede at redhat.com> writes:
>
> > From: Andrey Konovalov <andrey.konovalov at linaro.org>
> >
> > To enable the Simple Soft ISP and Soft IPA for simple pipeline handler
> > configure the build with:
> > -Dpipelines=simple -Dipas=simple
> >
> > Also using the Soft ISP for the particular hardware platform must
> > be enabled in the supportedDevices[] table.
>
> It would be nice if it was possible to use/test software ISP without specific
> patches to the array. Do I get it right that the idea is to enable softisp for
> certain devices where it can be used & there is no better pipeline handler to
> use? And there could be a runtime option for devices where it can be used &
> there is a better pipeline handler to use? Not a blocker but it should be
> clarified in the commit message.
I think for now - this is compile time code configuration, which I preferred
over having a compile meson option that would have been harder to
support to distributions to convey the diffence between "This pipeline
really needs the SoftISP" vs "This pipeline can optionally use the
SoftISP".
Having runtime configuration with a pipeline configuration file may be
appropriate in the near future, so it doesn't have to stay fixed like
this.
But we probably need to handle things like raw stream passthrough before
we 'enable' the SoftISP more generically.
--
Kieran
>
> > If the pipeline uses Converter, Soft ISP and Soft IPA aren't
> > available.
> >
> > Tested-by: Bryan O'Donoghue <bryan.odonoghue at linaro.org> # sc8280xp Lenovo x13s
> > Tested-by: Pavel Machek <pavel at ucw.cz>
> > Reviewed-by: Pavel Machek <pavel at ucw.cz>
> > Signed-off-by: Andrey Konovalov <andrey.konovalov at linaro.org>
> > Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> > ---
> > src/libcamera/pipeline/simple/simple.cpp | 136 ++++++++++++++++++-----
> > 1 file changed, 108 insertions(+), 28 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > index 78854ef8..eab5b56e 100644
> > --- a/src/libcamera/pipeline/simple/simple.cpp
> > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > @@ -34,6 +34,7 @@
> > #include "libcamera/internal/device_enumerator.h"
> > #include "libcamera/internal/media_device.h"
> > #include "libcamera/internal/pipeline_handler.h"
> > +#include "libcamera/internal/software_isp/software_isp.h"
> > #include "libcamera/internal/v4l2_subdevice.h"
> > #include "libcamera/internal/v4l2_videodevice.h"
> >
> > @@ -185,17 +186,22 @@ struct SimplePipelineInfo {
> > * and the number of streams it supports.
> > */
> > std::vector<std::pair<const char *, unsigned int>> converters;
> > + /*
> > + * Using Software ISP is to be enabled per driver.
> > + * The Software ISP can't be used together with the converters.
> > + */
> > + bool swIspEnabled;
> > };
> >
> > namespace {
> >
> > static const SimplePipelineInfo supportedDevices[] = {
> > - { "dcmipp", {} },
> > - { "imx7-csi", { { "pxp", 1 } } },
> > - { "j721e-csi2rx", {} },
> > - { "mxc-isi", {} },
> > - { "qcom-camss", {} },
> > - { "sun6i-csi", {} },
> > + { "dcmipp", {}, false },
> > + { "imx7-csi", { { "pxp", 1 } }, false },
> > + { "j721e-csi2rx", {}, false },
> > + { "mxc-isi", {}, false },
> > + { "qcom-camss", {}, true },
> > + { "sun6i-csi", {}, false },
> > };
> >
> > } /* namespace */
> > @@ -274,6 +280,7 @@ public:
> > bool useConversion_;
> >
> > std::unique_ptr<Converter> converter_;
> > + std::unique_ptr<SoftwareIsp> swIsp_;
> >
> > private:
> > void tryPipeline(unsigned int code, const Size &size);
> > @@ -281,6 +288,9 @@ private:
> >
> > void conversionInputDone(FrameBuffer *buffer);
> > void conversionOutputDone(FrameBuffer *buffer);
> > +
> > + void ispStatsReady(int dummy);
> > + void setSensorControls(const ControlList &sensorControls);
> > };
> >
> > class SimpleCameraConfiguration : public CameraConfiguration
> > @@ -332,6 +342,7 @@ public:
> > V4L2VideoDevice *video(const MediaEntity *entity);
> > V4L2Subdevice *subdev(const MediaEntity *entity);
> > MediaDevice *converter() { return converter_; }
> > + bool swIspEnabled() { return swIspEnabled_; }
> >
> > protected:
> > int queueRequestDevice(Camera *camera, Request *request) override;
> > @@ -360,6 +371,7 @@ private:
> > std::map<const MediaEntity *, EntityData> entities_;
> >
> > MediaDevice *converter_;
> > + bool swIspEnabled_;
> > };
> >
> > /* -----------------------------------------------------------------------------
> > @@ -509,6 +521,29 @@ int SimpleCameraData::init()
> > }
> > }
> >
> > + /*
> > + * Instantiate Soft ISP if this is enabled for the given driver and no converter is used.
> > + */
> > + if (!converter_ && pipe->swIspEnabled()) {
> > + swIsp_ = std::make_unique<SoftwareIsp>(pipe, sensor_->controls());
> > + if (!swIsp_->isValid()) {
> > + LOG(SimplePipeline, Warning)
> > + << "Failed to create software ISP, disabling software debayering";
> > + swIsp_.reset();
> > + } else {
> > + /*
> > + * \todo explain why SimpleCameraData::conversionInputDone() can't be directly
> > + * connected to inputBufferReady signal.
> > + */
> > + swIsp_->inputBufferReady.connect(pipe, [this](FrameBuffer *buffer) {
> > + this->conversionInputDone(buffer);
> > + });
> > + swIsp_->outputBufferReady.connect(this, &SimpleCameraData::conversionOutputDone);
> > + swIsp_->ispStatsReady.connect(this, &SimpleCameraData::ispStatsReady);
> > + swIsp_->setSensorControls.connect(this, &SimpleCameraData::setSensorControls);
> > + }
> > + }
> > +
> > video_ = pipe->video(entities_.back().entity);
> > ASSERT(video_);
> >
> > @@ -599,12 +634,20 @@ void SimpleCameraData::tryPipeline(unsigned int code, const Size &size)
> > config.captureFormat = pixelFormat;
> > config.captureSize = format.size;
> >
> > - if (!converter_) {
> > - config.outputFormats = { pixelFormat };
> > - config.outputSizes = config.captureSize;
> > - } else {
> > + if (converter_) {
> > config.outputFormats = converter_->formats(pixelFormat);
> > config.outputSizes = converter_->sizes(format.size);
> > + } else if (swIsp_) {
> > + config.outputFormats = swIsp_->formats(pixelFormat);
> > + config.outputSizes = swIsp_->sizes(pixelFormat, format.size);
> > + if (config.outputFormats.empty()) {
> > + /* Do not use swIsp for unsupported pixelFormat's: */
> > + config.outputFormats = { pixelFormat };
> > + config.outputSizes = config.captureSize;
> > + }
> > + } else {
> > + config.outputFormats = { pixelFormat };
> > + config.outputSizes = config.captureSize;
> > }
> >
> > configs_.push_back(config);
> > @@ -750,9 +793,9 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)
> > }
> >
> > /*
> > - * The converter is in use. Requeue the internal buffer for
> > - * capture (unless the stream is being stopped), and complete
> > - * the request with all the user-facing buffers.
> > + * The converter or Software ISP is in use. Requeue the internal
> > + * buffer for capture (unless the stream is being stopped), and
> > + * complete the request with all the user-facing buffers.
> > */
> > if (buffer->metadata().status != FrameMetadata::FrameCancelled)
> > video_->queueBuffer(buffer);
> > @@ -798,9 +841,9 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)
> > buffer->metadata().timestamp);
> >
> > /*
> > - * Queue the captured and the request buffer to the converter if format
> > - * conversion is needed. If there's no queued request, just requeue the
> > - * captured buffer for capture.
> > + * Queue the captured and the request buffer to the converter or Software
> > + * ISP if format conversion is needed. If there's no queued request, just
> > + * requeue the captured buffer for capture.
> > */
> > if (useConversion_) {
> > if (conversionQueue_.empty()) {
> > @@ -808,7 +851,11 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)
> > return;
> > }
> >
> > - converter_->queueBuffers(buffer, conversionQueue_.front());
> > + if (converter_)
> > + converter_->queueBuffers(buffer, conversionQueue_.front());
> > + else
> > + swIsp_->queueBuffers(buffer, conversionQueue_.front());
> > +
> > conversionQueue_.pop();
> > return;
> > }
> > @@ -834,6 +881,18 @@ void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)
> > pipe->completeRequest(request);
> > }
> >
> > +void SimpleCameraData::ispStatsReady([[maybe_unused]] int dummy)
> > +{
> > + swIsp_->processStats(sensor_->getControls({ V4L2_CID_ANALOGUE_GAIN,
> > + V4L2_CID_EXPOSURE }));
> > +}
> > +
> > +void SimpleCameraData::setSensorControls(const ControlList &sensorControls)
> > +{
> > + ControlList ctrls(sensorControls);
> > + sensor_->setControls(&ctrls);
> > +}
> > +
> > /* Retrieve all source pads connected to a sink pad through active routes. */
> > std::vector<const MediaPad *> SimpleCameraData::routedSourcePads(MediaPad *sink)
> > {
> > @@ -1046,8 +1105,10 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()
> > /* Set the stride, frameSize and bufferCount. */
> > if (needConversion_) {
> > std::tie(cfg.stride, cfg.frameSize) =
> > - data_->converter_->strideAndFrameSize(cfg.pixelFormat,
> > - cfg.size);
> > + (data_->converter_) ? data_->converter_->strideAndFrameSize(cfg.pixelFormat,
> > + cfg.size)
> > + : data_->swIsp_->strideAndFrameSize(cfg.pixelFormat,
> > + cfg.size);
> > if (cfg.stride == 0)
> > return Invalid;
> > } else {
> > @@ -1210,7 +1271,9 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
> > inputCfg.stride = captureFormat.planes[0].bpl;
> > inputCfg.bufferCount = kNumInternalBuffers;
> >
> > - return data->converter_->configure(inputCfg, outputCfgs);
> > + return (data->converter_) ? data->converter_->configure(inputCfg, outputCfgs)
> > + : data->swIsp_->configure(inputCfg, outputCfgs,
> > + data->sensor_->controls());
> > }
> >
> > int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,
> > @@ -1224,8 +1287,10 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,
> > * whether the converter is used or not.
> > */
> > if (data->useConversion_)
> > - return data->converter_->exportBuffers(data->streamIndex(stream),
> > - count, buffers);
> > + return (data->converter_) ? data->converter_->exportBuffers(data->streamIndex(stream),
> > + count, buffers)
> > + : data->swIsp_->exportBuffers(data->streamIndex(stream),
> > + count, buffers);
> > else
> > return data->video_->exportBuffers(count, buffers);
> > }
> > @@ -1270,10 +1335,18 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL
> > }
> >
> > if (data->useConversion_) {
> > - ret = data->converter_->start();
> > - if (ret < 0) {
> > - stop(camera);
> > - return ret;
> > + if (data->converter_) {
> > + ret = data->converter_->start();
> > + if (ret < 0) {
> > + stop(camera);
> > + return ret;
> > + }
> > + } else if (data->swIsp_) {
> > + ret = data->swIsp_->start();
> > + if (ret < 0) {
> > + stop(camera);
> > + return ret;
> > + }
> > }
> >
> > /* Queue all internal buffers for capture. */
> > @@ -1289,8 +1362,13 @@ void SimplePipelineHandler::stopDevice(Camera *camera)
> > SimpleCameraData *data = cameraData(camera);
> > V4L2VideoDevice *video = data->video_;
> >
> > - if (data->useConversion_)
> > - data->converter_->stop();
> > + if (data->useConversion_) {
> > + if (data->converter_)
> > + data->converter_->stop();
> > + else if (data->swIsp_) {
> > + data->swIsp_->stop();
> > + }
> > + }
> >
> > video->streamOff();
> > video->releaseBuffers();
> > @@ -1452,6 +1530,8 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
> > }
> > }
> >
> > + swIspEnabled_ = info->swIspEnabled;
> > +
> > /* Locate the sensors. */
> > std::vector<MediaEntity *> sensors = locateSensors();
> > if (sensors.empty()) {
>
More information about the libcamera-devel
mailing list