[libcamera-devel,v3,09/13] pipeline: rkisp1: Pass info pointer to tryCompleteRequest()
diff mbox series

Message ID 20221024000356.29521-10-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • Add Bayer format support for RkISP1
Related show

Commit Message

Laurent Pinchart Oct. 24, 2022, 12:03 a.m. UTC 
The tryCompleteRequest() function looks up the RkISP1FrameInfo that alls
but one of its callers already look up. Remove the double look up by
passing the info pointer to tryCompleteRequest().

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

Comments

Nicolas Dufresne via libcamera-devel Oct. 24, 2022, 8:54 a.m. UTC | #1 
On Mon, Oct 24, 2022 at 03:03:52AM +0300, Laurent Pinchart wrote:
> The tryCompleteRequest() function looks up the RkISP1FrameInfo that alls

s/alls/all/

> but one of its callers already look up. Remove the double look up by
> passing the info pointer to tryCompleteRequest().
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 50da92d4d6f8..cca89cc13bff 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -168,7 +168,7 @@ private:
>  	int initLinks(Camera *camera, const CameraSensor *sensor,
>  		      const RkISP1CameraConfiguration &config);
>  	int createCamera(MediaEntity *sensor);
> -	void tryCompleteRequest(Request *request);
> +	void tryCompleteRequest(RkISP1FrameInfo *info);
>  	void bufferReady(FrameBuffer *buffer);
>  	void paramReady(FrameBuffer *buffer);
>  	void statReady(FrameBuffer *buffer);
> @@ -391,7 +391,7 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta
>  	info->request->metadata().merge(metadata);
>  	info->metadataProcessed = true;
>  
> -	pipe()->tryCompleteRequest(info->request);
> +	pipe()->tryCompleteRequest(info);
>  }
>  
>  RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera,
> @@ -1123,12 +1123,10 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
>   * Buffer Handling
>   */
>  
> -void PipelineHandlerRkISP1::tryCompleteRequest(Request *request)
> +void PipelineHandlerRkISP1::tryCompleteRequest(RkISP1FrameInfo *info)
>  {
>  	RkISP1CameraData *data = cameraData(activeCamera_);
> -	RkISP1FrameInfo *info = data->frameInfo_.find(request);
> -	if (!info)
> -		return;
> +	Request *request = info->request;
>  
>  	if (request->hasPendingBuffers())
>  		return;
> @@ -1146,6 +1144,13 @@ void PipelineHandlerRkISP1::tryCompleteRequest(Request *request)
>  
>  void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
>  {
> +	ASSERT(activeCamera_);
> +	RkISP1CameraData *data = cameraData(activeCamera_);
> +
> +	RkISP1FrameInfo *info = data->frameInfo_.find(buffer);
> +	if (!info)
> +		return;
> +
>  	Request *request = buffer->request();
>  
>  	/*
> @@ -1158,7 +1163,7 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
>  				buffer->metadata().timestamp);
>  
>  	completeBuffer(request, buffer);
> -	tryCompleteRequest(request);
> +	tryCompleteRequest(info);
>  }
>  
>  void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)
> @@ -1171,7 +1176,7 @@ void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)
>  		return;
>  
>  	info->paramDequeued = true;
> -	tryCompleteRequest(info->request);
> +	tryCompleteRequest(info);
>  }
>  
>  void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
> @@ -1185,7 +1190,7 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
>  
>  	if (buffer->metadata().status == FrameMetadata::FrameCancelled) {
>  		info->metadataProcessed = true;
> -		tryCompleteRequest(info->request);
> +		tryCompleteRequest(info);
>  		return;
>  	}
>  
> -- 
> Regards,
> 
> Laurent Pinchart
>
Jacopo Mondi Oct. 26, 2022, 3:20 p.m. UTC | #2 
Hi Laurent

On Mon, Oct 24, 2022 at 03:03:52AM +0300, Laurent Pinchart via libcamera-devel wrote:
> The tryCompleteRequest() function looks up the RkISP1FrameInfo that alls
> but one of its callers already look up. Remove the double look up by
> passing the info pointer to tryCompleteRequest().
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 50da92d4d6f8..cca89cc13bff 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -168,7 +168,7 @@ private:
>  	int initLinks(Camera *camera, const CameraSensor *sensor,
>  		      const RkISP1CameraConfiguration &config);
>  	int createCamera(MediaEntity *sensor);
> -	void tryCompleteRequest(Request *request);
> +	void tryCompleteRequest(RkISP1FrameInfo *info);
>  	void bufferReady(FrameBuffer *buffer);
>  	void paramReady(FrameBuffer *buffer);
>  	void statReady(FrameBuffer *buffer);
> @@ -391,7 +391,7 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta
>  	info->request->metadata().merge(metadata);
>  	info->metadataProcessed = true;
>
> -	pipe()->tryCompleteRequest(info->request);
> +	pipe()->tryCompleteRequest(info);
>  }
>
>  RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera,
> @@ -1123,12 +1123,10 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
>   * Buffer Handling
>   */
>
> -void PipelineHandlerRkISP1::tryCompleteRequest(Request *request)
> +void PipelineHandlerRkISP1::tryCompleteRequest(RkISP1FrameInfo *info)
>  {
>  	RkISP1CameraData *data = cameraData(activeCamera_);
> -	RkISP1FrameInfo *info = data->frameInfo_.find(request);
> -	if (!info)
> -		return;
> +	Request *request = info->request;
>
>  	if (request->hasPendingBuffers())
>  		return;
> @@ -1146,6 +1144,13 @@ void PipelineHandlerRkISP1::tryCompleteRequest(Request *request)
>
>  void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
>  {
> +	ASSERT(activeCamera_);
> +	RkISP1CameraData *data = cameraData(activeCamera_);
> +
> +	RkISP1FrameInfo *info = data->frameInfo_.find(buffer);
> +	if (!info)
> +		return;
> +
>  	Request *request = buffer->request();
>
>  	/*
> @@ -1158,7 +1163,7 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
>  				buffer->metadata().timestamp);
>
>  	completeBuffer(request, buffer);
> -	tryCompleteRequest(request);
> +	tryCompleteRequest(info);
>  }
>
>  void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)
> @@ -1171,7 +1176,7 @@ void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)
>  		return;
>
>  	info->paramDequeued = true;
> -	tryCompleteRequest(info->request);
> +	tryCompleteRequest(info);
>  }
>
>  void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
> @@ -1185,7 +1190,7 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
>
>  	if (buffer->metadata().status == FrameMetadata::FrameCancelled) {
>  		info->metadataProcessed = true;
> -		tryCompleteRequest(info->request);
> +		tryCompleteRequest(info);
>  		return;
>  	}
>
> --
> Regards,
>
> Laurent Pinchart
>

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 50da92d4d6f8..cca89cc13bff 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -168,7 +168,7 @@  private:
 	int initLinks(Camera *camera, const CameraSensor *sensor,
 		      const RkISP1CameraConfiguration &config);
 	int createCamera(MediaEntity *sensor);
-	void tryCompleteRequest(Request *request);
+	void tryCompleteRequest(RkISP1FrameInfo *info);
 	void bufferReady(FrameBuffer *buffer);
 	void paramReady(FrameBuffer *buffer);
 	void statReady(FrameBuffer *buffer);
@@ -391,7 +391,7 @@  void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta
 	info->request->metadata().merge(metadata);
 	info->metadataProcessed = true;
 
-	pipe()->tryCompleteRequest(info->request);
+	pipe()->tryCompleteRequest(info);
 }
 
 RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera,
@@ -1123,12 +1123,10 @@  bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
  * Buffer Handling
  */
 
-void PipelineHandlerRkISP1::tryCompleteRequest(Request *request)
+void PipelineHandlerRkISP1::tryCompleteRequest(RkISP1FrameInfo *info)
 {
 	RkISP1CameraData *data = cameraData(activeCamera_);
-	RkISP1FrameInfo *info = data->frameInfo_.find(request);
-	if (!info)
-		return;
+	Request *request = info->request;
 
 	if (request->hasPendingBuffers())
 		return;
@@ -1146,6 +1144,13 @@  void PipelineHandlerRkISP1::tryCompleteRequest(Request *request)
 
 void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
 {
+	ASSERT(activeCamera_);
+	RkISP1CameraData *data = cameraData(activeCamera_);
+
+	RkISP1FrameInfo *info = data->frameInfo_.find(buffer);
+	if (!info)
+		return;
+
 	Request *request = buffer->request();
 
 	/*
@@ -1158,7 +1163,7 @@  void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
 				buffer->metadata().timestamp);
 
 	completeBuffer(request, buffer);
-	tryCompleteRequest(request);
+	tryCompleteRequest(info);
 }
 
 void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)
@@ -1171,7 +1176,7 @@  void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)
 		return;
 
 	info->paramDequeued = true;
-	tryCompleteRequest(info->request);
+	tryCompleteRequest(info);
 }
 
 void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
@@ -1185,7 +1190,7 @@  void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
 
 	if (buffer->metadata().status == FrameMetadata::FrameCancelled) {
 		info->metadataProcessed = true;
-		tryCompleteRequest(info->request);
+		tryCompleteRequest(info);
 		return;
 	}