[libcamera-devel] [PATCH 2/2] ipa: vimc: Synchronise parameter buffer ops naming

[Umang Jain]

Hi Kieran,

On 4/8/22 17:13, Kieran Bingham wrote:
> Quoting Umang Jain via libcamera-devel (2022-04-08 11:54:39)
>> Synchronise the names of the operations with respect to parameters
>> buffer with the names used in other IPA interfaces.
>>
> The VIMC pipeline handler doesn't yet use an ISP, so I was curious at
> what buffers this would represent - but actually I think it's fine to
> keep it aligned at least.


These are mock buffers. They enable IPA IPC tests we have in tests/.

Also look at,

https://git.linuxtv.org/libcamera.git/commit/?id=3c5732d04a879d71d928e200d399c8d61b0da27a

https://git.linuxtv.org/libcamera.git/commit/?id=c2437e8cdefc7ae7efbfbd8662d822a02133084d

>
> I expect we could also use the VIMC pipeline handler with any GPU based
> ISP we might develop so I expect it will get more alignment with that
> development too.


Yes.

>
> So for this - I think it's fine.
>
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>


Thanks!

>
>> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
>> ---
>>   include/libcamera/ipa/vimc.mojom     | 4 ++--
>>   src/ipa/vimc/vimc.cpp                | 6 +++---
>>   src/libcamera/pipeline/vimc/vimc.cpp | 8 ++++----
>>   3 files changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom
>> index cdc03ffb..718b9674 100644
>> --- a/include/libcamera/ipa/vimc.mojom
>> +++ b/include/libcamera/ipa/vimc.mojom
>> @@ -37,9 +37,9 @@ interface IPAVimcInterface {
>>           * interface functions that mimick how other pipeline handlers typically
>>           * handle parameters at runtime.
>>           */
>> -       [async] fillParams(uint32 frame, uint32 bufferId);
>> +       [async] fillParamsBuffer(uint32 frame, uint32 bufferId);
>>   };
>>   
>>   interface IPAVimcEventInterface {
>> -       paramsFilled(uint32 bufferId);
>> +       paramsBufferReady(uint32 bufferId);
>>   };
>> diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp
>> index a62e72b0..85afb279 100644
>> --- a/src/ipa/vimc/vimc.cpp
>> +++ b/src/ipa/vimc/vimc.cpp
>> @@ -44,7 +44,7 @@ public:
>>          void unmapBuffers(const std::vector<unsigned int> &ids) override;
>>   
>>          void queueRequest(uint32_t frame, const ControlList &controls) override;
>> -       void fillParams(uint32_t frame, uint32_t bufferId) override;
>> +       void fillParamsBuffer(uint32_t frame, uint32_t bufferId) override;
>>   
>>   private:
>>          void initTrace();
>> @@ -134,7 +134,7 @@ void IPAVimc::queueRequest([[maybe_unused]] uint32_t frame,
>>   {
>>   }
>>   
>> -void IPAVimc::fillParams([[maybe_unused]] uint32_t frame, uint32_t bufferId)
>> +void IPAVimc::fillParamsBuffer([[maybe_unused]] uint32_t frame, uint32_t bufferId)
>>   {
>>          auto it = buffers_.find(bufferId);
>>          if (it == buffers_.end()) {
>> @@ -142,7 +142,7 @@ void IPAVimc::fillParams([[maybe_unused]] uint32_t frame, uint32_t bufferId)
>>                  return;
>>          }
>>   
>> -       paramsFilled.emit(bufferId);
>> +       paramsBufferReady.emit(bufferId);
>>   }
>>   
>>   void IPAVimc::initTrace()
>> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
>> index 69b83d07..fff95a34 100644
>> --- a/src/libcamera/pipeline/vimc/vimc.cpp
>> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
>> @@ -53,7 +53,7 @@ public:
>>          int init();
>>          int allocateMockIPABuffers();
>>          void bufferReady(FrameBuffer *buffer);
>> -       void paramsFilled(unsigned int id);
>> +       void paramsBufferReady(unsigned int id);
>>   
>>          MediaDevice *media_;
>>          std::unique_ptr<CameraSensor> sensor_;
>> @@ -467,7 +467,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
>>                  return false;
>>          }
>>   
>> -       data->ipa_->paramsFilled.connect(data.get(), &VimcCameraData::paramsFilled);
>> +       data->ipa_->paramsBufferReady.connect(data.get(), &VimcCameraData::paramsBufferReady);
>>   
>>          std::string conf = data->ipa_->configurationFile("vimc.conf");
>>          data->ipa_->init(IPASettings{ conf, data->sensor_->model() });
>> @@ -589,7 +589,7 @@ void VimcCameraData::bufferReady(FrameBuffer *buffer)
>>          pipe->completeBuffer(request, buffer);
>>          pipe->completeRequest(request);
>>   
>> -       ipa_->fillParams(request->sequence(), mockIPABufs_[0]->cookie());
>> +       ipa_->fillParamsBuffer(request->sequence(), mockIPABufs_[0]->cookie());
>>   }
>>   
>>   int VimcCameraData::allocateMockIPABuffers()
>> @@ -607,7 +607,7 @@ int VimcCameraData::allocateMockIPABuffers()
>>          return video_->exportBuffers(kBufCount, &mockIPABufs_);
>>   }
>>   
>> -void VimcCameraData::paramsFilled([[maybe_unused]] unsigned int id)
>> +void VimcCameraData::paramsBufferReady([[maybe_unused]] unsigned int id)
>>   {
>>   }
>>   
>> -- 
>> 2.31.0
>>