[PATCH 1/2] pipeline: rkisp1: Use cached Request pointer

[Umang Jain]

Hi Kieran

On 18/04/24 3:38 pm, Kieran Bingham wrote:
> Quoting Umang Jain (2024-04-18 08:49:44)
>> The Request pointer is cached in RkISP1FrameInfo. Use it directly
>> instead of retrieving it via buffer->request().
> I don't think there's any specific speed up here. I think the compiler
> will map this into a single data read in both cases ?
>
> As this is in the bufferReady() context, I think useing the request
> associat3ed with the buffer is 'currently' correct.
>
> That said, I think this is an area that's likely about to get reworked
> in two fronts. Dan has looked at how to clean up the FrameInfo handling,
> and Stefan has been looking at how to be able to split/spearate buffers
> and requests.

There is a third front in this case, where the buffer is the internal 
buffer which pipes into another component. In that case, 
buffer->request() will fail (since buffer is an internal buffer in this 
case).

Regarding the latter point, I thought it already is in line with 
separation of request/buffers ... since we are using cached value of 
request, instead of getting it from buffer


>
> So I suspect that we should already merge 2/2 from this series but
> probably leave this one out for now...
>

OK
>
>
>> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
>> ---
>>   src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
>> index abb21968..5a6c9e1e 100644
>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
>> @@ -1261,7 +1261,7 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
>>                  return;
>>   
>>          const FrameMetadata &metadata = buffer->metadata();
>> -       Request *request = buffer->request();
>> +       Request *request = info->request;
>>   
>>          if (metadata.status != FrameMetadata::FrameCancelled) {
>>                  /*
>> -- 
>> 2.44.0
>>