[libcamera-devel] [PATCH v2 7/7] qcam: Provide save image functionality
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri Feb 14 12:36:02 CET 2020
Hi Kieran,
Thank you for the patch.
On Fri, Feb 14, 2020 at 12:18:10AM +0000, Kieran Bingham wrote:
> Implement a save image button on the toolbar which will take a current
> viewfinder image and present the user with a QFileDialog to allow them
> to choose where to save the image.
>
> Utilise the QImageWriter to perform the output task.
>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> ---
> v2:
> - Rename save to saveAs
> - Add empty file check
> - Lock concurrent access to the ViewFinder image
>
> src/qcam/assets/feathericons/feathericons.qrc | 1 +
> src/qcam/main_window.cpp | 22 +++++++++++++++++++
> src/qcam/main_window.h | 1 +
> src/qcam/viewfinder.cpp | 11 ++++++++++
> src/qcam/viewfinder.h | 5 +++++
> 5 files changed, 40 insertions(+)
>
> diff --git a/src/qcam/assets/feathericons/feathericons.qrc b/src/qcam/assets/feathericons/feathericons.qrc
> index b8e5c2266408..6ca3a846803c 100644
> --- a/src/qcam/assets/feathericons/feathericons.qrc
> +++ b/src/qcam/assets/feathericons/feathericons.qrc
> @@ -1,6 +1,7 @@
> <!DOCTYPE RCC><RCC version="1.0">
> <qresource>
> <file>./play-circle.svg</file>
> +<file>./save.svg</file>
> <file>./stop-circle.svg</file>
> <file>./x-circle.svg</file>
> </qresource>
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index ec93e0177b41..8ee2a7d68c96 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -12,7 +12,10 @@
>
> #include <QComboBox>
> #include <QCoreApplication>
> +#include <QFileDialog>
> #include <QIcon>
> +#include <QImage>
> +#include <QImageWriter>
> #include <QInputDialog>
> #include <QTimer>
> #include <QToolBar>
> @@ -88,6 +91,9 @@ int MainWindow::createToolbars()
> action = toolbar_->addAction(QIcon(":stop-circle.svg"), "stop");
> connect(action, &QAction::triggered, this, &MainWindow::stopCapture);
>
> + action = toolbar_->addAction(QIcon(":save.svg"), "saveAs");
> + connect(action, &QAction::triggered, this, &MainWindow::saveImageAs);
> +
> return 0;
> }
>
> @@ -339,6 +345,22 @@ void MainWindow::stopCapture()
> setWindowTitle(title_);
> }
>
> +void MainWindow::saveImageAs()
> +{
> + QImage image = viewfinder_->getCurrentImage();
> +
> + QString filename = QFileDialog::getSaveFileName(this, "Save Image", "",
> + "Image Files (*.png *.jpg *.jpeg)");
> +
> + std::cerr << "Save image to " << filename.toStdString() << std::endl;
Should this be cout ? It's not an error.
> +
> + if (filename == "")
if (filename.isEmpty())
is slightly more efficient.
> + return;
> +
> + QImageWriter writer(filename);
> + writer.write(image);
> +}
> +
> void MainWindow::requestComplete(Request *request)
> {
> if (request->status() == Request::RequestCancelled)
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index 27ceed611d59..dcc39d7de948 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -48,6 +48,7 @@ private Q_SLOTS:
>
> int startCapture();
> void stopCapture();
I'd add a blank line here as it's unrelated.
> + void saveImageAs();
>
> private:
> int createToolbars();
> diff --git a/src/qcam/viewfinder.cpp b/src/qcam/viewfinder.cpp
> index 6de284d1b782..211926e185d3 100644
> --- a/src/qcam/viewfinder.cpp
> +++ b/src/qcam/viewfinder.cpp
> @@ -6,6 +6,8 @@
> */
>
> #include <QImage>
> +#include <QImageWriter>
> +#include <QMutexLocker>
> #include <QPainter>
>
> #include "format_converter.h"
> @@ -23,10 +25,19 @@ ViewFinder::~ViewFinder()
>
> void ViewFinder::display(const unsigned char *raw, size_t size)
> {
> + QMutexLocker locker(&mutex_);
> +
> converter_.convert(raw, size, image_);
> update();
> }
>
> +QImage ViewFinder::getCurrentImage()
> +{
> + QMutexLocker locker(&mutex_);
> +
> + return *image_;
I'm afraid this is still unsafe despite the lock as QImage using
implicit data sharing. You can return image_->copy() instead as a first
step.
> +}
> +
> int ViewFinder::setFormat(unsigned int format, unsigned int width,
> unsigned int height)
> {
> diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h
> index ef5fd45b264a..06e8034fce1d 100644
> --- a/src/qcam/viewfinder.h
> +++ b/src/qcam/viewfinder.h
> @@ -7,6 +7,7 @@
> #ifndef __QCAM_VIEWFINDER_H__
> #define __QCAM_VIEWFINDER_H__
>
> +#include <QMutex>
> #include <QWidget>
>
> #include "format_converter.h"
> @@ -23,6 +24,8 @@ public:
> unsigned int height);
> void display(const unsigned char *rgb, size_t size);
>
> + QImage getCurrentImage();
> +
> protected:
> void paintEvent(QPaintEvent *) override;
> QSize sizeHint() const override;
> @@ -33,7 +36,9 @@ private:
> unsigned int height_;
>
> FormatConverter converter_;
> +
> QImage *image_;
> + QMutex mutex_; // Prevent concurrent access to image_
C-style comments as in the rest of the code base ?
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> };
>
> #endif /* __QCAM_VIEWFINDER__ */
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list