[libcamera-devel] qcam: Use standard PicturesLocation path for capture
diff mbox series

Message ID 20200217153941.20713-1-kieran.bingham@ideasonboard.com
State Superseded
Headers show
Series
  • [libcamera-devel] qcam: Use standard PicturesLocation path for capture
Related show

Commit Message

Kieran Bingham Feb. 17, 2020, 3:39 p.m. UTC 
Utilise the QStandardPaths::PicturesLocation as a starting point for
saving images from qcam.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/qcam/main_window.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Laurent Pinchart Feb. 18, 2020, 1:05 a.m. UTC | #1 
Hi Kieran,

Thank you for the patch.

On Mon, Feb 17, 2020 at 03:39:41PM +0000, Kieran Bingham wrote:
> Utilise the QStandardPaths::PicturesLocation as a starting point for
> saving images from qcam.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  src/qcam/main_window.cpp | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 29eaba8454f8..09e4bfc11300 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -17,6 +17,7 @@
>  #include <QImage>
>  #include <QImageWriter>
>  #include <QInputDialog>
> +#include <QStandardPaths>
>  #include <QTimer>
>  #include <QToolBar>
>  #include <QToolButton>
> @@ -348,8 +349,9 @@ void MainWindow::stopCapture()
>  void MainWindow::saveImageAs()
>  {
>  	QImage image = viewfinder_->getCurrentImage();
> +	QString defaultPath = QStandardPaths::standardLocations(QStandardPaths::PicturesLocation).last();

QStandardPaths::standardLocations() sorts the result from high to low
priority, shouldn't we thus pick the first entry ? How about using
QStandardPaths::writableLocation() instead ?

>  
> -	QString filename = QFileDialog::getSaveFileName(this, "Save Image", "",
> +	QString filename = QFileDialog::getSaveFileName(this, "Save Image", defaultPath,
>  							"Image Files (*.png *.jpg *.jpeg)");
>  
>  	std::cout << "Save image to " << filename.toStdString() << std::endl;
Kieran Bingham Feb. 18, 2020, 9:02 a.m. UTC | #2 
Hi Laurent,

On 18/02/2020 01:05, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Mon, Feb 17, 2020 at 03:39:41PM +0000, Kieran Bingham wrote:
>> Utilise the QStandardPaths::PicturesLocation as a starting point for
>> saving images from qcam.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>> ---
>>  src/qcam/main_window.cpp | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
>> index 29eaba8454f8..09e4bfc11300 100644
>> --- a/src/qcam/main_window.cpp
>> +++ b/src/qcam/main_window.cpp
>> @@ -17,6 +17,7 @@
>>  #include <QImage>
>>  #include <QImageWriter>
>>  #include <QInputDialog>
>> +#include <QStandardPaths>
>>  #include <QTimer>
>>  #include <QToolBar>
>>  #include <QToolButton>
>> @@ -348,8 +349,9 @@ void MainWindow::stopCapture()
>>  void MainWindow::saveImageAs()
>>  {
>>  	QImage image = viewfinder_->getCurrentImage();
>> +	QString defaultPath = QStandardPaths::standardLocations(QStandardPaths::PicturesLocation).last();
> 
> QStandardPaths::standardLocations() sorts the result from high to low
> priority, shouldn't we thus pick the first entry ? How about using
> QStandardPaths::writableLocation() instead ?


Ok, indeed I see it returns arrays like the following on Linux:

> FontsLocation	"~/.fonts", "~/.local/share/fonts", "/usr/local/share/fonts", "/usr/share/fonts"
> ApplicationsLocation	"~/.local/share/applications", "/usr/local/share/applications", "/usr/share/applications"
> MusicLocation	"~/Music"
> MoviesLocation	"~/Videos"
> PicturesLocation	"~/Pictures"


So for PicturesLocation first() == last() anyway :-)

But I'm fine with QStandardPaths::writableLocation(), and that reduces
the line length.


> 
>>  
>> -	QString filename = QFileDialog::getSaveFileName(this, "Save Image", "",
>> +	QString filename = QFileDialog::getSaveFileName(this, "Save Image", defaultPath,
>>  							"Image Files (*.png *.jpg *.jpeg)");
>>  
>>  	std::cout << "Save image to " << filename.toStdString() << std::endl;
>
Laurent Pinchart Feb. 18, 2020, 9:20 a.m. UTC | #3 
Hi Kieran,

On Tue, Feb 18, 2020 at 09:02:18AM +0000, Kieran Bingham wrote:
> On 18/02/2020 01:05, Laurent Pinchart wrote:
> > On Mon, Feb 17, 2020 at 03:39:41PM +0000, Kieran Bingham wrote:
> >> Utilise the QStandardPaths::PicturesLocation as a starting point for
> >> saving images from qcam.
> >>
> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> >> ---
> >>  src/qcam/main_window.cpp | 4 +++-
> >>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> >> index 29eaba8454f8..09e4bfc11300 100644
> >> --- a/src/qcam/main_window.cpp
> >> +++ b/src/qcam/main_window.cpp
> >> @@ -17,6 +17,7 @@
> >>  #include <QImage>
> >>  #include <QImageWriter>
> >>  #include <QInputDialog>
> >> +#include <QStandardPaths>
> >>  #include <QTimer>
> >>  #include <QToolBar>
> >>  #include <QToolButton>
> >> @@ -348,8 +349,9 @@ void MainWindow::stopCapture()
> >>  void MainWindow::saveImageAs()
> >>  {
> >>  	QImage image = viewfinder_->getCurrentImage();
> >> +	QString defaultPath = QStandardPaths::standardLocations(QStandardPaths::PicturesLocation).last();
> > 
> > QStandardPaths::standardLocations() sorts the result from high to low
> > priority, shouldn't we thus pick the first entry ? How about using
> > QStandardPaths::writableLocation() instead ?
> 
> Ok, indeed I see it returns arrays like the following on Linux:
> 
> > FontsLocation	"~/.fonts", "~/.local/share/fonts", "/usr/local/share/fonts", "/usr/share/fonts"
> > ApplicationsLocation	"~/.local/share/applications", "/usr/local/share/applications", "/usr/share/applications"
> > MusicLocation	"~/Music"
> > MoviesLocation	"~/Videos"
> > PicturesLocation	"~/Pictures"
> 
> So for PicturesLocation first() == last() anyway :-)
> 
> But I'm fine with QStandardPaths::writableLocation(), and that reduces
> the line length.

With that change,

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

> >>  
> >> -	QString filename = QFileDialog::getSaveFileName(this, "Save Image", "",
> >> +	QString filename = QFileDialog::getSaveFileName(this, "Save Image", defaultPath,
> >>  							"Image Files (*.png *.jpg *.jpeg)");
> >>  
> >>  	std::cout << "Save image to " << filename.toStdString() << std::endl;
Niklas Söderlund Feb. 18, 2020, 8:25 p.m. UTC | #4 
Hi Kieran,

Thanks for your patch.

On 2020-02-17 15:39:41 +0000, Kieran Bingham wrote:
> Utilise the QStandardPaths::PicturesLocation as a starting point for
> saving images from qcam.

I feel applications that use these default locations for file types are 
annoying and anti the *NIX way of working, which to me is to default all 
interactions to the current working directory.

I don't feel strongly about this so I won't block this change just want 
to voice that ;-)

> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  src/qcam/main_window.cpp | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 29eaba8454f8..09e4bfc11300 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -17,6 +17,7 @@
>  #include <QImage>
>  #include <QImageWriter>
>  #include <QInputDialog>
> +#include <QStandardPaths>
>  #include <QTimer>
>  #include <QToolBar>
>  #include <QToolButton>
> @@ -348,8 +349,9 @@ void MainWindow::stopCapture()
>  void MainWindow::saveImageAs()
>  {
>  	QImage image = viewfinder_->getCurrentImage();
> +	QString defaultPath = QStandardPaths::standardLocations(QStandardPaths::PicturesLocation).last();
>  
> -	QString filename = QFileDialog::getSaveFileName(this, "Save Image", "",
> +	QString filename = QFileDialog::getSaveFileName(this, "Save Image", defaultPath,
>  							"Image Files (*.png *.jpg *.jpeg)");
>  
>  	std::cout << "Save image to " << filename.toStdString() << std::endl;
> -- 
> 2.20.1
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Kieran Bingham Feb. 19, 2020, 9:23 a.m. UTC | #5 
Hi Niklas,

On 18/02/2020 20:25, Niklas Söderlund wrote:
> Hi Kieran,
> 
> Thanks for your patch.
> 
> On 2020-02-17 15:39:41 +0000, Kieran Bingham wrote:
>> Utilise the QStandardPaths::PicturesLocation as a starting point for
>> saving images from qcam.
> 
> I feel applications that use these default locations for file types are 
> annoying and anti the *NIX way of working, which to me is to default all 
> interactions to the current working directory.

My only issue with that, is

"What is the current working directory for an application started from a
GUI interface of an 'installed' qcam?" (/usr/bin/?)

In my instance, I have added QCam to the gnome menu. So now when I
launch it, I want the output saved in a better default location.



> I don't feel strongly about this so I won't block this change just want 
> to voice that ;-)

I understand, heard, but I'm afraid I disagree in this instance :-)

--
Kieran


> 
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>> ---
>>  src/qcam/main_window.cpp | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
>> index 29eaba8454f8..09e4bfc11300 100644
>> --- a/src/qcam/main_window.cpp
>> +++ b/src/qcam/main_window.cpp
>> @@ -17,6 +17,7 @@
>>  #include <QImage>
>>  #include <QImageWriter>
>>  #include <QInputDialog>
>> +#include <QStandardPaths>
>>  #include <QTimer>
>>  #include <QToolBar>
>>  #include <QToolButton>
>> @@ -348,8 +349,9 @@ void MainWindow::stopCapture()
>>  void MainWindow::saveImageAs()
>>  {
>>  	QImage image = viewfinder_->getCurrentImage();
>> +	QString defaultPath = QStandardPaths::standardLocations(QStandardPaths::PicturesLocation).last();
>>  
>> -	QString filename = QFileDialog::getSaveFileName(this, "Save Image", "",
>> +	QString filename = QFileDialog::getSaveFileName(this, "Save Image", defaultPath,
>>  							"Image Files (*.png *.jpg *.jpeg)");
>>  
>>  	std::cout << "Save image to " << filename.toStdString() << std::endl;
>> -- 
>> 2.20.1
>>
>> _______________________________________________
>> libcamera-devel mailing list
>> libcamera-devel@lists.libcamera.org
>> https://lists.libcamera.org/listinfo/libcamera-devel
>

Patch
diff mbox series 

diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
index 29eaba8454f8..09e4bfc11300 100644
--- a/src/qcam/main_window.cpp
+++ b/src/qcam/main_window.cpp
@@ -17,6 +17,7 @@ 
 #include <QImage>
 #include <QImageWriter>
 #include <QInputDialog>
+#include <QStandardPaths>
 #include <QTimer>
 #include <QToolBar>
 #include <QToolButton>
@@ -348,8 +349,9 @@  void MainWindow::stopCapture()
 void MainWindow::saveImageAs()
 {
 	QImage image = viewfinder_->getCurrentImage();
+	QString defaultPath = QStandardPaths::standardLocations(QStandardPaths::PicturesLocation).last();
 
-	QString filename = QFileDialog::getSaveFileName(this, "Save Image", "",
+	QString filename = QFileDialog::getSaveFileName(this, "Save Image", defaultPath,
 							"Image Files (*.png *.jpg *.jpeg)");
 
 	std::cout << "Save image to " << filename.toStdString() << std::endl;