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

[Kieran Bingham]

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.


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

-- 
Regards
--
Kieran