[PATCH v3 4/4] libcamera: rkisp1: Plumb the ConverterDW100 converter

Kieran Bingham kieran.bingham at ideasonboard.com
Tue Jun 25 23:04:34 CEST 2024


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.

--
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