[libcamera-devel,v7,5/8] qcam: Queue requests only through MainWindow::queueRequest()
diff mbox series

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

Commit Message

Utkarsh Tiwari Aug. 9, 2022, 8:50 p.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 for 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 at gmail.com>
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/qcam/main_window.cpp | 14 +++++++++-----
 src/qcam/main_window.h   |  3 ++-
 2 files changed, 11 insertions(+), 6 deletions(-)

Comments

Utkarsh Tiwari Aug. 9, 2022, 8:55 p.m. UTC | #1 
On Wed, Aug 10, 2022 at 2:23 AM Utkarsh Tiwari <utkarsh02t@gmail.com> wrote:

> 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 for 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 at gmail.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
Fix mail id here:
Reviewed-by: Kieran Bingham <kieran.bingham@ ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>

> ---
>  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 15b150ec..3feabcff 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -120,14 +120,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
> @@ -513,7 +513,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;
> @@ -745,7 +745,7 @@ void MainWindow::processViewfinder(FrameBuffer *buffer)
>         viewfinder_->render(buffer, mappedBuffers_[buffer].get());
>  }
>
> -void MainWindow::queueRequest(FrameBuffer *buffer)
> +void MainWindow::renderComplete(FrameBuffer *buffer)
>  {
>         Request *request;
>         {
> @@ -774,6 +774,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 bbdbb21c..bd6f0172 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -70,7 +70,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();
> @@ -84,6 +84,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
>
>
Kieran Bingham Aug. 9, 2022, 9:46 p.m. UTC | #2 
Quoting Utkarsh Tiwari (2022-08-09 21:55:15)
> On Wed, Aug 10, 2022 at 2:23 AM Utkarsh Tiwari <utkarsh02t@gmail.com> wrote:
> 
> > 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 for 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 at gmail.com>
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> Fix mail id here:
> Reviewed-by: Kieran Bingham <kieran.bingham@ ideasonboard.com>

Still not quite ;-) but it's ok, it can be handled when applying.


> Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
> 
> > ---
> >  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 15b150ec..3feabcff 100644
> > --- a/src/qcam/main_window.cpp
> > +++ b/src/qcam/main_window.cpp
> > @@ -120,14 +120,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
> > @@ -513,7 +513,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;
> > @@ -745,7 +745,7 @@ void MainWindow::processViewfinder(FrameBuffer *buffer)
> >         viewfinder_->render(buffer, mappedBuffers_[buffer].get());
> >  }
> >
> > -void MainWindow::queueRequest(FrameBuffer *buffer)
> > +void MainWindow::renderComplete(FrameBuffer *buffer)
> >  {
> >         Request *request;
> >         {
> > @@ -774,6 +774,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 bbdbb21c..bd6f0172 100644
> > --- a/src/qcam/main_window.h
> > +++ b/src/qcam/main_window.h
> > @@ -70,7 +70,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();
> > @@ -84,6 +84,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 15b150ec..3feabcff 100644
--- a/src/qcam/main_window.cpp
+++ b/src/qcam/main_window.cpp
@@ -120,14 +120,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
@@ -513,7 +513,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;
@@ -745,7 +745,7 @@  void MainWindow::processViewfinder(FrameBuffer *buffer)
 	viewfinder_->render(buffer, mappedBuffers_[buffer].get());
 }
 
-void MainWindow::queueRequest(FrameBuffer *buffer)
+void MainWindow::renderComplete(FrameBuffer *buffer)
 {
 	Request *request;
 	{
@@ -774,6 +774,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 bbdbb21c..bd6f0172 100644
--- a/src/qcam/main_window.h
+++ b/src/qcam/main_window.h
@@ -70,7 +70,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();
@@ -84,6 +84,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);