<div dir="ltr"><div dir="ltr">Hi Paul,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 17 Feb 2021 at 10:24, <<a href="mailto:paul.elder@ideasonboard.com">paul.elder@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Naush,<br>
<br>
On Wed, Feb 17, 2021 at 10:08:51AM +0000, Naushir Patuck wrote:<br>
> Rename the IPA interface namespace to ipa::RPi for consistency with<br>
> the libcamera::RPi namespace label.<br>
> <br>
> There is no functional change in this commit.<br>
> <br>
> Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> ---<br>
> include/libcamera/ipa/raspberrypi.mojom | 2 +-<br>
> src/ipa/raspberrypi/raspberrypi.cpp | 38 +++++++++---------<br>
> .../pipeline/raspberrypi/raspberrypi.cpp | 40 +++++++++----------<br>
> .../pipeline/raspberrypi/rpi_stream.cpp | 4 +-<br>
> .../pipeline/raspberrypi/rpi_stream.h | 4 +-<br>
> 5 files changed, 42 insertions(+), 46 deletions(-)<br>
> <br>
> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom<br>
> index 9c05cc68cceb..5a27b1e4fc2d 100644<br>
> --- a/include/libcamera/ipa/raspberrypi.mojom<br>
> +++ b/include/libcamera/ipa/raspberrypi.mojom<br>
> @@ -1,6 +1,6 @@<br>
> /* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
> <br>
> -module ipa.rpi;<br>
> +module ipa.RPi;<br>
> <br>
> import "include/libcamera/ipa/core.mojom";<br>
> <br>
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp<br>
> index 974f4ec63058..1226ea514521 100644<br>
> --- a/src/ipa/raspberrypi/raspberrypi.cpp<br>
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp<br>
> @@ -63,7 +63,7 @@ constexpr double defaultMaxFrameDuration = 1e6 / 0.01;<br>
> <br>
> LOG_DEFINE_CATEGORY(IPARPI)<br>
> <br>
> -class IPARPi : public ipa::rpi::IPARPiInterface<br>
> +class IPARPi : public ipa::RPi::IPARPiInterface<br>
> {<br>
> public:<br>
> IPARPi()<br>
> @@ -76,24 +76,24 @@ public:<br>
> ~IPARPi()<br>
> {<br>
> if (lsTable_)<br>
> - munmap(lsTable_, ipa::rpi::MaxLsGridSize);<br>
> + munmap(lsTable_, ipa::RPi::MaxLsGridSize);<br>
> }<br>
> <br>
> int init(const IPASettings &settings) override;<br>
> - void start(const ipa::rpi::StartControls &data,<br>
> - ipa::rpi::StartControls *result) override;<br>
> + void start(const ipa::RPi::StartControls &data,<br>
> + ipa::RPi::StartControls *result) override;<br>
> void stop() override {}<br>
> <br>
> void configure(const CameraSensorInfo &sensorInfo,<br>
> const std::map<unsigned int, IPAStream> &streamConfig,<br>
> const std::map<unsigned int, ControlInfoMap> &entityControls,<br>
> - const ipa::rpi::ConfigInput &data,<br>
> - ipa::rpi::ConfigOutput *response, int32_t *ret) override;<br>
> + const ipa::RPi::ConfigInput &data,<br>
> + ipa::RPi::ConfigOutput *response, int32_t *ret) override;<br>
> void mapBuffers(const std::vector<IPABuffer> &buffers) override;<br>
> void unmapBuffers(const std::vector<unsigned int> &ids) override;<br>
> void signalStatReady(const uint32_t bufferId) override;<br>
> void signalQueueRequest(const ControlList &controls) override;<br>
> - void signalIspPrepare(const ipa::rpi::ISPConfig &data) override;<br>
> + void signalIspPrepare(const ipa::RPi::ISPConfig &data) override;<br>
> <br>
> private:<br>
> void setMode(const CameraSensorInfo &sensorInfo);<br>
> @@ -168,8 +168,8 @@ int IPARPi::init(const IPASettings &settings)<br>
> return 0;<br>
> }<br>
> <br>
> -void IPARPi::start(const ipa::rpi::StartControls &data,<br>
> - ipa::rpi::StartControls *result)<br>
> +void IPARPi::start(const ipa::RPi::StartControls &data,<br>
> + ipa::RPi::StartControls *result)<br>
> {<br>
> RPiController::Metadata metadata;<br>
> <br>
> @@ -291,8 +291,8 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo)<br>
> void IPARPi::configure(const CameraSensorInfo &sensorInfo,<br>
> [[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig,<br>
> const std::map<unsigned int, ControlInfoMap> &entityControls,<br>
> - const ipa::rpi::ConfigInput &ipaConfig,<br>
> - ipa::rpi::ConfigOutput *result, int32_t *ret)<br>
> + const ipa::RPi::ConfigInput &ipaConfig,<br>
> + ipa::RPi::ConfigOutput *result, int32_t *ret)<br>
> {<br>
> if (entityControls.size() != 2) {<br>
> LOG(IPARPI, Error) << "No ISP or sensor controls found.";<br>
> @@ -344,7 +344,7 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,<br>
> helper_->GetDelays(exposureDelay, gainDelay);<br>
> sensorMetadata = helper_->SensorEmbeddedDataPresent();<br>
> <br>
> - result->params |= ipa::rpi::ConfigSensorParams;<br>
> + result->params |= ipa::RPi::ConfigSensorParams;<br>
> result->sensorConfig.gainDelay = gainDelay;<br>
> result->sensorConfig.exposureDelay = exposureDelay;<br>
> result->sensorConfig.vblank = exposureDelay;<br>
> @@ -360,14 +360,14 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,<br>
> if (ipaConfig.lsTableHandle.isValid()) {<br>
> /* Remove any previous table, if there was one. */<br>
> if (lsTable_) {<br>
> - munmap(lsTable_, ipa::rpi::MaxLsGridSize);<br>
> + munmap(lsTable_, ipa::RPi::MaxLsGridSize);<br>
> lsTable_ = nullptr;<br>
> }<br>
> <br>
> /* Map the LS table buffer into user space. */<br>
> lsTableHandle_ = std::move(ipaConfig.lsTableHandle);<br>
> if (lsTableHandle_.isValid()) {<br>
> - lsTable_ = mmap(nullptr, ipa::rpi::MaxLsGridSize, PROT_READ | PROT_WRITE,<br>
> + lsTable_ = mmap(nullptr, ipa::RPi::MaxLsGridSize, PROT_READ | PROT_WRITE,<br>
> MAP_SHARED, lsTableHandle_.fd(), 0);<br>
> <br>
> if (lsTable_ == MAP_FAILED) {<br>
> @@ -432,7 +432,7 @@ void IPARPi::signalStatReady(uint32_t bufferId)<br>
> <br>
> reportMetadata();<br>
> <br>
> - statsMetadataComplete.emit(bufferId & ipa::rpi::MaskID, libcameraMetadata_);<br>
> + statsMetadataComplete.emit(bufferId & ipa::RPi::MaskID, libcameraMetadata_);<br>
> }<br>
> <br>
> void IPARPi::signalQueueRequest(const ControlList &controls)<br>
> @@ -440,7 +440,7 @@ void IPARPi::signalQueueRequest(const ControlList &controls)<br>
> queueRequest(controls);<br>
> }<br>
> <br>
> -void IPARPi::signalIspPrepare(const ipa::rpi::ISPConfig &data)<br>
> +void IPARPi::signalIspPrepare(const ipa::RPi::ISPConfig &data)<br>
> {<br>
> /*<br>
> * At start-up, or after a mode-switch, we may want to<br>
> @@ -451,7 +451,7 @@ void IPARPi::signalIspPrepare(const ipa::rpi::ISPConfig &data)<br>
> frameCount_++;<br>
> <br>
> /* Ready to push the input buffer into the ISP. */<br>
> - runIsp.emit(data.bayerBufferId & ipa::rpi::MaskID);<br>
> + runIsp.emit(data.bayerBufferId & ipa::RPi::MaskID);<br>
> }<br>
> <br>
> void IPARPi::reportMetadata()<br>
> @@ -906,7 +906,7 @@ void IPARPi::queueRequest(const ControlList &controls)<br>
> <br>
> void IPARPi::returnEmbeddedBuffer(unsigned int bufferId)<br>
> {<br>
> - embeddedComplete.emit(bufferId & ipa::rpi::MaskID);<br>
> + embeddedComplete.emit(bufferId & ipa::RPi::MaskID);<br>
> }<br>
> <br>
> void IPARPi::prepareISP(unsigned int bufferId)<br>
> @@ -1271,7 +1271,7 @@ void IPARPi::applyLS(const struct AlscStatus *lsStatus, ControlList &ctrls)<br>
> .gain_format = GAIN_FORMAT_U4P10<br>
> };<br>
> <br>
> - if (!lsTable_ || w * h * 4 * sizeof(uint16_t) > ipa::rpi::MaxLsGridSize) {<br>
> + if (!lsTable_ || w * h * 4 * sizeof(uint16_t) > ipa::RPi::MaxLsGridSize) {<br>
> LOG(IPARPI, Error) << "Do not have a correctly allocate lens shading table!";<br>
> return;<br>
> }<br>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> index 8770ae66a21a..9dd4d112a907 100644<br>
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> @@ -166,7 +166,7 @@ public:<br>
> void handleState();<br>
> void applyScalerCrop(const ControlList &controls);<br>
> <br>
> - std::unique_ptr<ipa::rpi::IPAProxyRPi> ipa_;<br>
> + std::unique_ptr<ipa::RPi::IPAProxyRPi> ipa_;<br>
> <br>
> std::unique_ptr<CameraSensor> sensor_;<br>
> /* Array of Unicam and ISP device streams and associated buffers/streams. */<br>
> @@ -778,8 +778,8 @@ int PipelineHandlerRPi::start(Camera *camera, ControlList *controls)<br>
> data->applyScalerCrop(*controls);<br>
> <br>
> /* Start the IPA. */<br>
> - ipa::rpi::StartControls ipaData;<br>
> - ipa::rpi::StartControls result;<br>
> + ipa::RPi::StartControls ipaData;<br>
> + ipa::RPi::StartControls result;<br>
> if (controls)<br>
> ipaData.controls = *controls;<br>
> data->ipa_->start(ipaData, &result);<br>
> @@ -1114,8 +1114,8 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)<br>
> * Pass the stats and embedded data buffers to the IPA. No other<br>
> * buffers need to be passed.<br>
> */<br>
> - mapBuffers(camera, data->isp_[Isp::Stats].getBuffers(), ipa::rpi::MaskStats);<br>
> - mapBuffers(camera, data->unicam_[Unicam::Embedded].getBuffers(), ipa::rpi::MaskEmbeddedData);<br>
> + mapBuffers(camera, data->isp_[Isp::Stats].getBuffers(), ipa::RPi::MaskStats);<br>
> + mapBuffers(camera, data->unicam_[Unicam::Embedded].getBuffers(), ipa::RPi::MaskEmbeddedData);<br>
> <br>
> return 0;<br>
> }<br>
> @@ -1164,7 +1164,7 @@ void RPiCameraData::frameStarted(uint32_t sequence)<br>
> <br>
> int RPiCameraData::loadIPA()<br>
> {<br>
> - ipa_ = IPAManager::createIPA<ipa::rpi::IPAProxyRPi>(pipe_, 1, 1);<br>
> + ipa_ = IPAManager::createIPA<ipa::RPi::IPAProxyRPi>(pipe_, 1, 1);<br>
> <br>
> if (!ipa_)<br>
> return -ENOENT;<br>
> @@ -1188,7 +1188,7 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)<br>
> <br>
> std::map<unsigned int, IPAStream> streamConfig;<br>
> std::map<unsigned int, ControlInfoMap> entityControls;<br>
> - ipa::rpi::ConfigInput ipaConfig;<br>
> + ipa::RPi::ConfigInput ipaConfig;<br>
> <br>
> /* Get the device format to pass to the IPA. */<br>
> V4L2DeviceFormat sensorFormat;<br>
> @@ -1211,7 +1211,7 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)<br>
> <br>
> /* Allocate the lens shading table via dmaHeap and pass to the IPA. */<br>
> if (!lsTable_.isValid()) {<br>
> - lsTable_ = dmaHeap_.alloc("ls_grid", ipa::rpi::MaxLsGridSize);<br>
> + lsTable_ = dmaHeap_.alloc("ls_grid", ipa::RPi::MaxLsGridSize);<br>
> if (!lsTable_.isValid())<br>
> return -ENOMEM;<br>
> <br>
> @@ -1231,7 +1231,7 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)<br>
> }<br>
> <br>
> /* Ready the IPA - it must know about the sensor resolution. */<br>
> - ipa::rpi::ConfigOutput result;<br>
> + ipa::RPi::ConfigOutput result;<br>
> <br>
> ipa_->configure(sensorInfo_, streamConfig, entityControls, ipaConfig,<br>
> &result, &ret);<br>
> @@ -1241,7 +1241,7 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)<br>
> return -EPIPE;<br>
> }<br>
> <br>
> - if (result.params & ipa::rpi::ConfigSensorParams) {<br>
> + if (result.params & ipa::RPi::ConfigSensorParams) {<br>
> /*<br>
> * Setup our delayed control writer with the sensor default<br>
> * gain and exposure delays.<br>
> @@ -1343,13 +1343,9 @@ void RPiCameraData::setIspControls(const ControlList &controls)<br>
> if (ctrls.contains(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING)) {<br>
> Span<const uint8_t> s =<br>
> ctrls.get(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING).data();<br>
> - bcm2835_isp_lens_shading ls =<br>
> - *reinterpret_cast<const bcm2835_isp_lens_shading *>(s.data());<br>
> - ls.dmabuf = lsTable_.fd();<br>
> -<br>
> - ControlValue c(Span<const uint8_t>{ reinterpret_cast<uint8_t *>(&ls),<br>
> - sizeof(ls) });<br>
> - ctrls.set(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING, c);<br>
> + const bcm2835_isp_lens_shading *ls =<br>
> + reinterpret_cast<const bcm2835_isp_lens_shading *>(s.data());<br>
> + ls->dmabuf = lsTable_.fd();<br>
<br>
I don't quite understand this hunk. It doesn't seem to match what the<br>
rest of this patch does, and it's not the final product of the next<br>
patch (which is what seems to be its purpose).<br></blockquote><div><br></div><div>Apologies, this was an error, it should be in the next patch. I will fix and post an update.</div><div><br></div><div>Naush</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<br>
Paul<br>
<br>
> }<br>
> <br>
> isp_[Isp::Input].dev()->setControls(&ctrls);<br>
> @@ -1457,7 +1453,7 @@ void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)<br>
> * application until after the IPA signals so.<br>
> */<br>
> if (stream == &isp_[Isp::Stats]) {<br>
> - ipa_->signalStatReady(ipa::rpi::MaskStats | static_cast<unsigned int>(index));<br>
> + ipa_->signalStatReady(ipa::RPi::MaskStats | static_cast<unsigned int>(index));<br>
> } else {<br>
> /* Any other ISP output can be handed back to the application now. */<br>
> handleStreamBuffer(buffer, stream);<br>
> @@ -1561,7 +1557,7 @@ void RPiCameraData::handleExternalBuffer(FrameBuffer *buffer, RPi::Stream *strea<br>
> {<br>
> unsigned int id = stream->getBufferId(buffer);<br>
> <br>
> - if (!(id & ipa::rpi::MaskExternalBuffer))<br>
> + if (!(id & ipa::RPi::MaskExternalBuffer))<br>
> return;<br>
> <br>
> /* Stop the Stream object from tracking the buffer. */<br>
> @@ -1693,9 +1689,9 @@ void RPiCameraData::tryRunPipeline()<br>
> << " Bayer buffer id: " << bayerId<br>
> << " Embedded buffer id: " << embeddedId;<br>
> <br>
> - ipa::rpi::ISPConfig ispPrepare;<br>
> - ispPrepare.embeddedBufferId = ipa::rpi::MaskEmbeddedData | embeddedId;<br>
> - ispPrepare.bayerBufferId = ipa::rpi::MaskBayerData | bayerId;<br>
> + ipa::RPi::ISPConfig ispPrepare;<br>
> + ispPrepare.embeddedBufferId = ipa::RPi::MaskEmbeddedData | embeddedId;<br>
> + ispPrepare.bayerBufferId = ipa::RPi::MaskBayerData | bayerId;<br>
> ipa_->signalIspPrepare(ispPrepare);<br>
> }<br>
> <br>
> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp<br>
> index 496dd36fabbc..f2430415d32d 100644<br>
> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp<br>
> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp<br>
> @@ -72,7 +72,7 @@ int Stream::getBufferId(FrameBuffer *buffer) const<br>
> <br>
> void Stream::setExternalBuffer(FrameBuffer *buffer)<br>
> {<br>
> - bufferMap_.emplace(ipa::rpi::MaskExternalBuffer | id_.get(), buffer);<br>
> + bufferMap_.emplace(ipa::RPi::MaskExternalBuffer | id_.get(), buffer);<br>
> }<br>
> <br>
> void Stream::removeExternalBuffer(FrameBuffer *buffer)<br>
> @@ -80,7 +80,7 @@ void Stream::removeExternalBuffer(FrameBuffer *buffer)<br>
> int id = getBufferId(buffer);<br>
> <br>
> /* Ensure we have this buffer in the stream, and it is marked external. */<br>
> - ASSERT(id != -1 && (id & ipa::rpi::MaskExternalBuffer));<br>
> + ASSERT(id != -1 && (id & ipa::RPi::MaskExternalBuffer));<br>
> bufferMap_.erase(id);<br>
> }<br>
> <br>
> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h b/src/libcamera/pipeline/raspberrypi/rpi_stream.h<br>
> index 701110d04bdb..f1ac715f4221 100644<br>
> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h<br>
> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h<br>
> @@ -32,13 +32,13 @@ class Stream : public libcamera::Stream<br>
> {<br>
> public:<br>
> Stream()<br>
> - : id_(ipa::rpi::MaskID)<br>
> + : id_(ipa::RPi::MaskID)<br>
> {<br>
> }<br>
> <br>
> Stream(const char *name, MediaEntity *dev, bool importOnly = false)<br>
> : external_(false), importOnly_(importOnly), name_(name),<br>
> - dev_(std::make_unique<V4L2VideoDevice>(dev)), id_(ipa::rpi::MaskID)<br>
> + dev_(std::make_unique<V4L2VideoDevice>(dev)), id_(ipa::RPi::MaskID)<br>
> {<br>
> }<br>
> <br>
> -- <br>
> 2.25.1<br>
> <br>
> _______________________________________________<br>
> libcamera-devel mailing list<br>
> <a href="mailto:libcamera-devel@lists.libcamera.org" target="_blank">libcamera-devel@lists.libcamera.org</a><br>
> <a href="https://lists.libcamera.org/listinfo/libcamera-devel" rel="noreferrer" target="_blank">https://lists.libcamera.org/listinfo/libcamera-devel</a><br>
</blockquote></div></div>