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

paul.elder at ideasonboard.com paul.elder at ideasonboard.com
Wed Feb 17 11:24:49 CET 2021


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


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


More information about the libcamera-devel mailing list