[libcamera-devel,v3,1/3] qcam: Queue requests only through MainWindow::queueRequest()
diff mbox series

Message ID 20220627030159.30730-2-utkarsh02t@gmail.com
State Superseded
Headers show
Series
  • Introduce capture scripts to qcam
Related show

Commit Message

Utkarsh Tiwari June 27, 2022, 3:01 a.m. UTC
Currently to request a frame, we operate the camera directly.
This approach is also scattered in two places,
MainWindow::startCapture() and MainWindow::queueRequest().
This makes it difficult to account requests.

Centralize all the queuing to a single function queueRequest()

Rename the current queueRequest() to renderComplete(),
This makes more sense as this slot is triggered when
the render is complete and we want to queue another
request.

Signed-off-by: Utkarsh Tiwari <utkarsh02t@gmail.com>
---
 src/qcam/main_window.cpp | 14 +++++++++-----
 src/qcam/main_window.h   |  3 ++-
 2 files changed, 11 insertions(+), 6 deletions(-)

Comments

Kieran Bingham July 1, 2022, 3:27 p.m. UTC | #1
Quoting Utkarsh Tiwari via libcamera-devel (2022-06-27 04:01:57)
> Currently to request a frame, we operate the camera directly.
> This approach is also scattered in two places,
> MainWindow::startCapture() and MainWindow::queueRequest().
> This makes it difficult to account requests.
> 
> Centralize all the queuing to a single function queueRequest()
> 
> Rename the current queueRequest() to renderComplete(),
> This makes more sense as this slot is triggered when
> the render is complete and we want to queue another
> request.

I gave this on both v1 and v2 so I think you can copy it directly into
the commit for any later versions (if required).

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>



> 
> Signed-off-by: Utkarsh Tiwari <utkarsh02t@gmail.com>
> ---

After these three dashes is a good place to put extra comments like a
changelog to say what has (or has not) changed between versions.


>  src/qcam/main_window.cpp | 14 +++++++++-----
>  src/qcam/main_window.h   |  3 ++-
>  2 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index dd0e51f5..adeb3181 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -119,14 +119,14 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options)
>         if (renderType == "qt") {
>                 ViewFinderQt *viewfinder = new ViewFinderQt(this);
>                 connect(viewfinder, &ViewFinderQt::renderComplete,
> -                       this, &MainWindow::queueRequest);
> +                       this, &MainWindow::renderComplete);
>                 viewfinder_ = viewfinder;
>                 setCentralWidget(viewfinder);
>  #ifndef QT_NO_OPENGL
>         } else if (renderType == "gles") {
>                 ViewFinderGL *viewfinder = new ViewFinderGL(this);
>                 connect(viewfinder, &ViewFinderGL::renderComplete,
> -                       this, &MainWindow::queueRequest);
> +                       this, &MainWindow::renderComplete);
>                 viewfinder_ = viewfinder;
>                 setCentralWidget(viewfinder);
>  #endif
> @@ -521,7 +521,7 @@ int MainWindow::startCapture()
>  
>         /* Queue all requests. */
>         for (std::unique_ptr<Request> &request : requests_) {
> -               ret = camera_->queueRequest(request.get());
> +               ret = queueRequest(request.get());
>                 if (ret < 0) {
>                         qWarning() << "Can't queue request";
>                         goto error_disconnect;
> @@ -755,7 +755,7 @@ void MainWindow::processViewfinder(FrameBuffer *buffer)
>         viewfinder_->render(buffer, mappedBuffers_[buffer].get());
>  }
>  
> -void MainWindow::queueRequest(FrameBuffer *buffer)
> +void MainWindow::renderComplete(FrameBuffer *buffer)
>  {
>         Request *request;
>         {
> @@ -784,6 +784,10 @@ void MainWindow::queueRequest(FrameBuffer *buffer)
>                         qWarning() << "No free buffer available for RAW capture";
>                 }
>         }
> +       queueRequest(request);
> +}
>  
> -       camera_->queueRequest(request);
> +int MainWindow::queueRequest(Request *request)
> +{
> +       return camera_->queueRequest(request);
>  }
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index 3fbe872c..c3e4b665 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -66,7 +66,7 @@ private Q_SLOTS:
>         void processRaw(libcamera::FrameBuffer *buffer,
>                         const libcamera::ControlList &metadata);
>  
> -       void queueRequest(libcamera::FrameBuffer *buffer);
> +       void renderComplete(libcamera::FrameBuffer *buffer);
>  
>  private:
>         int createToolbars();
> @@ -80,6 +80,7 @@ private:
>         void addCamera(std::shared_ptr<libcamera::Camera> camera);
>         void removeCamera(std::shared_ptr<libcamera::Camera> camera);
>  
> +       int queueRequest(libcamera::Request *request);
>         void requestComplete(libcamera::Request *request);
>         void processCapture();
>         void processHotplug(HotplugEvent *e);
> -- 
> 2.25.1
>

