| Message ID | 20240214170122.60754-13-hdegoede@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hans de Goede <hdegoede@redhat.com> writes: > From: Andrey Konovalov <andrey.konovalov@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. > If the pipeline uses Converter, Soft ISP and Soft IPA aren't > available. > > Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # sc8280xp Lenovo x13s > Tested-by: Pavel Machek <pavel@ucw.cz> > Reviewed-by: Pavel Machek <pavel@ucw.cz> > Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org> > Signed-off-by: Hans de Goede <hdegoede@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()) {
Quoting Milan Zamazal (2024-02-15 16:47:54) > Hans de Goede <hdegoede@redhat.com> writes: > > > From: Andrey Konovalov <andrey.konovalov@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@linaro.org> # sc8280xp Lenovo x13s > > Tested-by: Pavel Machek <pavel@ucw.cz> > > Reviewed-by: Pavel Machek <pavel@ucw.cz> > > Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org> > > Signed-off-by: Hans de Goede <hdegoede@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()) { >
Hi Kieran, On 16.02.2024 18:25, Kieran Bingham wrote: > Quoting Milan Zamazal (2024-02-15 16:47:54) >> Hans de Goede <hdegoede@redhat.com> writes: >> >>> From: Andrey Konovalov <andrey.konovalov@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. Getting the raw bayer frames with "--stream pixelformat=$RAW_BAYER_FMT" vs the processed by SoftISP ones with e.g. "--stream pixelformat=RGB888" - would it count as raw stream passthrough? This doesn't currently work with simple pipeline handler, but I have two patches to fix this. (They conflict with the SoftISP patch set, so I am keeping them locally not to mess with the current work.) Thanks, Andrei > -- > Kieran > > >> >>> If the pipeline uses Converter, Soft ISP and Soft IPA aren't >>> available. >>> >>> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # sc8280xp Lenovo x13s >>> Tested-by: Pavel Machek <pavel@ucw.cz> >>> Reviewed-by: Pavel Machek <pavel@ucw.cz> >>> Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org> >>> Signed-off-by: Hans de Goede <hdegoede@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()) { >>
Quoting Andrei Konovalov (2024-02-16 16:19:25) > Hi Kieran, > > On 16.02.2024 18:25, Kieran Bingham wrote: > > Quoting Milan Zamazal (2024-02-15 16:47:54) > >> Hans de Goede <hdegoede@redhat.com> writes: > >> > >>> From: Andrey Konovalov <andrey.konovalov@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. > > Getting the raw bayer frames with "--stream pixelformat=$RAW_BAYER_FMT" vs > the processed by SoftISP ones with e.g. "--stream pixelformat=RGB888" - would it > count as raw stream passthrough? Yes. > This doesn't currently work with simple pipeline handler, but I have two > patches to fix this. (They conflict with the SoftISP patch set, so I am keeping > them locally not to mess with the current work.) That's fine for now I think. But the 'big picture' goal here for simple pipeline handler is that Raw streams should still be accessible. That could be initially that you can only support a single stream (either RAW or SoftProcessed) ... but I could imagine later (particurly when the ISP can be GPU accellerated) that there might also be the RAW+Processed streams desired. Maybe we should really rename 'simple' pipeline handler sometime ... -- Kieran > > Thanks, > Andrei > > > -- > > Kieran > > > > > >> > >>> If the pipeline uses Converter, Soft ISP and Soft IPA aren't > >>> available. > >>> > >>> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # sc8280xp Lenovo x13s > >>> Tested-by: Pavel Machek <pavel@ucw.cz> > >>> Reviewed-by: Pavel Machek <pavel@ucw.cz> > >>> Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org> > >>> Signed-off-by: Hans de Goede <hdegoede@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()) { > >>
On 16.02.2024 20:14, Kieran Bingham wrote: > Quoting Andrei Konovalov (2024-02-16 16:19:25) >> Hi Kieran, >> >> On 16.02.2024 18:25, Kieran Bingham wrote: >>> Quoting Milan Zamazal (2024-02-15 16:47:54) >>>> Hans de Goede <hdegoede@redhat.com> writes: >>>> >>>>> From: Andrey Konovalov <andrey.konovalov@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. >> >> Getting the raw bayer frames with "--stream pixelformat=$RAW_BAYER_FMT" vs >> the processed by SoftISP ones with e.g. "--stream pixelformat=RGB888" - would it >> count as raw stream passthrough? > > Yes. > >> This doesn't currently work with simple pipeline handler, but I have two >> patches to fix this. (They conflict with the SoftISP patch set, so I am keeping >> them locally not to mess with the current work.) > > That's fine for now I think. But the 'big picture' goal here for simple > pipeline handler is that Raw streams should still be accessible. That > could be initially that you can only support a single stream (either RAW > or SoftProcessed) ... but I could imagine later (particurly when the ISP > can be GPU accellerated) that there might also be the RAW+Processed > streams desired. I see. Multiple streams do make sense in longer term. > Maybe we should really rename 'simple' pipeline handler sometime ... Maybe. Next time one should think twice before calling a piece of software 'simple', as this is hard to predict what the things end up with :) Thanks, Andrei > -- > Kieran > >> >> Thanks, >> Andrei >> >>> -- >>> Kieran >>> >>> >>>> >>>>> If the pipeline uses Converter, Soft ISP and Soft IPA aren't >>>>> available. >>>>> >>>>> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # sc8280xp Lenovo x13s >>>>> Tested-by: Pavel Machek <pavel@ucw.cz> >>>>> Reviewed-by: Pavel Machek <pavel@ucw.cz> >>>>> Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org> >>>>> Signed-off-by: Hans de Goede <hdegoede@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()) { >>>>
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()) {