[libcamera-devel] [PATCH 04/10] libcamera: camera: Remove explicit stream to buffer map in requestCompleted signal

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Mon, Oct 28, 2019 at 03:22:18AM +0100, Niklas Söderlund wrote:
> The stream to buffer map in the requestCompleted signal is taken
> directly from the request which is part of the same signal. Remove the
> map as it can be fetched directly from the request.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  include/libcamera/camera.h | 2 +-
>  src/cam/capture.cpp        | 4 +++-
>  src/cam/capture.h          | 3 +--
>  src/libcamera/camera.cpp   | 2 +-
>  src/qcam/main_window.cpp   | 5 +++--
>  src/qcam/main_window.h     | 3 +--
>  test/camera/capture.cpp    | 4 +++-
>  7 files changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> index 21fac550f4121fdc..268d2d37ccac8b71 100644
> --- a/include/libcamera/camera.h
> +++ b/include/libcamera/camera.h
> @@ -79,7 +79,7 @@ public:
>  	const std::string &name() const;
>  
>  	Signal<Request *, Buffer *> bufferCompleted;
> -	Signal<Request *, const std::map<Stream *, Buffer *> &> requestCompleted;
> +	Signal<Request *> requestCompleted;

This was the only user of std::map, you can drop the include.

>  	Signal<Camera *> disconnected;
>  
>  	int acquire();
> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
> index 0d7854d674e0a78d..27332df1d55a5c2d 100644
> --- a/src/cam/capture.cpp
> +++ b/src/cam/capture.cpp
> @@ -133,11 +133,13 @@ int Capture::capture(EventLoop *loop)
>  	return ret;
>  }
>  
> -void Capture::requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers)
> +void Capture::requestComplete(Request *request)
>  {
>  	if (request->status() == Request::RequestCancelled)
>  		return;
>  
> +	const std::map<Stream *, Buffer *> &buffers = request->buffers();
> +
>  	std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now();
>  	double fps = std::chrono::duration_cast<std::chrono::milliseconds>(now - last_).count();
>  	fps = last_ != std::chrono::steady_clock::time_point() && fps
> diff --git a/src/cam/capture.h b/src/cam/capture.h
> index ee0dc421111197c1..4d396afb8c771a74 100644
> --- a/src/cam/capture.h
> +++ b/src/cam/capture.h
> @@ -28,8 +28,7 @@ public:
>  private:
>  	int capture(EventLoop *loop);
>  
> -	void requestComplete(libcamera::Request *request,
> -			     const std::map<libcamera::Stream *, libcamera::Buffer *> &buffers);
> +	void requestComplete(libcamera::Request *request);
>  
>  	libcamera::Camera *camera_;
>  	libcamera::CameraConfiguration *config_;
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 76c737cb93813115..e810fb725d81350d 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -919,7 +919,7 @@ void Camera::requestComplete(Request *request)
>  			stream->unmapBuffer(buffer);
>  	}
>  
> -	requestCompleted.emit(request, request->buffers());
> +	requestCompleted.emit(request);
>  	delete request;
>  }
>  
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 3d56309227235fec..cca7365ae75687f9 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -248,12 +248,13 @@ void MainWindow::stopCapture()
>  	setWindowTitle(title_);
>  }
>  
> -void MainWindow::requestComplete(Request *request,
> -				 const std::map<Stream *, Buffer *> &buffers)
> +void MainWindow::requestComplete(Request *request)
>  {
>  	if (request->status() == Request::RequestCancelled)
>  		return;
>  
> +	const std::map<Stream *, Buffer *> &buffers = request->buffers();
> +
>  	framesCaptured_++;
>  
>  	Buffer *buffer = buffers.begin()->second;
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index 30dd8743104d75e0..78511581a8d5c025 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -49,8 +49,7 @@ private:
>  	int startCapture();
>  	void stopCapture();
>  
> -	void requestComplete(Request *request,
> -			     const std::map<Stream *, Buffer *> &buffers);
> +	void requestComplete(Request *request);

Same here, you can drop the #include <map>.

Apart from this, and with the HAL fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

>  	int display(Buffer *buffer);
>  
>  	QString title_;
> diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
> index 791ccad15f7042f9..83f974749affd3cd 100644
> --- a/test/camera/capture.cpp
> +++ b/test/camera/capture.cpp
> @@ -27,11 +27,13 @@ protected:
>  		completeBuffersCount_++;
>  	}
>  
> -	void requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers)
> +	void requestComplete(Request *request)
>  	{
>  		if (request->status() != Request::RequestComplete)
>  			return;
>  
> +		const std::map<Stream *, Buffer *> &buffers = request->buffers();
> +
>  		completeRequestsCount_++;
>  
>  		/* Create a new request. */

-- 
Regards,

Laurent Pinchart