[libcamera-devel] [PATCH 05/10] libcamera: pipeline: Use existing variable definitions

[Kieran Bingham]

Hi Niklas,

On 14/10/2020 13:27, Niklas Söderlund wrote:
> Hi Kieran,
> 
> On 2020-10-13 16:12:36 +0100, Kieran Bingham wrote:
>> Prevent variable aliasing by removing the redeclaration of variables
>> with the same name (and type) where the existing variable can be reused.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>> ---
>>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 2 +-
>>  src/libcamera/pipeline/raspberrypi/rpi_stream.cpp  | 2 +-
>>  src/libcamera/pipeline/simple/simple.cpp           | 4 ++--
>>  3 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
>> index 85e0a1f26ab6..2d70d984a276 100644
>> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
>> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
>> @@ -1008,7 +1008,7 @@ int PipelineHandlerRPi::queueAllBuffers(Camera *camera)
>>  			 */
>>  			unsigned int i;
>>  			for (i = 0; i < data->dropFrameCount_; i++) {
>> -				int ret = stream->queueBuffer(nullptr);
>> +				ret = stream->queueBuffer(nullptr);
>>  				if (ret)
>>  					return ret;
>>  			}
>> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
>> index 1a42cc17bcba..17e38924d653 100644
>> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
>> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
>> @@ -163,7 +163,7 @@ void Stream::returnBuffer(FrameBuffer *buffer)
>>  	 * If so, do it now as availableBuffers_ will not be empty.
>>  	 */
>>  	while (!requestBuffers_.empty()) {
>> -		FrameBuffer *buffer = requestBuffers_.front();
>> +		buffer = requestBuffers_.front();
> 
> Same comment as for 2/10 about reusing a function argument.

This one seems a bit less clear-cut.

The incoming buffer is one that gets put on the availableBuffers_ queue.

This new 'buffer' is used from two locations. Both the availableBuffers_
queue and the requestBuffers_ queue.

So there's no clear 'This is a raw Buffer' case like before.

We could say that it is distinct, that it is the 'next buffer if
identified' to queue to the device...

So that could give a name of ...

 nextBuffer, or deviceBuffer ... or just 'buffer'?

I'm inclined to leave this as just re-using the buffer variable
currently....

Any thoughts or better naming?

--
Kieran




> 
>>  
>>  		if (!buffer) {
>>  			/*
>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
>> index 999c44515023..33daa2fb1b7b 100644
>> --- a/src/libcamera/pipeline/simple/simple.cpp
>> +++ b/src/libcamera/pipeline/simple/simple.cpp
>> @@ -612,8 +612,8 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
>>  	useConverter_ = config->needConversion();
>>  
>>  	if (useConverter_) {
>> -		int ret = converter_->configure(pipeConfig.pixelFormat,
>> -						pipeConfig.captureSize, &cfg);
>> +		ret = converter_->configure(pipeConfig.pixelFormat,
>> +					    pipeConfig.captureSize, &cfg);
>>  		if (ret < 0) {
>>  			LOG(SimplePipeline, Error)
>>  				<< "Unable to configure converter";
>> -- 
>> 2.25.1
>>
>> _______________________________________________
>> libcamera-devel mailing list
>> libcamera-devel at lists.libcamera.org
>> https://lists.libcamera.org/listinfo/libcamera-devel
> 

-- 
Regards
--
Kieran