[libcamera-devel] [PATCH] pipeline: ipa: raspberrypi: Rename IPA Interface namespace to ipa::RPi

Naushir Patuck naush at raspberrypi.com
Thu Feb 18 12:19:36 CET 2021


Hi Kieran,


On Thu, 18 Feb 2021 at 11:16, Kieran Bingham <
kieran.bingham at ideasonboard.com> wrote:

>
>
> On 18/02/2021 11:14, Kieran Bingham wrote:
> > Hi Naush,
> >
> > On 17/02/2021 10:34, Naushir Patuck wrote:
> >> Rename the IPA interface namespace to ipa::RPi for consistency with
> >> the libcamera::RPi namespace label.
> >>
> >> There is no functional change in this commit.
> >
> > Is this a v2 of [1/3]?
>
> Ahem, this is 2/3
>
> For me it helps to include a cover letter on a series, even if it's
> blank - because otherwise all of the patcheas get chained from 1/3,
> making 1/3 look like a cover letter :S
>

Yes, it is the v2.  Sorry for the confusion, I will always put a cover
letter on patch series in the future!

Regards,
Naush


>
> --
> Kieran
>
>
> >
> > It looks like it so I think we can add:
> >
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> >
> >
> >> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> >> ---
> >>  include/libcamera/ipa/raspberrypi.mojom       |  2 +-
> >>  src/ipa/raspberrypi/raspberrypi.cpp           | 38 +++++++++----------
> >>  .../pipeline/raspberrypi/raspberrypi.cpp      | 30 +++++++--------
> >>  .../pipeline/raspberrypi/rpi_stream.cpp       |  4 +-
> >>  .../pipeline/raspberrypi/rpi_stream.h         |  4 +-
> >>  5 files changed, 39 insertions(+), 39 deletions(-)
> >>
> >> diff --git a/include/libcamera/ipa/raspberrypi.mojom
> b/include/libcamera/ipa/raspberrypi.mojom
> >> index 9c05cc68cceb..5a27b1e4fc2d 100644
> >> --- a/include/libcamera/ipa/raspberrypi.mojom
> >> +++ b/include/libcamera/ipa/raspberrypi.mojom
> >> @@ -1,6 +1,6 @@
> >>  /* SPDX-License-Identifier: LGPL-2.1-or-later */
> >>
> >> -module ipa.rpi;
> >> +module ipa.RPi;
> >>
> >>  import "include/libcamera/ipa/core.mojom";
> >>
> >> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp
> b/src/ipa/raspberrypi/raspberrypi.cpp
> >> index 974f4ec63058..1226ea514521 100644
> >> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> >> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> >> @@ -63,7 +63,7 @@ constexpr double defaultMaxFrameDuration = 1e6 / 0.01;
> >>
> >>  LOG_DEFINE_CATEGORY(IPARPI)
> >>
> >> -class IPARPi : public ipa::rpi::IPARPiInterface
> >> +class IPARPi : public ipa::RPi::IPARPiInterface
> >>  {
> >>  public:
> >>      IPARPi()
> >> @@ -76,24 +76,24 @@ public:
> >>      ~IPARPi()
> >>      {
> >>              if (lsTable_)
> >> -                    munmap(lsTable_, ipa::rpi::MaxLsGridSize);
> >> +                    munmap(lsTable_, ipa::RPi::MaxLsGridSize);
> >>      }
> >>
> >>      int init(const IPASettings &settings) override;
> >> -    void start(const ipa::rpi::StartControls &data,
> >> -               ipa::rpi::StartControls *result) override;
> >> +    void start(const ipa::RPi::StartControls &data,
> >> +               ipa::RPi::StartControls *result) override;
> >>      void stop() override {}
> >>
> >>      void configure(const CameraSensorInfo &sensorInfo,
> >>                     const std::map<unsigned int, IPAStream>
> &streamConfig,
> >>                     const std::map<unsigned int, ControlInfoMap>
> &entityControls,
> >> -                   const ipa::rpi::ConfigInput &data,
> >> -                   ipa::rpi::ConfigOutput *response, int32_t *ret)
> override;
> >> +                   const ipa::RPi::ConfigInput &data,
> >> +                   ipa::RPi::ConfigOutput *response, int32_t *ret)
> override;
> >>      void mapBuffers(const std::vector<IPABuffer> &buffers) override;
> >>      void unmapBuffers(const std::vector<unsigned int> &ids) override;
> >>      void signalStatReady(const uint32_t bufferId) override;
> >>      void signalQueueRequest(const ControlList &controls) override;
> >> -    void signalIspPrepare(const ipa::rpi::ISPConfig &data) override;
> >> +    void signalIspPrepare(const ipa::RPi::ISPConfig &data) override;
> >>
> >>  private:
> >>      void setMode(const CameraSensorInfo &sensorInfo);
> >> @@ -168,8 +168,8 @@ int IPARPi::init(const IPASettings &settings)
> >>      return 0;
> >>  }
> >>
> >> -void IPARPi::start(const ipa::rpi::StartControls &data,
> >> -               ipa::rpi::StartControls *result)
> >> +void IPARPi::start(const ipa::RPi::StartControls &data,
> >> +               ipa::RPi::StartControls *result)
> >>  {
> >>      RPiController::Metadata metadata;
> >>
> >> @@ -291,8 +291,8 @@ void IPARPi::setMode(const CameraSensorInfo
> &sensorInfo)
> >>  void IPARPi::configure(const CameraSensorInfo &sensorInfo,
> >>                     [[maybe_unused]] const std::map<unsigned int,
> IPAStream> &streamConfig,
> >>                     const std::map<unsigned int, ControlInfoMap>
> &entityControls,
> >> -                   const ipa::rpi::ConfigInput &ipaConfig,
> >> -                   ipa::rpi::ConfigOutput *result, int32_t *ret)
> >> +                   const ipa::RPi::ConfigInput &ipaConfig,
> >> +                   ipa::RPi::ConfigOutput *result, int32_t *ret)
> >>  {
> >>      if (entityControls.size() != 2) {
> >>              LOG(IPARPI, Error) << "No ISP or sensor controls found.";
> >> @@ -344,7 +344,7 @@ void IPARPi::configure(const CameraSensorInfo
> &sensorInfo,
> >>              helper_->GetDelays(exposureDelay, gainDelay);
> >>              sensorMetadata = helper_->SensorEmbeddedDataPresent();
> >>
> >> -            result->params |= ipa::rpi::ConfigSensorParams;
> >> +            result->params |= ipa::RPi::ConfigSensorParams;
> >>              result->sensorConfig.gainDelay = gainDelay;
> >>              result->sensorConfig.exposureDelay = exposureDelay;
> >>              result->sensorConfig.vblank = exposureDelay;
> >> @@ -360,14 +360,14 @@ void IPARPi::configure(const CameraSensorInfo
> &sensorInfo,
> >>      if (ipaConfig.lsTableHandle.isValid()) {
> >>              /* Remove any previous table, if there was one. */
> >>              if (lsTable_) {
> >> -                    munmap(lsTable_, ipa::rpi::MaxLsGridSize);
> >> +                    munmap(lsTable_, ipa::RPi::MaxLsGridSize);
> >>                      lsTable_ = nullptr;
> >>              }
> >>
> >>              /* Map the LS table buffer into user space. */
> >>              lsTableHandle_ = std::move(ipaConfig.lsTableHandle);
> >>              if (lsTableHandle_.isValid()) {
> >> -                    lsTable_ = mmap(nullptr, ipa::rpi::MaxLsGridSize,
> PROT_READ | PROT_WRITE,
> >> +                    lsTable_ = mmap(nullptr, ipa::RPi::MaxLsGridSize,
> PROT_READ | PROT_WRITE,
> >>                                      MAP_SHARED, lsTableHandle_.fd(),
> 0);
> >>
> >>                      if (lsTable_ == MAP_FAILED) {
> >> @@ -432,7 +432,7 @@ void IPARPi::signalStatReady(uint32_t bufferId)
> >>
> >>      reportMetadata();
> >>
> >> -    statsMetadataComplete.emit(bufferId & ipa::rpi::MaskID,
> libcameraMetadata_);
> >> +    statsMetadataComplete.emit(bufferId & ipa::RPi::MaskID,
> libcameraMetadata_);
> >>  }
> >>
> >>  void IPARPi::signalQueueRequest(const ControlList &controls)
> >> @@ -440,7 +440,7 @@ void IPARPi::signalQueueRequest(const ControlList
> &controls)
> >>      queueRequest(controls);
> >>  }
> >>
> >> -void IPARPi::signalIspPrepare(const ipa::rpi::ISPConfig &data)
> >> +void IPARPi::signalIspPrepare(const ipa::RPi::ISPConfig &data)
> >>  {
> >>      /*
> >>       * At start-up, or after a mode-switch, we may want to
> >> @@ -451,7 +451,7 @@ void IPARPi::signalIspPrepare(const
> ipa::rpi::ISPConfig &data)
> >>      frameCount_++;
> >>
> >>      /* Ready to push the input buffer into the ISP. */
> >> -    runIsp.emit(data.bayerBufferId & ipa::rpi::MaskID);
> >> +    runIsp.emit(data.bayerBufferId & ipa::RPi::MaskID);
> >>  }
> >>
> >>  void IPARPi::reportMetadata()
> >> @@ -906,7 +906,7 @@ void IPARPi::queueRequest(const ControlList
> &controls)
> >>
> >>  void IPARPi::returnEmbeddedBuffer(unsigned int bufferId)
> >>  {
> >> -    embeddedComplete.emit(bufferId & ipa::rpi::MaskID);
> >> +    embeddedComplete.emit(bufferId & ipa::RPi::MaskID);
> >>  }
> >>
> >>  void IPARPi::prepareISP(unsigned int bufferId)
> >> @@ -1271,7 +1271,7 @@ void IPARPi::applyLS(const struct AlscStatus
> *lsStatus, ControlList &ctrls)
> >>              .gain_format = GAIN_FORMAT_U4P10
> >>      };
> >>
> >> -    if (!lsTable_ || w * h * 4 * sizeof(uint16_t) >
> ipa::rpi::MaxLsGridSize) {
> >> +    if (!lsTable_ || w * h * 4 * sizeof(uint16_t) >
> ipa::RPi::MaxLsGridSize) {
> >>              LOG(IPARPI, Error) << "Do not have a correctly allocate
> lens shading table!";
> >>              return;
> >>      }
> >> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> >> index 8770ae66a21a..1a0bd6275578 100644
> >> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> >> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> >> @@ -166,7 +166,7 @@ public:
> >>      void handleState();
> >>      void applyScalerCrop(const ControlList &controls);
> >>
> >> -    std::unique_ptr<ipa::rpi::IPAProxyRPi> ipa_;
> >> +    std::unique_ptr<ipa::RPi::IPAProxyRPi> ipa_;
> >>
> >>      std::unique_ptr<CameraSensor> sensor_;
> >>      /* Array of Unicam and ISP device streams and associated
> buffers/streams. */
> >> @@ -778,8 +778,8 @@ int PipelineHandlerRPi::start(Camera *camera,
> ControlList *controls)
> >>              data->applyScalerCrop(*controls);
> >>
> >>      /* Start the IPA. */
> >> -    ipa::rpi::StartControls ipaData;
> >> -    ipa::rpi::StartControls result;
> >> +    ipa::RPi::StartControls ipaData;
> >> +    ipa::RPi::StartControls result;
> >>      if (controls)
> >>              ipaData.controls = *controls;
> >>      data->ipa_->start(ipaData, &result);
> >> @@ -1114,8 +1114,8 @@ int PipelineHandlerRPi::prepareBuffers(Camera
> *camera)
> >>       * Pass the stats and embedded data buffers to the IPA. No other
> >>       * buffers need to be passed.
> >>       */
> >> -    mapBuffers(camera, data->isp_[Isp::Stats].getBuffers(),
> ipa::rpi::MaskStats);
> >> -    mapBuffers(camera, data->unicam_[Unicam::Embedded].getBuffers(),
> ipa::rpi::MaskEmbeddedData);
> >> +    mapBuffers(camera, data->isp_[Isp::Stats].getBuffers(),
> ipa::RPi::MaskStats);
> >> +    mapBuffers(camera, data->unicam_[Unicam::Embedded].getBuffers(),
> ipa::RPi::MaskEmbeddedData);
> >>
> >>      return 0;
> >>  }
> >> @@ -1164,7 +1164,7 @@ void RPiCameraData::frameStarted(uint32_t
> sequence)
> >>
> >>  int RPiCameraData::loadIPA()
> >>  {
> >> -    ipa_ = IPAManager::createIPA<ipa::rpi::IPAProxyRPi>(pipe_, 1, 1);
> >> +    ipa_ = IPAManager::createIPA<ipa::RPi::IPAProxyRPi>(pipe_, 1, 1);
> >>
> >>      if (!ipa_)
> >>              return -ENOENT;
> >> @@ -1188,7 +1188,7 @@ int RPiCameraData::configureIPA(const
> CameraConfiguration *config)
> >>
> >>      std::map<unsigned int, IPAStream> streamConfig;
> >>      std::map<unsigned int, ControlInfoMap> entityControls;
> >> -    ipa::rpi::ConfigInput ipaConfig;
> >> +    ipa::RPi::ConfigInput ipaConfig;
> >>
> >>      /* Get the device format to pass to the IPA. */
> >>      V4L2DeviceFormat sensorFormat;
> >> @@ -1211,7 +1211,7 @@ int RPiCameraData::configureIPA(const
> CameraConfiguration *config)
> >>
> >>      /* Allocate the lens shading table via dmaHeap and pass to the
> IPA. */
> >>      if (!lsTable_.isValid()) {
> >> -            lsTable_ = dmaHeap_.alloc("ls_grid",
> ipa::rpi::MaxLsGridSize);
> >> +            lsTable_ = dmaHeap_.alloc("ls_grid",
> ipa::RPi::MaxLsGridSize);
> >>              if (!lsTable_.isValid())
> >>                      return -ENOMEM;
> >>
> >> @@ -1231,7 +1231,7 @@ int RPiCameraData::configureIPA(const
> CameraConfiguration *config)
> >>      }
> >>
> >>      /* Ready the IPA - it must know about the sensor resolution. */
> >> -    ipa::rpi::ConfigOutput result;
> >> +    ipa::RPi::ConfigOutput result;
> >>
> >>      ipa_->configure(sensorInfo_, streamConfig, entityControls,
> ipaConfig,
> >>                      &result, &ret);
> >> @@ -1241,7 +1241,7 @@ int RPiCameraData::configureIPA(const
> CameraConfiguration *config)
> >>              return -EPIPE;
> >>      }
> >>
> >> -    if (result.params & ipa::rpi::ConfigSensorParams) {
> >> +    if (result.params & ipa::RPi::ConfigSensorParams) {
> >>              /*
> >>               * Setup our delayed control writer with the sensor default
> >>               * gain and exposure delays.
> >> @@ -1457,7 +1457,7 @@ void RPiCameraData::ispOutputDequeue(FrameBuffer
> *buffer)
> >>       * application until after the IPA signals so.
> >>       */
> >>      if (stream == &isp_[Isp::Stats]) {
> >> -            ipa_->signalStatReady(ipa::rpi::MaskStats |
> static_cast<unsigned int>(index));
> >> +            ipa_->signalStatReady(ipa::RPi::MaskStats |
> static_cast<unsigned int>(index));
> >>      } else {
> >>              /* Any other ISP output can be handed back to the
> application now. */
> >>              handleStreamBuffer(buffer, stream);
> >> @@ -1561,7 +1561,7 @@ void
> RPiCameraData::handleExternalBuffer(FrameBuffer *buffer, RPi::Stream *strea
> >>  {
> >>      unsigned int id = stream->getBufferId(buffer);
> >>
> >> -    if (!(id & ipa::rpi::MaskExternalBuffer))
> >> +    if (!(id & ipa::RPi::MaskExternalBuffer))
> >>              return;
> >>
> >>      /* Stop the Stream object from tracking the buffer. */
> >> @@ -1693,9 +1693,9 @@ void RPiCameraData::tryRunPipeline()
> >>                      << " Bayer buffer id: " << bayerId
> >>                      << " Embedded buffer id: " << embeddedId;
> >>
> >> -    ipa::rpi::ISPConfig ispPrepare;
> >> -    ispPrepare.embeddedBufferId = ipa::rpi::MaskEmbeddedData |
> embeddedId;
> >> -    ispPrepare.bayerBufferId = ipa::rpi::MaskBayerData | bayerId;
> >> +    ipa::RPi::ISPConfig ispPrepare;
> >> +    ispPrepare.embeddedBufferId = ipa::RPi::MaskEmbeddedData |
> embeddedId;
> >> +    ispPrepare.bayerBufferId = ipa::RPi::MaskBayerData | bayerId;
> >>      ipa_->signalIspPrepare(ispPrepare);
> >>  }
> >>
> >> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> >> index 496dd36fabbc..f2430415d32d 100644
> >> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> >> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> >> @@ -72,7 +72,7 @@ int Stream::getBufferId(FrameBuffer *buffer) const
> >>
> >>  void Stream::setExternalBuffer(FrameBuffer *buffer)
> >>  {
> >> -    bufferMap_.emplace(ipa::rpi::MaskExternalBuffer | id_.get(),
> buffer);
> >> +    bufferMap_.emplace(ipa::RPi::MaskExternalBuffer | id_.get(),
> buffer);
> >>  }
> >>
> >>  void Stream::removeExternalBuffer(FrameBuffer *buffer)
> >> @@ -80,7 +80,7 @@ void Stream::removeExternalBuffer(FrameBuffer *buffer)
> >>      int id = getBufferId(buffer);
> >>
> >>      /* Ensure we have this buffer in the stream, and it is marked
> external. */
> >> -    ASSERT(id != -1 && (id & ipa::rpi::MaskExternalBuffer));
> >> +    ASSERT(id != -1 && (id & ipa::RPi::MaskExternalBuffer));
> >>      bufferMap_.erase(id);
> >>  }
> >>
> >> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> b/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> >> index 701110d04bdb..f1ac715f4221 100644
> >> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> >> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> >> @@ -32,13 +32,13 @@ class Stream : public libcamera::Stream
> >>  {
> >>  public:
> >>      Stream()
> >> -            : id_(ipa::rpi::MaskID)
> >> +            : id_(ipa::RPi::MaskID)
> >>      {
> >>      }
> >>
> >>      Stream(const char *name, MediaEntity *dev, bool importOnly = false)
> >>              : external_(false), importOnly_(importOnly), name_(name),
> >> -              dev_(std::make_unique<V4L2VideoDevice>(dev)),
> id_(ipa::rpi::MaskID)
> >> +              dev_(std::make_unique<V4L2VideoDevice>(dev)),
> id_(ipa::RPi::MaskID)
> >>      {
> >>      }
> >>
> >>
> >
>
> --
> Regards
> --
> Kieran
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210218/3407fc3d/attachment-0001.htm>


More information about the libcamera-devel mailing list