Patch
diff mbox series

diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
index dd0e51f5..adeb3181 100644
--- a/src/qcam/main_window.cpp
+++ b/src/qcam/main_window.cpp
@@ -119,14 +119,14 @@  MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options)
 	if (renderType == "qt") {
 		ViewFinderQt *viewfinder = new ViewFinderQt(this);
 		connect(viewfinder, &ViewFinderQt::renderComplete,
-			this, &MainWindow::queueRequest);
+			this, &MainWindow::renderComplete);
 		viewfinder_ = viewfinder;
 		setCentralWidget(viewfinder);
 #ifndef QT_NO_OPENGL
 	} else if (renderType == "gles") {
 		ViewFinderGL *viewfinder = new ViewFinderGL(this);
 		connect(viewfinder, &ViewFinderGL::renderComplete,
-			this, &MainWindow::queueRequest);
+			this, &MainWindow::renderComplete);
 		viewfinder_ = viewfinder;
 		setCentralWidget(viewfinder);
 #endif
@@ -521,7 +521,7 @@  int MainWindow::startCapture()
 
 	/* Queue all requests. */
 	for (std::unique_ptr<Request> &request : requests_) {
-		ret = camera_->queueRequest(request.get());
+		ret = queueRequest(request.get());
 		if (ret < 0) {
 			qWarning() << "Can't queue request";
 			goto error_disconnect;
@@ -755,7 +755,7 @@  void MainWindow::processViewfinder(FrameBuffer *buffer)
 	viewfinder_->render(buffer, mappedBuffers_[buffer].get());
 }
 
-void MainWindow::queueRequest(FrameBuffer *buffer)
+void MainWindow::renderComplete(FrameBuffer *buffer)
 {
 	Request *request;
 	{
@@ -784,6 +784,10 @@  void MainWindow::queueRequest(FrameBuffer *buffer)
 			qWarning() << "No free buffer available for RAW capture";
 		}
 	}
+	queueRequest(request);
+}
 
-	camera_->queueRequest(request);
+int MainWindow::queueRequest(Request *request)
+{
+	return camera_->queueRequest(request);
 }
diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
index 3fbe872c..c3e4b665 100644
--- a/src/qcam/main_window.h
+++ b/src/qcam/main_window.h
@@ -66,7 +66,7 @@  private Q_SLOTS:
 	void processRaw(libcamera::FrameBuffer *buffer,
 			const libcamera::ControlList &metadata);
 
-	void queueRequest(libcamera::FrameBuffer *buffer);
+	void renderComplete(libcamera::FrameBuffer *buffer);
 
 private:
 	int createToolbars();
@@ -80,6 +80,7 @@  private:
 	void addCamera(std::shared_ptr<libcamera::Camera> camera);
 	void removeCamera(std::shared_ptr<libcamera::Camera> camera);
 
+	int queueRequest(libcamera::Request *request);
 	void requestComplete(libcamera::Request *request);
 	void processCapture();
 	void processHotplug(HotplugEvent *e);