[libcamera-devel] [PATCH v4 17/19] ipa: ipu3: Implement an empty stop() function

[Umang Jain]

Hi JM.

On 10/26/21 4:00 PM, Jean-Michel Hautbois wrote:
> Hi Umang,
>
> On 26/10/2021 12:13, Umang Jain wrote:
>> Hi JM,
>>
>> On 10/26/21 3:25 PM, Jean-Michel Hautbois wrote:
>>> While the stop() function does not currently perform any action, it 
>>> forms
>>> part of the IPA interface and is a public function in the class.
>>>
>>> Promote it to a full (but basic) function implementation and begin the
>>> documentation accordingly so that there is an appropriate stub to
>>> perform stop operations if they come up.
>>>
>>> Signed-off-by: Jean-Michel Hautbois 
>>> <jeanmichel.hautbois at ideasonboard.com>
>>> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>>> ---
>>>   src/ipa/ipu3/ipu3.cpp | 9 ++++++++-
>>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
>>> index a10fdd4a..5c51607d 100644
>>> --- a/src/ipa/ipu3/ipu3.cpp
>>> +++ b/src/ipa/ipu3/ipu3.cpp
>>> @@ -135,7 +135,7 @@ public:
>>>            ControlInfoMap *ipaControls) override;
>>>       int start() override;
>>> -    void stop() override {}
>>> +    void stop() override;
>>>       int configure(const IPAConfigInfo &configInfo,
>>>                 ControlInfoMap *ipaControls) override;
>>> @@ -323,6 +323,13 @@ int IPAIPU3::start()
>>>       return 0;
>>>   }
>>> +/**
>>> + * \brief Ensure that all processing has completed
>>
>>
>> This sounds less like a \brief but more like a \todo. Since, it's a 
>> stub function, I am not sure how well we can document it as of now.
>
> Indeed, but this is a reference documentation, so we are telling the 
> purpose of the function, even if not implemented yet or empty.


Ok, sounds okay to me then

Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>

>
>>
>> Do we have some "stopping" criteria which needs to be satisfied to 
>> ensure we have stopped the IPA?
>
> Right now, no, but in the future, maybe (not really thought about it yet)
>
>>
>>> + */
>>> +void IPAIPU3::stop()
>>> +{
>>> +}
>>> +
>>>   /**
>>>    * \brief Calculate a grid for the AWB statistics
>>>    *