[libcamera-devel] [PATCH v4 09/15] android: camera_device: Rename shadowed variable

[Kieran Bingham]

Hi Laurent,

On 21/10/2020 16:55, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Wed, Oct 21, 2020 at 04:41:42PM +0100, Kieran Bingham wrote:
>> A FrameBuffer *buffer is used to obtain the 'first' buffer from a
>> request which is used purely to identify the timestamp from the
>> metadata.
>>
>> This should be determined by the Request, and set appropriately by the
>> pipeline handlers, but make sure that this buffer instance is distinct
>> for now.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>> ---
>>  src/android/camera_device.cpp | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
>> index 9cf1c98410f9..7bb2560025e2 100644
>> --- a/src/android/camera_device.cpp
>> +++ b/src/android/camera_device.cpp
>> @@ -1488,9 +1488,9 @@ void CameraDevice::requestComplete(Request *request)
>>  	 * It might be appropriate to return a 'correct' (as determined by
>>  	 * pipeline handlers) timestamp in the Request itself.
>>  	 */
>> -	FrameBuffer *buffer = buffers.begin()->second;
>> +	FrameBuffer *timestampBuf = buffers.begin()->second;
> 
> How about
> 
> 	uint64_t timestamp = buffers.at(0)->metadata().timestamp;
> 
> ? You can then reuse timestamp in the second hunk below. With this,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Indeed, that's much better.
--
Kieran


> 
>>  	resultMetadata = getResultMetadata(descriptor->frameNumber_,
>> -					   buffer->metadata().timestamp);
>> +					   timestampBuf->metadata().timestamp);
>>  
>>  	/* Handle any JPEG compression. */
>>  	for (unsigned int i = 0; i < descriptor->numBuffers_; ++i) {
>> @@ -1548,7 +1548,7 @@ void CameraDevice::requestComplete(Request *request)
>>  
>>  	if (status == CAMERA3_BUFFER_STATUS_OK) {
>>  		notifyShutter(descriptor->frameNumber_,
>> -			      buffer->metadata().timestamp);
>> +			      timestampBuf->metadata().timestamp);
>>  
>>  		captureResult.partial_result = 1;
>>  		captureResult.result = resultMetadata->get();
> 

-- 
Regards
--
Kieran