[libcamera-devel] [PATCH v3 14/14] libcamera: pipeline: Cast to derived pipeline handler with helpers

[Kieran Bingham]

On 16/08/2021 13:26, Laurent Pinchart wrote:
>>> @@ -332,8 +338,7 @@ void RkISP1CameraData::queueFrameAction(unsigned int frame,
>>>  		break;
>>>  	}
>>>  	case ipa::rkisp1::ActionParamFilled: {
>>> -		PipelineHandlerRkISP1 *pipe =
>>> -			static_cast<PipelineHandlerRkISP1 *>(this->pipe());
>>> +		PipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();
>>
>>
>> Why the RkISP1CameraData:: prefix?
> 
> Because an unqualified 'pipe' refers to the local variable, so
> 
> 		PipelineHandlerRkISP1 *pipe = pipe();
> 
> makes the compiler complain that 'PipelineHandlerRkISP1 *' has no
> operator() defined. It doesn't resolve 'pipe' to the function, but the
> local variable.

But is the local variable needed?

You could change the two lines :

 		pipe->param_->queueBuffer(info->paramBuffer);
 		pipe->stat_->queueBuffer(info->statBuffer);

to
 		pipe()->param_->queueBuffer(info->paramBuffer);
 		pipe()->stat_->queueBuffer(info->statBuffer);


Which would match the usage everywhere else...


Anyway, it's a valid construct as you have it - it just looked out of place.

Tag still holds however you decide to do this.


> 
>> Isn't this just pipe(); here?
>>
>> We're in RkISP1CameraData::queueFrameAction() right ?
>>
>> With that resolved,
>>
>> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>