[libcamera-devel,04/10] libcamera: camera: Remove explicit stream to buffer map in requestCompleted signal
diff mbox series

Message ID 20191028022224.795355-5-niklas.soderlund@ragnatech.se
State Accepted
Headers show
Series
  • libcamera: Fixes found while working on new buffer API
Related show

Commit Message

Niklas Söderlund Oct. 28, 2019, 2:22 a.m. UTC 
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@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(-)

Comments

Jacopo Mondi Nov. 6, 2019, 8:57 a.m. UTC | #1 
Hi Niklas,

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.

Please be aware this breaks teh android HAL. You should update that
one too

Thanks
  j

>
> Signed-off-by: Niklas Söderlund <niklas.soderlund@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;
>  	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);
>  	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. */
> --
> 2.23.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart Nov. 18, 2019, 11:23 a.m. UTC | #2 
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@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@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. */

Patch
diff mbox series 

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;
 	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);
 	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. */