[libcamera-devel,v2,6/7] qcam: Provide initial icon buttons "Play/Stop"
diff mbox series

Message ID 20200214001810.19302-7-kieran.bingham@ideasonboard.com
State Accepted
Headers show
Series
  • qcam: Provide an initial toolbar
Related show

Commit Message

Kieran Bingham Feb. 14, 2020, 12:18 a.m. UTC 
Provide Quit, Play, Stop icons.

Create a Qt resource to compile icons into the binary and present them
on the toolbar.

Update the Quit button with a 'cross', and implement Play/Stop buttons.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

---
v2
 - Remove pause button

 src/qcam/assets/feathericons/feathericons.qrc | 7 +++++++
 src/qcam/main_window.cpp                      | 9 ++++++++-
 src/qcam/main_window.h                        | 6 +++---
 src/qcam/meson.build                          | 9 +++++++--
 4 files changed, 25 insertions(+), 6 deletions(-)
 create mode 100644 src/qcam/assets/feathericons/feathericons.qrc

Comments

Laurent Pinchart Feb. 14, 2020, 12:28 a.m. UTC | #1 
Hi Kieran,

Thank you for the patch.

On Fri, Feb 14, 2020 at 12:18:09AM +0000, Kieran Bingham wrote:
> Provide Quit, Play, Stop icons.
> 
> Create a Qt resource to compile icons into the binary and present them
> on the toolbar.
> 
> Update the Quit button with a 'cross', and implement Play/Stop buttons.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> ---
> v2
>  - Remove pause button
> 
>  src/qcam/assets/feathericons/feathericons.qrc | 7 +++++++
>  src/qcam/main_window.cpp                      | 9 ++++++++-
>  src/qcam/main_window.h                        | 6 +++---
>  src/qcam/meson.build                          | 9 +++++++--
>  4 files changed, 25 insertions(+), 6 deletions(-)
>  create mode 100644 src/qcam/assets/feathericons/feathericons.qrc
> 
> diff --git a/src/qcam/assets/feathericons/feathericons.qrc b/src/qcam/assets/feathericons/feathericons.qrc
> new file mode 100644
> index 000000000000..b8e5c2266408
> --- /dev/null
> +++ b/src/qcam/assets/feathericons/feathericons.qrc
> @@ -0,0 +1,7 @@
> +<!DOCTYPE RCC><RCC version="1.0">
> +<qresource>
> +<file>./play-circle.svg</file>
> +<file>./stop-circle.svg</file>
> +<file>./x-circle.svg</file>
> +</qresource>
> +</RCC>
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 0e9e717b7e1a..ec93e0177b41 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -12,6 +12,7 @@
>  
>  #include <QComboBox>
>  #include <QCoreApplication>
> +#include <QIcon>
>  #include <QInputDialog>
>  #include <QTimer>
>  #include <QToolBar>
> @@ -66,7 +67,7 @@ int MainWindow::createToolbars()
>  	/* Disable right click context menu */
>  	toolbar_->setContextMenuPolicy(Qt::PreventContextMenu);
>  
> -	action = toolbar_->addAction("Quit");
> +	action = toolbar_->addAction(QIcon(":x-circle.svg"), "Quit");
>  	connect(action, &QAction::triggered, this, &MainWindow::quit);
>  
>  	/* Camera selection */
> @@ -81,6 +82,12 @@ int MainWindow::createToolbars()
>  
>  	toolbar_->addSeparator();
>  
> +	action = toolbar_->addAction(QIcon(":play-circle.svg"), "start");
> +	connect(action, &QAction::triggered, this, &MainWindow::startCapture);
> +
> +	action = toolbar_->addAction(QIcon(":stop-circle.svg"), "stop");
> +	connect(action, &QAction::triggered, this, &MainWindow::stopCapture);

Do I understand correctly that if capture is stopped and we select
another camera through the combo box, capture will be started on the new
camera automatically ? Is this desired ?

