[libcamera-devel] [PATCH] ipa: ipu3: Move the IPA to the ipa::ipu3 namespace

[Umang Jain]

On 4/17/21 3:55 AM, Kieran Bingham wrote:
> Hi Laurent,
>
> On 16/04/2021 23:14, Laurent Pinchart wrote:
>> Hi Kieran,
>>
>> Thank you for the patch.
>>
>> On Fri, Apr 16, 2021 at 05:04:11PM +0100, Kieran Bingham wrote:
>>> Simplify name-spacing of the IPU3 components by placing it in the
>>> ipa::ipu3 namespace directly.
>>>
>>> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>>> ---
>>>   src/ipa/ipu3/ipu3.cpp | 34 +++++++++++++++++++++-------------
>>>   1 file changed, 21 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
>>> index 34a907f23ef5..edd325555b51 100644
>>> --- a/src/ipa/ipu3/ipu3.cpp
>>> +++ b/src/ipa/ipu3/ipu3.cpp
>>> @@ -25,7 +25,11 @@ namespace libcamera {
>>>   
>>>   LOG_DEFINE_CATEGORY(IPAIPU3)
>>>   
>>> -class IPAIPU3 : public ipa::ipu3::IPAIPU3Interface
>>> +namespace ipa {
>>> +
>>> +namespace ipu3 {
>> I think you can write this
>>
>> namespace ipa::ipu3 {
>>
> Ah yes, and I've tried to move towards that in other (internal) places
> too, so I've missed this one.
>
> I think it's better, so I'll update it.
yes, I was about to ask you the namespacing style is going a bit 
different for our different components. But it's already noticed, cool

Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
> --
> Kieran
>
>
>> in C++17.
>>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>>
>>> +
>>> +class IPAIPU3 : public IPAIPU3Interface
>>>   {
>>>   public:
>>>   	int init([[maybe_unused]] const IPASettings &settings) override
>>> @@ -40,7 +44,7 @@ public:
>>>   
>>>   	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
>>>   	void unmapBuffers(const std::vector<unsigned int> &ids) override;
>>> -	void processEvent(const ipa::ipu3::IPU3Event &event) override;
>>> +	void processEvent(const IPU3Event &event) override;
>>>   
>>>   private:
>>>   	void processControls(unsigned int frame, const ControlList &controls);
>>> @@ -119,14 +123,14 @@ void IPAIPU3::unmapBuffers(const std::vector<unsigned int> &ids)
>>>   	}
>>>   }
>>>   
>>> -void IPAIPU3::processEvent(const ipa::ipu3::IPU3Event &event)
>>> +void IPAIPU3::processEvent(const IPU3Event &event)
>>>   {
>>>   	switch (event.op) {
>>> -	case ipa::ipu3::EventProcessControls: {
>>> +	case EventProcessControls: {
>>>   		processControls(event.frame, event.controls);
>>>   		break;
>>>   	}
>>> -	case ipa::ipu3::EventStatReady: {
>>> +	case EventStatReady: {
>>>   		auto it = buffers_.find(event.bufferId);
>>>   		if (it == buffers_.end()) {
>>>   			LOG(IPAIPU3, Error) << "Could not find stats buffer!";
>>> @@ -140,7 +144,7 @@ void IPAIPU3::processEvent(const ipa::ipu3::IPU3Event &event)
>>>   		parseStatistics(event.frame, stats);
>>>   		break;
>>>   	}
>>> -	case ipa::ipu3::EventFillParams: {
>>> +	case EventFillParams: {
>>>   		auto it = buffers_.find(event.bufferId);
>>>   		if (it == buffers_.end()) {
>>>   			LOG(IPAIPU3, Error) << "Could not find param buffer!";
>>> @@ -173,8 +177,8 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)
>>>   
>>>   	/* \todo Fill in parameters buffer. */
>>>   
>>> -	ipa::ipu3::IPU3Action op;
>>> -	op.op = ipa::ipu3::ActionParamFilled;
>>> +	IPU3Action op;
>>> +	op.op = ActionParamFilled;
>>>   
>>>   	queueFrameAction.emit(frame, op);
>>>   }
>>> @@ -187,8 +191,8 @@ void IPAIPU3::parseStatistics(unsigned int frame,
>>>   	/* \todo React to statistics and update internal state machine. */
>>>   	/* \todo Add meta-data information to ctrls. */
>>>   
>>> -	ipa::ipu3::IPU3Action op;
>>> -	op.op = ipa::ipu3::ActionMetadataReady;
>>> +	IPU3Action op;
>>> +	op.op = ActionMetadataReady;
>>>   	op.controls = ctrls;
>>>   
>>>   	queueFrameAction.emit(frame, op);
>>> @@ -196,8 +200,8 @@ void IPAIPU3::parseStatistics(unsigned int frame,
>>>   
>>>   void IPAIPU3::setControls(unsigned int frame)
>>>   {
>>> -	ipa::ipu3::IPU3Action op;
>>> -	op.op = ipa::ipu3::ActionSetSensorControls;
>>> +	IPU3Action op;
>>> +	op.op = ActionSetSensorControls;
>>>   
>>>   	ControlList ctrls(ctrls_);
>>>   	ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_));
>>> @@ -207,6 +211,10 @@ void IPAIPU3::setControls(unsigned int frame)
>>>   	queueFrameAction.emit(frame, op);
>>>   }
>>>   
>>> +} /* namespace ipu3 */
>>> +
>>> +} /* namespace ipa */
>>> +
>>>   /*
>>>    * External IPA module interface
>>>    */
>>> @@ -221,7 +229,7 @@ const struct IPAModuleInfo ipaModuleInfo = {
>>>   
>>>   IPAInterface *ipaCreate()
>>>   {
>>> -	return new IPAIPU3();
>>> +	return new ipa::ipu3::IPAIPU3();
>>>   }
>>>   }
>>>