[PATCH v3 4/4] libcamera: rkisp1: Plumb the ConverterDW100 converter
Umang Jain
umang.jain at ideasonboard.com
Wed Jun 26 05:46:58 CEST 2024
Hi Kieran,
On 26/06/24 2:34 am, Kieran Bingham wrote:
> Quoting Umang Jain (2024-06-25 07:23:27)
>> Plumb the ConverterDW100 converter in the rkisp1 pipeline handler.
>> If the dewarper is found, it is instantiated and buffers are exported
>> from it, instead of RkISP1Path. Internal buffers are allocated for the
>> RkISP1Path in case where dewarper is going to be used.
>>
>> The RKISP1 pipeline handler now supports scaler crop control through
>> ConverterDW100. Register the ScalerCrop control for the cameras created
>> in the RKISP1 pipeline handler.
>>
>> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
>> ---
>> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 143 +++++++++++++++++-
>> src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 12 +-
>> src/libcamera/pipeline/rkisp1/rkisp1_path.h | 14 ++
>> 3 files changed, 161 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
>> index bfc44239..b1314d73 100644
>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
>> @@ -8,9 +8,11 @@
>> #include <algorithm>
>> #include <array>
>> #include <iomanip>
>> +#include <map>
>> #include <memory>
>> #include <numeric>
>> #include <queue>
>> +#include <vector>
>>
>> #include <linux/media-bus-format.h>
>> #include <linux/rkisp1-config.h>
>> @@ -33,6 +35,7 @@
>>
>> #include "libcamera/internal/camera.h"
>> #include "libcamera/internal/camera_sensor.h"
>> +#include "libcamera/internal/converter/converter_dw100.h"
>> #include "libcamera/internal/delayed_controls.h"
>> #include "libcamera/internal/device_enumerator.h"
>> #include "libcamera/internal/framebuffer.h"
>> @@ -62,6 +65,8 @@ struct RkISP1FrameInfo {
>>
>> bool paramDequeued;
>> bool metadataProcessed;
>> +
>> + std::optional<Rectangle> scalerCrop;
>> };
>>
>> class RkISP1Frames
>> @@ -165,6 +170,8 @@ public:
>>
>> private:
>> static constexpr Size kRkISP1PreviewSize = { 1920, 1080 };
>> + static constexpr Size kMinDewarpSize = { 176, 144 };
>> + static constexpr Size kMaxDewarpSize = { 4096, 3072 };
> Shouldn't these be properties of the m2m dewarp class somehow?
>
>>
>> RkISP1CameraData *cameraData(Camera *camera)
>> {
>> @@ -181,6 +188,7 @@ private:
>> void bufferReady(FrameBuffer *buffer);
>> void paramReady(FrameBuffer *buffer);
>> void statReady(FrameBuffer *buffer);
>> + void dewarpBufferReady(FrameBuffer *buffer);
>> void frameStart(uint32_t sequence);
>>
>> int allocateBuffers(Camera *camera);
>> @@ -200,6 +208,12 @@ private:
>> RkISP1MainPath mainPath_;
>> RkISP1SelfPath selfPath_;
>>
>> + std::unique_ptr<ConverterDW100> dewarper_;
>> +
>> + /* Internal buffers used when dewarper is being used */
>> + std::vector<std::unique_ptr<FrameBuffer>> mainPathBuffers_;
>> + std::queue<FrameBuffer *> availableMainPathBuffers_;
>> +
>> std::vector<std::unique_ptr<FrameBuffer>> paramBuffers_;
>> std::vector<std::unique_ptr<FrameBuffer>> statBuffers_;
>> std::queue<FrameBuffer *> availableParamBuffers_;
>> @@ -222,6 +236,8 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req
>>
>> FrameBuffer *paramBuffer = nullptr;
>> FrameBuffer *statBuffer = nullptr;
>> + FrameBuffer *mainPathBuffer = nullptr;
>> + FrameBuffer *selfPathBuffer = nullptr;
>>
>> if (!isRaw) {
>> if (pipe_->availableParamBuffers_.empty()) {
>> @@ -239,10 +255,16 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req
>>
>> statBuffer = pipe_->availableStatBuffers_.front();
>> pipe_->availableStatBuffers_.pop();
>> +
>> + if (pipe_->dewarper_) {
>> + mainPathBuffer = pipe_->availableMainPathBuffers_.front();
>> + pipe_->availableMainPathBuffers_.pop();
>> + }
>> }
>>
>> - FrameBuffer *mainPathBuffer = request->findBuffer(&data->mainPathStream_);
>> - FrameBuffer *selfPathBuffer = request->findBuffer(&data->selfPathStream_);
>> + if (!mainPathBuffer)
>> + mainPathBuffer = request->findBuffer(&data->mainPathStream_);
>> + selfPathBuffer = request->findBuffer(&data->selfPathStream_);
>>
>> RkISP1FrameInfo *info = new RkISP1FrameInfo;
>>
>> @@ -268,6 +290,7 @@ int RkISP1Frames::destroy(unsigned int frame)
>>
>> pipe_->availableParamBuffers_.push(info->paramBuffer);
>> pipe_->availableStatBuffers_.push(info->statBuffer);
>> + pipe_->availableMainPathBuffers_.push(info->mainPathBuffer);
>>
>> frameInfo_.erase(info->frame);
>>
>> @@ -283,6 +306,7 @@ void RkISP1Frames::clear()
>>
>> pipe_->availableParamBuffers_.push(info->paramBuffer);
>> pipe_->availableStatBuffers_.push(info->statBuffer);
>> + pipe_->availableMainPathBuffers_.push(info->mainPathBuffer);
>>
>> delete info;
>> }
>> @@ -785,12 +809,26 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>> << " crop " << rect;
>>
>> std::map<unsigned int, IPAStream> streamConfig;
>> + std::vector<std::reference_wrapper<StreamConfiguration>> outputCfgs;
>>
>> for (const StreamConfiguration &cfg : *config) {
>> if (cfg.stream() == &data->mainPathStream_) {
>> ret = mainPath_.configure(cfg, format);
>> streamConfig[0] = IPAStream(cfg.pixelFormat,
>> cfg.size);
>> + /* Configure dewarp */
>> + if (dewarper_ && !isRaw_) {
>> + /*
>> + * \todo Converter::configure() API should be changed
>> + * to use std::vector<std::reference_wrapper<const StreamConfiguration>> ?
>> + */
>> + outputCfgs.push_back(const_cast<StreamConfiguration &>(cfg));
>> + dewarper_->configure(cfg, outputCfgs);
>> + } else if (dewarper_ && isRaw_) {
>> + LOG(RkISP1, Debug) << "Dewarper disabled on RAW configuration";
>> + dewarper_.reset();
> Does this remove the dewarper if someone asks for a raw stream, and mean
> it could no longer be available if they then switch back to ask for a
> processed stream?
>
> I think so ... and fixing that will require reworking quite a bit of the
> patch below so I'll hold off here.
>
> I don't think the 'presence' of dewarper_ can be used to fully determine
> if we use it or not - that' based on the configuration. And we shouldn't
> allocate unless we're using it - and we shouldn't destroy dewarper_ if
> we don't use if for just a single streaming session.
You are correct, we shouldn't be resetting the dewarper_, if not in use.
It should be dependent on the configuration
The raw configuration is tracked by isRaw_ member variable in the
pipeline, so probably I'll replace
if (dewarper_)
...
with
if (dewarper_ && !isRaw_)
.,..
at most places - in the next version, without destroying the dewarper_.
>
> --
> Kieran
>
>
>> + }
>> +
>> } else if (hasSelfPath_) {
>> ret = selfPath_.configure(cfg, format);
>> streamConfig[1] = IPAStream(cfg.pixelFormat,
>> @@ -839,6 +877,9 @@ int PipelineHandlerRkISP1::exportFrameBuffers([[maybe_unused]] Camera *camera, S
>> RkISP1CameraData *data = cameraData(camera);
>> unsigned int count = stream->configuration().bufferCount;
>>
>> + if (!isRaw_ && dewarper_)
>> + return dewarper_->exportBuffers(&data->mainPathStream_, count, buffers);
>> +
>> if (stream == &data->mainPathStream_)
>> return mainPath_.exportBuffers(count, buffers);
>> else if (hasSelfPath_ && stream == &data->selfPathStream_)
>> @@ -866,6 +907,16 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
>> ret = stat_->allocateBuffers(maxCount, &statBuffers_);
>> if (ret < 0)
>> goto error;
>> +
>> + /* If the dewarper is being used, allocate internal buffers for ISP */
>> + if (dewarper_) {
>> + ret = mainPath_.allocateBuffers(maxCount, &mainPathBuffers_);
>> + if (ret < 0)
>> + goto error;
>> +
>> + for (std::unique_ptr<FrameBuffer> &buffer : mainPathBuffers_)
>> + availableMainPathBuffers_.push(buffer.get());
>> + }
>> }
>>
>> for (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) {
>> @@ -889,6 +940,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
>> error:
>> paramBuffers_.clear();
>> statBuffers_.clear();
>> + mainPathBuffers_.clear();
>>
>> return ret;
>> }
>> @@ -903,8 +955,12 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)
>> while (!availableParamBuffers_.empty())
>> availableParamBuffers_.pop();
>>
>> + while (!availableMainPathBuffers_.empty())
>> + availableMainPathBuffers_.pop();
>> +
>> paramBuffers_.clear();
>> statBuffers_.clear();
>> + mainPathBuffers_.clear();
>>
>> std::vector<unsigned int> ids;
>> for (IPABuffer &ipabuf : data->ipaBuffers_)
>> @@ -919,6 +975,9 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)
>> if (stat_->releaseBuffers())
>> LOG(RkISP1, Error) << "Failed to release stat buffers";
>>
>> + if (dewarper_)
>> + mainPath_.releaseBuffers();
>> +
>> return 0;
>> }
>>
>> @@ -961,6 +1020,14 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL
>> << "Failed to start statistics " << camera->id();
>> return ret;
>> }
>> +
>> + if (dewarper_) {
>> + ret = dewarper_->start();
>> + if (ret) {
>> + LOG(RkISP1, Error) << "Failed to start dewarper";
>> + return ret;
>> + }
>> + }
>> }
>>
>> if (data->mainPath_->isEnabled()) {
>> @@ -1015,6 +1082,9 @@ void PipelineHandlerRkISP1::stopDevice(Camera *camera)
>> if (ret)
>> LOG(RkISP1, Warning)
>> << "Failed to stop parameters for " << camera->id();
>> +
>> + if (dewarper_)
>> + dewarper_->stop();
>> }
>>
>> ASSERT(data->queuedRequests_.empty());
>> @@ -1045,6 +1115,13 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)
>> info->paramBuffer->cookie());
>> }
>>
>> + const auto &crop = request->controls().get(controls::ScalerCrop);
>> + if (crop && !isRaw_) {
>> + int ret = dewarper_->setScalerCrop(&data->mainPathStream_, crop.value());
>> + if (!ret)
>> + info->scalerCrop = crop;
>> + }
>> +
>> data->frame_++;
>>
>> return 0;
>> @@ -1110,6 +1187,13 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)
>> {
>> ControlInfoMap::Map rkisp1Controls;
>>
>> + if (dewarper_) {
>> + Rectangle maxCrop(kMaxDewarpSize);
>> + Rectangle minCrop = kMinDewarpSize.centeredTo(maxCrop.center());
>> +
>> + rkisp1Controls[&controls::ScalerCrop] = ControlInfo(minCrop, maxCrop, maxCrop);
>> + }
>> +
>> /* Add the IPA registered controls to list of camera controls. */
>> for (const auto &ipaControl : data->ipaControls_)
>> rkisp1Controls[ipaControl.first] = ipaControl.second;
>> @@ -1173,6 +1257,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
>>
>> bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
>> {
>> + std::shared_ptr<MediaDevice> dwpMediaDevice;
>> const MediaPad *pad;
>>
>> DeviceMatch dm("rkisp1");
>> @@ -1237,6 +1322,26 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
>> stat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statReady);
>> param_->bufferReady.connect(this, &PipelineHandlerRkISP1::paramReady);
>>
>> + /* If dewarper is present, create its instance. */
>> + DeviceMatch dwp("dw100");
>> + dwp.add("dw100-source");
>> + dwp.add("dw100-sink");
>> +
>> + dwpMediaDevice = enumerator->search(dwp);
>> + if (dwpMediaDevice) {
>> + dewarper_ = std::make_unique<ConverterDW100>(std::move(dwpMediaDevice));
>> + if (dewarper_->isValid()) {
>> + dewarper_->outputBufferReady.connect(
>> + this, &PipelineHandlerRkISP1::dewarpBufferReady);
>> +
>> + LOG(RkISP1, Info) << "Using DW100 dewarper " << dewarper_->deviceNode();
>> + } else {
>> + LOG(RkISP1, Debug) << "Found DW100 dewarper " << dewarper_->deviceNode()
>> + << " but invalid";
>> + dewarper_.reset();
>> + }
>> + }
>> +
>> /*
>> * Enumerate all sensors connected to the ISP and create one
>> * camera instance for each of them.
>> @@ -1283,7 +1388,7 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
>> return;
>>
>> const FrameMetadata &metadata = buffer->metadata();
>> - Request *request = buffer->request();
>> + Request *request = info->request;
>>
>> if (metadata.status != FrameMetadata::FrameCancelled) {
>> /*
>> @@ -1300,11 +1405,43 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
>> data->delayedCtrls_->get(metadata.sequence);
>> data->ipa_->processStatsBuffer(info->frame, 0, ctrls);
>> }
>> +
>> } else {
>> if (isRaw_)
>> info->metadataProcessed = true;
>> }
>>
>> + if (dewarper_) {
>> + /*
>> + * Queue input and output buffers to the dewarper. The output
>> + * buffers for the dewarper are the buffers of the request, supplied
>> + * by the application.
>> + */
>> + int ret = dewarper_->queueBuffers(buffer, request->buffers());
>> + if (ret < 0)
>> + LOG(RkISP1, Error) << "Cannot queue buffers to dewarper: "
>> + << strerror(-ret);
>> +
>> + return;
>> + }
>> +
>> + completeBuffer(request, buffer);
>> + tryCompleteRequest(info);
>> +}
>> +
>> +void PipelineHandlerRkISP1::dewarpBufferReady(FrameBuffer *buffer)
>> +{
>> + ASSERT(activeCamera_);
>> + RkISP1CameraData *data = cameraData(activeCamera_);
>> + Request *request = buffer->request();
>> +
>> + RkISP1FrameInfo *info = data->frameInfo_.find(buffer->request());
>> + if (!info)
>> + return;
>> +
>> + if (info->scalerCrop)
>> + request->metadata().set(controls::ScalerCrop, info->scalerCrop.value());
>> +
>> completeBuffer(request, buffer);
>> tryCompleteRequest(info);
>> }
>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
>> index c49017d1..c96ec1d6 100644
>> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
>> @@ -56,7 +56,7 @@ const std::map<PixelFormat, uint32_t> formatToMediaBus = {
>>
>> RkISP1Path::RkISP1Path(const char *name, const Span<const PixelFormat> &formats,
>> const Size &minResolution, const Size &maxResolution)
>> - : name_(name), running_(false), formats_(formats),
>> + : name_(name), running_(false), internalBufs_(false), formats_(formats),
>> minResolution_(minResolution), maxResolution_(maxResolution),
>> link_(nullptr)
>> {
>> @@ -402,10 +402,12 @@ int RkISP1Path::start()
>> if (running_)
>> return -EBUSY;
>>
>> - /* \todo Make buffer count user configurable. */
>> - ret = video_->importBuffers(RKISP1_BUFFER_COUNT);
>> - if (ret)
>> - return ret;
>> + if (!internalBufs_) {
>> + /* \todo Make buffer count user configurable. */
>> + ret = video_->importBuffers(RKISP1_BUFFER_COUNT);
>> + if (ret)
>> + return ret;
>> + }
>>
>> ret = video_->streamOn();
>> if (ret) {
>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
>> index 08edefec..b7fa82d0 100644
>> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h
>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
>> @@ -55,6 +55,19 @@ public:
>> return video_->exportBuffers(bufferCount, buffers);
>> }
>>
>> + int allocateBuffers(unsigned int bufferCount,
>> + std::vector<std::unique_ptr<FrameBuffer>> *buffers)
>> + {
>> + internalBufs_ = true;
>> + return video_->allocateBuffers(bufferCount, buffers);
>> + }
>> +
>> + int releaseBuffers()
>> + {
>> + ASSERT(internalBufs_);
>> + return video_->releaseBuffers();
>> + }
>> +
>> int start();
>> void stop();
>>
>> @@ -68,6 +81,7 @@ private:
>>
>> const char *name_;
>> bool running_;
>> + bool internalBufs_;
>>
>> const Span<const PixelFormat> formats_;
>> std::set<PixelFormat> streamFormats_;
>> --
>> 2.44.0
>>
More information about the libcamera-devel
mailing list