> +
>  	return 0;
>  }
>  
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index 12af103f87d0..27ceed611d59 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -46,14 +46,14 @@ private Q_SLOTS:
>  
>  	void switchCamera(int index);
>  
> +	int startCapture();
> +	void stopCapture();
> +
>  private:
>  	int createToolbars();
>  	std::string chooseCamera();
>  	int openCamera();
>  
> -	int startCapture();
> -	void stopCapture();
> -
>  	void requestComplete(Request *request);
>  	int display(FrameBuffer *buffer);
>  
> diff --git a/src/qcam/meson.build b/src/qcam/meson.build
> index 1e71f20fa15e..5b877a84da85 100644
> --- a/src/qcam/meson.build
> +++ b/src/qcam/meson.build
> @@ -11,6 +11,10 @@ qcam_moc_headers = files([
>      'main_window.h',
>  ])
>  
> +qcam_resources = files([
> +    'assets/feathericons/feathericons.qrc',
> +])
> +
>  qt5 = import('qt5')
>  qt5_dep = dependency('qt5',
>                       method : 'pkg-config',
> @@ -30,10 +34,11 @@ if qt5_dep.found()
>          endif
>      endif
>  
> -    moc_files = qt5.preprocess(moc_headers: qcam_moc_headers,
> +    resources = qt5.preprocess(moc_headers: qcam_moc_headers,
> +                               qresources : qcam_resources,
>                                 dependencies: qt5_dep)
>  
> -    qcam  = executable('qcam', qcam_sources, moc_files,
> +    qcam  = executable('qcam', qcam_sources, resources,
>                         install : true,
>                         dependencies : [libcamera_dep, qt5_dep],
>                         cpp_args : qt5_cpp_args)
Kieran Bingham Feb. 14, 2020, 8:28 a.m. UTC | #2 
Hi Laurent,

On 14/02/2020 00:28, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Fri, Feb 14, 2020 at 12:18:09AM +0000, Kieran Bingham wrote:
>> Provide Quit, Play, Stop icons.
>>
>> Create a Qt resource to compile icons into the binary and present them
>> on the toolbar.
>>
>> Update the Quit button with a 'cross', and implement Play/Stop buttons.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>
>> ---
>> v2
>>  - Remove pause button
>>
>>  src/qcam/assets/feathericons/feathericons.qrc | 7 +++++++
>>  src/qcam/main_window.cpp                      | 9 ++++++++-
>>  src/qcam/main_window.h                        | 6 +++---
>>  src/qcam/meson.build                          | 9 +++++++--
>>  4 files changed, 25 insertions(+), 6 deletions(-)
>>  create mode 100644 src/qcam/assets/feathericons/feathericons.qrc
>>
>> diff --git a/src/qcam/assets/feathericons/feathericons.qrc b/src/qcam/assets/feathericons/feathericons.qrc
>> new file mode 100644
>> index 000000000000..b8e5c2266408
>> --- /dev/null
>> +++ b/src/qcam/assets/feathericons/feathericons.qrc
>> @@ -0,0 +1,7 @@
>> +<!DOCTYPE RCC><RCC version="1.0">
>> +<qresource>
>> +<file>./play-circle.svg</file>
>> +<file>./stop-circle.svg</file>
>> +<file>./x-circle.svg</file>
>> +</qresource>
>> +</RCC>
>> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
>> index 0e9e717b7e1a..ec93e0177b41 100644
>> --- a/src/qcam/main_window.cpp
>> +++ b/src/qcam/main_window.cpp
>> @@ -12,6 +12,7 @@
>>  
>>  #include <QComboBox>
>>  #include <QCoreApplication>
>> +#include <QIcon>
>>  #include <QInputDialog>
>>  #include <QTimer>
>>  #include <QToolBar>
>> @@ -66,7 +67,7 @@ int MainWindow::createToolbars()
>>  	/* Disable right click context menu */
>>  	toolbar_->setContextMenuPolicy(Qt::PreventContextMenu);
>>  
>> -	action = toolbar_->addAction("Quit");
>> +	action = toolbar_->addAction(QIcon(":x-circle.svg"), "Quit");
>>  	connect(action, &QAction::triggered, this, &MainWindow::quit);
>>  
>>  	/* Camera selection */
>> @@ -81,6 +82,12 @@ int MainWindow::createToolbars()
>>  
>>  	toolbar_->addSeparator();
>>  
>> +	action = toolbar_->addAction(QIcon(":play-circle.svg"), "start");
>> +	connect(action, &QAction::triggered, this, &MainWindow::startCapture);
>> +
>> +	action = toolbar_->addAction(QIcon(":stop-circle.svg"), "stop");
>> +	connect(action, &QAction::triggered, this, &MainWindow::stopCapture);
> 
> Do I understand correctly that if capture is stopped and we select
> another camera through the combo box, capture will be started on the new
> camera automatically ? Is this desired ?


I was fine with it.

Do you want to add a patch to track streaming state and only start the
stream if one was already running?

That will complicate switchCamera a little, but not by much.

I'm not sure I (yet) see the benefit of switching to a camera but not
starting it ...

Especially as the ViewFinder would then be left with the previous image,
so it would be quite unclear as to whether the switch stream worked.


Ideally we need a status bar (and further in the future a log viewer) to
make sure we can notify users of what has / hasn't happened.

--
Kieran



>> +
>>  	return 0;
>>  }
>>  
>> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
>> index 12af103f87d0..27ceed611d59 100644
>> --- a/src/qcam/main_window.h
>> +++ b/src/qcam/main_window.h
>> @@ -46,14 +46,14 @@ private Q_SLOTS:
>>  
>>  	void switchCamera(int index);
>>  
>> +	int startCapture();
>> +	void stopCapture();
>> +
>>  private:
>>  	int createToolbars();
>>  	std::string chooseCamera();
>>  	int openCamera();
>>  
>> -	int startCapture();
>> -	void stopCapture();
>> -
>>  	void requestComplete(Request *request);
>>  	int display(FrameBuffer *buffer);
>>  
>> diff --git a/src/qcam/meson.build b/src/qcam/meson.build
>> index 1e71f20fa15e..5b877a84da85 100644
>> --- a/src/qcam/meson.build
>> +++ b/src/qcam/meson.build
>> @@ -11,6 +11,10 @@ qcam_moc_headers = files([
>>      'main_window.h',
>>  ])
>>  
>> +qcam_resources = files([
>> +    'assets/feathericons/feathericons.qrc',
>> +])
>> +
>>  qt5 = import('qt5')
>>  qt5_dep = dependency('qt5',
>>                       method : 'pkg-config',
>> @@ -30,10 +34,11 @@ if qt5_dep.found()
>>          endif
>>      endif
>>  
>> -    moc_files = qt5.preprocess(moc_headers: qcam_moc_headers,
>> +    resources = qt5.preprocess(moc_headers: qcam_moc_headers,
>> +                               qresources : qcam_resources,
>>                                 dependencies: qt5_dep)
>>  
>> -    qcam  = executable('qcam', qcam_sources, moc_files,
>> +    qcam  = executable('qcam', qcam_sources, resources,
>>                         install : true,
>>                         dependencies : [libcamera_dep, qt5_dep],
>>                         cpp_args : qt5_cpp_args)
>
Laurent Pinchart Feb. 14, 2020, 9:49 a.m. UTC | #3 
Hi Kieran,

On Fri, Feb 14, 2020 at 08:28:16AM +0000, Kieran Bingham wrote:
> On 14/02/2020 00:28, Laurent Pinchart wrote:
> > On Fri, Feb 14, 2020 at 12:18:09AM +0000, Kieran Bingham wrote:
> >> Provide Quit, Play, Stop icons.
> >>
> >> Create a Qt resource to compile icons into the binary and present them
> >> on the toolbar.
> >>
> >> Update the Quit button with a 'cross', and implement Play/Stop buttons.
> >>
> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> >> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>
> >> ---
> >> v2
> >>  - Remove pause button
> >>
> >>  src/qcam/assets/feathericons/feathericons.qrc | 7 +++++++
> >>  src/qcam/main_window.cpp                      | 9 ++++++++-
> >>  src/qcam/main_window.h                        | 6 +++---
> >>  src/qcam/meson.build                          | 9 +++++++--
> >>  4 files changed, 25 insertions(+), 6 deletions(-)
> >>  create mode 100644 src/qcam/assets/feathericons/feathericons.qrc
> >>
> >> diff --git a/src/qcam/assets/feathericons/feathericons.qrc b/src/qcam/assets/feathericons/feathericons.qrc
> >> new file mode 100644
> >> index 000000000000..b8e5c2266408
> >> --- /dev/null
> >> +++ b/src/qcam/assets/feathericons/feathericons.qrc
> >> @@ -0,0 +1,7 @@
> >> +<!DOCTYPE RCC><RCC version="1.0">
> >> +<qresource>
> >> +<file>./play-circle.svg</file>
> >> +<file>./stop-circle.svg</file>
> >> +<file>./x-circle.svg</file>
> >> +</qresource>
> >> +</RCC>
> >> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> >> index 0e9e717b7e1a..ec93e0177b41 100644
> >> --- a/src/qcam/main_window.cpp
> >> +++ b/src/qcam/main_window.cpp
> >> @@ -12,6 +12,7 @@
> >>  
> >>  #include <QComboBox>
> >>  #include <QCoreApplication>
> >> +#include <QIcon>
> >>  #include <QInputDialog>
> >>  #include <QTimer>
> >>  #include <QToolBar>
> >> @@ -66,7 +67,7 @@ int MainWindow::createToolbars()
> >>  	/* Disable right click context menu */
> >>  	toolbar_->setContextMenuPolicy(Qt::PreventContextMenu);
> >>  
> >> -	action = toolbar_->addAction("Quit");
> >> +	action = toolbar_->addAction(QIcon(":x-circle.svg"), "Quit");
> >>  	connect(action, &QAction::triggered, this, &MainWindow::quit);
> >>  
> >>  	/* Camera selection */
> >> @@ -81,6 +82,12 @@ int MainWindow::createToolbars()
> >>  
> >>  	toolbar_->addSeparator();
> >>  
> >> +	action = toolbar_->addAction(QIcon(":play-circle.svg"), "start");
> >> +	connect(action, &QAction::triggered, this, &MainWindow::startCapture);
> >> +
> >> +	action = toolbar_->addAction(QIcon(":stop-circle.svg"), "stop");
> >> +	connect(action, &QAction::triggered, this, &MainWindow::stopCapture);
> > 
> > Do I understand correctly that if capture is stopped and we select
> > another camera through the combo box, capture will be started on the new
> > camera automatically ? Is this desired ?
> 
> I was fine with it.
> 
> Do you want to add a patch to track streaming state and only start the
> stream if one was already running?
> 
> That will complicate switchCamera a little, but not by much.
> 
> I'm not sure I (yet) see the benefit of switching to a camera but not
> starting it ...
> 
> Especially as the ViewFinder would then be left with the previous image,
> so it would be quite unclear as to whether the switch stream worked.
> 
> 
> Ideally we need a status bar (and further in the future a log viewer) to
> make sure we can notify users of what has / hasn't happened.

I'm fine with this behaviour for now, I just wanted to make sure you
were aware of it. We'll keep improving qcam, and I'm sure streaming
state and camera switching will be reworked anyway.

> >> +
> >>  	return 0;
> >>  }
> >>  
> >> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> >> index 12af103f87d0..27ceed611d59 100644
> >> --- a/src/qcam/main_window.h
> >> +++ b/src/qcam/main_window.h
> >> @@ -46,14 +46,14 @@ private Q_SLOTS:
> >>  
> >>  	void switchCamera(int index);
> >>  
> >> +	int startCapture();
> >> +	void stopCapture();
> >> +
> >>  private:
> >>  	int createToolbars();
> >>  	std::string chooseCamera();
> >>  	int openCamera();
> >>  
> >> -	int startCapture();
> >> -	void stopCapture();
> >> -
> >>  	void requestComplete(Request *request);
> >>  	int display(FrameBuffer *buffer);
> >>  
> >> diff --git a/src/qcam/meson.build b/src/qcam/meson.build
> >> index 1e71f20fa15e..5b877a84da85 100644
> >> --- a/src/qcam/meson.build
> >> +++ b/src/qcam/meson.build
> >> @@ -11,6 +11,10 @@ qcam_moc_headers = files([
> >>      'main_window.h',
> >>  ])
> >>  
> >> +qcam_resources = files([
> >> +    'assets/feathericons/feathericons.qrc',
> >> +])
> >> +
> >>  qt5 = import('qt5')
> >>  qt5_dep = dependency('qt5',
> >>                       method : 'pkg-config',
> >> @@ -30,10 +34,11 @@ if qt5_dep.found()
> >>          endif
> >>      endif
> >>  
> >> -    moc_files = qt5.preprocess(moc_headers: qcam_moc_headers,
> >> +    resources = qt5.preprocess(moc_headers: qcam_moc_headers,
> >> +                               qresources : qcam_resources,
> >>                                 dependencies: qt5_dep)
> >>  
> >> -    qcam  = executable('qcam', qcam_sources, moc_files,
> >> +    qcam  = executable('qcam', qcam_sources, resources,
> >>                         install : true,
> >>                         dependencies : [libcamera_dep, qt5_dep],
> >>                         cpp_args : qt5_cpp_args)

Patch
diff mbox series 

diff --git a/src/qcam/assets/feathericons/feathericons.qrc b/src/qcam/assets/feathericons/feathericons.qrc
new file mode 100644
index 000000000000..b8e5c2266408
--- /dev/null
+++ b/src/qcam/assets/feathericons/feathericons.qrc
@@ -0,0 +1,7 @@ 
+<!DOCTYPE RCC><RCC version="1.0">
+<qresource>
+<file>./play-circle.svg</file>
+<file>./stop-circle.svg</file>
+<file>./x-circle.svg</file>
+</qresource>
+</RCC>
diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
index 0e9e717b7e1a..ec93e0177b41 100644
--- a/src/qcam/main_window.cpp
+++ b/src/qcam/main_window.cpp
@@ -12,6 +12,7 @@ 
 
 #include <QComboBox>
 #include <QCoreApplication>
+#include <QIcon>
 #include <QInputDialog>
 #include <QTimer>
 #include <QToolBar>
@@ -66,7 +67,7 @@  int MainWindow::createToolbars()
 	/* Disable right click context menu */
 	toolbar_->setContextMenuPolicy(Qt::PreventContextMenu);
 
-	action = toolbar_->addAction("Quit");
+	action = toolbar_->addAction(QIcon(":x-circle.svg"), "Quit");
 	connect(action, &QAction::triggered, this, &MainWindow::quit);
 
 	/* Camera selection */
@@ -81,6 +82,12 @@  int MainWindow::createToolbars()
 
 	toolbar_->addSeparator();
 
+	action = toolbar_->addAction(QIcon(":play-circle.svg"), "start");
+	connect(action, &QAction::triggered, this, &MainWindow::startCapture);
+
+	action = toolbar_->addAction(QIcon(":stop-circle.svg"), "stop");
+	connect(action, &QAction::triggered, this, &MainWindow::stopCapture);
+
 	return 0;
 }
 
diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
index 12af103f87d0..27ceed611d59 100644
--- a/src/qcam/main_window.h
+++ b/src/qcam/main_window.h
@@ -46,14 +46,14 @@  private Q_SLOTS:
 
 	void switchCamera(int index);
 
+	int startCapture();
+	void stopCapture();
+
 private:
 	int createToolbars();
 	std::string chooseCamera();
 	int openCamera();
 
-	int startCapture();
-	void stopCapture();
-
 	void requestComplete(Request *request);
 	int display(FrameBuffer *buffer);
 
diff --git a/src/qcam/meson.build b/src/qcam/meson.build
index 1e71f20fa15e..5b877a84da85 100644
--- a/src/qcam/meson.build
+++ b/src/qcam/meson.build
@@ -11,6 +11,10 @@  qcam_moc_headers = files([
     'main_window.h',
 ])
 
+qcam_resources = files([
+    'assets/feathericons/feathericons.qrc',
+])
+
 qt5 = import('qt5')
 qt5_dep = dependency('qt5',
                      method : 'pkg-config',
@@ -30,10 +34,11 @@  if qt5_dep.found()
         endif
     endif
 
-    moc_files = qt5.preprocess(moc_headers: qcam_moc_headers,
+    resources = qt5.preprocess(moc_headers: qcam_moc_headers,
+                               qresources : qcam_resources,
                                dependencies: qt5_dep)
 
-    qcam  = executable('qcam', qcam_sources, moc_files,
+    qcam  = executable('qcam', qcam_sources, resources,
                        install : true,
                        dependencies : [libcamera_dep, qt5_dep],
                        cpp_args : qt5_cpp_args)