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

Kieran Bingham kieran.bingham at ideasonboard.com
Thu Feb 18 12:16:40 CET 2021



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

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


More information about the libcamera-devel mailing list