[libcamera-devel] [PATCH 1/2] libtest: camera_test: Plumb constructor to set LIBCAMERA_IPA_FORCE_ISOLATION

[Umang Jain]

Hi Kieran

On 8/17/21 6:14 PM, Kieran Bingham wrote:
> On 17/08/2021 13:29, Kieran Bingham wrote:
>> Hi Umang,
>>
>> On 17/08/2021 13:26, Umang Jain wrote:
>>> Some tests might require to have LIBCAMERA_IPA_FORCE_ISOLATION set
>>> to ensure they can test the IPA running in isolated mode. These tests
>>> are likely to leverage CameraTest. The environment variable should
>>> be set before CameraManager::start() call which happens in CameraTest's
>>> constructor. Hence, plumb the constructor with a flag so that the
>>> LIBCAMERA_IPA_FORCE_ISOLATION can be set before CameraManager::start().
>>>
>>> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
>>> ---
>>>   test/libtest/camera_test.cpp | 5 ++++-
>>>   test/libtest/camera_test.h   | 2 +-
>>>   2 files changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/test/libtest/camera_test.cpp b/test/libtest/camera_test.cpp
>>> index 2ae4d677..9cbc4d82 100644
>>> --- a/test/libtest/camera_test.cpp
>>> +++ b/test/libtest/camera_test.cpp
>>> @@ -13,10 +13,13 @@
>>>   using namespace libcamera;
>>>   using namespace std;
>>>   
>>> -CameraTest::CameraTest(const char *name)
>>> +CameraTest::CameraTest(const char *name, bool isolate)
> Reading this where it is used, I wonder if this flag should be an enum,
> but it's not essential - and more a question for debate.
>
> i.e.
>    CameraTest(kCamId_, ISOLATED_IPA)
>
>
> might be clearer than
>
>    CameraTest(kCamId_, true);
>
>
> Though the ISOLATED_IPA name is also debatable, and might end up being
> flags? But if it's only the one flag for now ... that's overkill anyway.


In process of writing this, I actually envisioned that a map of 
environment variables that can be passed in to CameraTest that needed to 
be setenv() before CameraManager::start()

However, my thought process was the same, that is overkill for one 
environment variable.


>
>
>>>   {
>>>   	cm_ = new CameraManager();
>>>   
>>> +	if (isolate)
>>> +		setenv("LIBCAMERA_IPA_FORCE_ISOLATION", "TRUE", 1);
>>> +
>> Ahh nice. Good so we can now isolate IPA's from tests!
>>
>> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>>
>>
>>
>>>   	if (cm_->start()) {
>>>   		cerr << "Failed to start camera manager" << endl;
>>>   		status_ = TestFail;
>>> diff --git a/test/libtest/camera_test.h b/test/libtest/camera_test.h
>>> index 7939798f..f56e343e 100644
>>> --- a/test/libtest/camera_test.h
>>> +++ b/test/libtest/camera_test.h
>>> @@ -17,7 +17,7 @@ using namespace libcamera;
>>>   class CameraTest
>>>   {
>>>   public:
>>> -	CameraTest(const char *name);
>>> +	CameraTest(const char *name, bool isolate = false);
>>>   	~CameraTest();
>>>   
>>>   protected:
>>>