[libcamera-devel] [PATCH 8/9] libcamera: pipeline: ipu3: Use buffer mapping

[Kieran Bingham]

Hi Jacopo,

On 06/07/2019 13:12, Niklas Söderlund wrote:
> Hi Jacopo,
> 
> Thanks for your work.
> 
> On 2019-07-05 00:53:33 +0200, Jacopo Mondi wrote:
>> In order to support the usage of application provided buffer, retrieve
>> the buffer to use on video devices using the Request in order to allow
>> the stream to perform buffer mapping, if requested.
>>
>> The IPU3 was the only pipeline handler to access the Request map
>> directly instead of using Request::findBuffer().
> 
> I now see why we really wish to hide the buffers() method. I think we 
> need to think hard on how to do that. But that could be done on-top of 
> this work.
> 
>>
>> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> 
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> 
>> ---
>>  src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
>> index 28dcefe3d19f..49aa27ff20d4 100644
>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
>> @@ -725,7 +725,7 @@ int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request)
>>  
>>  	for (auto it : request->buffers()) {
>>  		IPU3Stream *stream = static_cast<IPU3Stream *>(it.first);
>> -		Buffer *buffer = it.second;
>> +		Buffer *buffer = request->findBuffer(stream);

This looks really weird ...

"For each buffer, identify the stream, and then identify the buffer for
that stream..." ... isn't that just ... the buffer(iterator) you started
with ?


I think from my understanding of the series it's because there are now 2
'Buffer*' objects for each buffer queued (one to hold the V4L2
information, and one to hold .. the dmabuf?

In my head, all of this information should be in the same Buffer* ...





>>  
>>  		int ret = stream->device_->dev->queueBuffer(buffer);
>>  		if (ret < 0)
>> -- 
>> 2.21.0
>>
>> _______________________________________________
>> libcamera-devel mailing list
>> libcamera-devel at lists.libcamera.org
>> https://lists.libcamera.org/listinfo/libcamera-devel
> 

-- 
Regards
--
Kieran