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

Naushir Patuck naush at raspberrypi.com
Wed Feb 17 11:26:59 CET 2021


Hi Paul,

On Wed, 17 Feb 2021 at 10:24, <paul.elder at ideasonboard.com> wrote:

> Hi Naush,
>
> On Wed, Feb 17, 2021 at 10:08:51AM +0000, 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.
> >
> > 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      | 40 +++++++++----------
> >  .../pipeline/raspberrypi/rpi_stream.cpp       |  4 +-
> >  .../pipeline/raspberrypi/rpi_stream.h         |  4 +-
> >  5 files changed, 42 insertions(+), 46 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..9dd4d112a907 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.
> > @@ -1343,13 +1343,9 @@ void RPiCameraData::setIspControls(const
> ControlList &controls)
> >       if (ctrls.contains(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING)) {
> >               Span<const uint8_t> s =
> >
>  ctrls.get(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING).data();
> > -             bcm2835_isp_lens_shading ls =
> > -                     *reinterpret_cast<const bcm2835_isp_lens_shading
> *>(s.data());
> > -             ls.dmabuf = lsTable_.fd();
> > -
> > -             ControlValue c(Span<const uint8_t>{
> reinterpret_cast<uint8_t *>(&ls),
> > -                                                 sizeof(ls) });
> > -             ctrls.set(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING, c);
> > +             const bcm2835_isp_lens_shading *ls =
> > +                     reinterpret_cast<const bcm2835_isp_lens_shading
> *>(s.data());
> > +             ls->dmabuf = lsTable_.fd();
>
> I don't quite understand this hunk. It doesn't seem to match what the
> rest of this patch does, and it's not the final product of the next
> patch (which is what seems to be its purpose).
>

Apologies, this was an error, it should be in the next patch.  I will fix
and post an update.

Naush


>
>
> Paul
>
> >       }
> >
> >       isp_[Isp::Input].dev()->setControls(&ctrls);
> > @@ -1457,7 +1453,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 +1557,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 +1689,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)
> >       {
> >       }
> >
> > --
> > 2.25.1
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210217/5d15612a/attachment-0001.htm>


More information about the libcamera-devel mailing list