[libcamera-devel] [PATCH v5 2/4] qcam: new viewfinder hierarchy
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sun Sep 6 03:23:28 CEST 2020
Hi Show,
Thank you for the patch.
On Fri, Sep 04, 2020 at 04:43:14PM +0800, Show Liu wrote:
> Create viewfinder base class and rename the original viewfinder
> as default Qt render widget
>
> Signed-off-by: Show Liu <show.liu at linaro.org>
> ---
> src/qcam/meson.build | 4 +-
> src/qcam/viewfinder.h | 60 ++++-------------
> .../{viewfinder.cpp => viewfinder_qt.cpp} | 24 +++----
> src/qcam/viewfinder_qt.h | 67 +++++++++++++++++++
> 4 files changed, 94 insertions(+), 61 deletions(-)
> rename src/qcam/{viewfinder.cpp => viewfinder_qt.cpp} (86%)
> create mode 100644 src/qcam/viewfinder_qt.h
>
> diff --git a/src/qcam/meson.build b/src/qcam/meson.build
> index e0c6f26..a4bad0a 100644
> --- a/src/qcam/meson.build
> +++ b/src/qcam/meson.build
> @@ -6,12 +6,12 @@ qcam_sources = files([
> 'format_converter.cpp',
> 'main.cpp',
> 'main_window.cpp',
> - 'viewfinder.cpp',
> + 'viewfinder_qt.cpp',
> ])
>
> qcam_moc_headers = files([
> 'main_window.h',
> - 'viewfinder.h',
> + 'viewfinder_qt.h',
> ])
>
> qcam_resources = files([
> diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h
> index 26a1320..fa7785f 100644
> --- a/src/qcam/viewfinder.h
> +++ b/src/qcam/viewfinder.h
> @@ -2,70 +2,36 @@
> /*
> * Copyright (C) 2019, Google Inc.
> *
> - * viewfinder.h - qcam - Viewfinder
> + * viewfinder.h - qcam - Viewfinder base class
> */
> #ifndef __QCAM_VIEWFINDER_H__
> #define __QCAM_VIEWFINDER_H__
>
> -#include <stddef.h>
> -
> -#include <QIcon>
> -#include <QList>
> #include <QImage>
> -#include <QMutex>
> +#include <QList>
> +#include <QMap>
QMap isn't needed.
> #include <QSize>
> -#include <QWidget>
>
> #include <libcamera/buffer.h>
> -#include <libcamera/pixel_format.h>
> -
> -#include "format_converter.h"
> -
> -class QImage;
> +#include <libcamera/formats.h>
>
> struct MappedBuffer {
> void *memory;
> size_t size;
> };
>
> -class ViewFinder : public QWidget
> +class ViewFinder
> {
> - Q_OBJECT
> -
> public:
> - ViewFinder(QWidget *parent);
> - ~ViewFinder();
> -
> - const QList<libcamera::PixelFormat> &nativeFormats() const;
> + ViewFinder(){};
You don't need to declare an empty constructor, you can just leave it
out.
> + virtual ~ViewFinder(){};
Missing space before {}, and no need for a ; at the end of the line.
>
> - int setFormat(const libcamera::PixelFormat &format, const QSize &size);
> - void render(libcamera::FrameBuffer *buffer, MappedBuffer *map);
> - void stop();
> + virtual const QList<libcamera::PixelFormat> &nativeFormats() const = 0;
>
> - QImage getCurrentImage();
> + virtual int setFormat(const libcamera::PixelFormat &format, const QSize &size) = 0;
> + virtual void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) = 0;
> + virtual void stop() = 0;
>
> -Q_SIGNALS:
> - void renderComplete(libcamera::FrameBuffer *buffer);
> -
> -protected:
> - void paintEvent(QPaintEvent *) override;
> - QSize sizeHint() const override;
> -
> -private:
> - FormatConverter converter_;
> -
> - libcamera::PixelFormat format_;
> - QSize size_;
> -
> - /* Camera stopped icon */
> - QSize vfSize_;
> - QIcon icon_;
> - QPixmap pixmap_;
> -
> - /* Buffer and render image */
> - libcamera::FrameBuffer *buffer_;
> - QImage image_;
> - QMutex mutex_; /* Prevent concurrent access to image_ */
> + virtual QImage getCurrentImage() = 0;
> };
> -
You can keep the blank line here.
> -#endif /* __QCAM_VIEWFINDER__ */
> +#endif /* __QCAM_VIEWFINDER_H__ */
Oops, good catch :-)
> diff --git a/src/qcam/viewfinder.cpp b/src/qcam/viewfinder_qt.cpp
> similarity index 86%
> rename from src/qcam/viewfinder.cpp
> rename to src/qcam/viewfinder_qt.cpp
> index dcf8a15..072f024 100644
> --- a/src/qcam/viewfinder.cpp
> +++ b/src/qcam/viewfinder_qt.cpp
> @@ -2,10 +2,10 @@
> /*
> * Copyright (C) 2019, Google Inc.
> *
> - * viewfinder.cpp - qcam - Viewfinder
> + * viewfinder_qt.cpp - qcam - default Viewfinder for rendering by Qt
How about "QPainter-based ViewFinder" ? Same for the header file below.
> */
>
> -#include "viewfinder.h"
> +#include "viewfinder_qt.h"
>
> #include <stdint.h>
> #include <utility>
> @@ -33,24 +33,24 @@ static const QMap<libcamera::PixelFormat, QImage::Format> nativeFormats
> { libcamera::formats::BGR888, QImage::Format_RGB888 },
> };
>
> -ViewFinder::ViewFinder(QWidget *parent)
> +ViewFinderQt::ViewFinderQt(QWidget *parent)
> : QWidget(parent), buffer_(nullptr)
> {
> icon_ = QIcon(":camera-off.svg");
> }
>
> -ViewFinder::~ViewFinder()
> +ViewFinderQt::~ViewFinderQt()
> {
> }
>
> -const QList<libcamera::PixelFormat> &ViewFinder::nativeFormats() const
> +const QList<libcamera::PixelFormat> &ViewFinderQt::nativeFormats() const
> {
> static const QList<libcamera::PixelFormat> formats = ::nativeFormats.keys();
> return formats;
> }
>
> -int ViewFinder::setFormat(const libcamera::PixelFormat &format,
> - const QSize &size)
> +int ViewFinderQt::setFormat(const libcamera::PixelFormat &format,
> + const QSize &size)
> {
> image_ = QImage();
>
> @@ -78,7 +78,7 @@ int ViewFinder::setFormat(const libcamera::PixelFormat &format,
> return 0;
> }
>
> -void ViewFinder::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)
> +void ViewFinderQt::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)
> {
> if (buffer->planes().size() != 1) {
> qWarning() << "Multi-planar buffers are not supported";
> @@ -121,7 +121,7 @@ void ViewFinder::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)
> renderComplete(buffer);
> }
>
> -void ViewFinder::stop()
> +void ViewFinderQt::stop()
> {
> image_ = QImage();
>
> @@ -133,14 +133,14 @@ void ViewFinder::stop()
> update();
> }
>
> -QImage ViewFinder::getCurrentImage()
> +QImage ViewFinderQt::getCurrentImage()
> {
> QMutexLocker locker(&mutex_);
>
> return image_.copy();
> }
>
> -void ViewFinder::paintEvent(QPaintEvent *)
> +void ViewFinderQt::paintEvent(QPaintEvent *)
> {
> QPainter painter(this);
>
> @@ -175,7 +175,7 @@ void ViewFinder::paintEvent(QPaintEvent *)
> painter.drawPixmap(point, pixmap_);
> }
>
> -QSize ViewFinder::sizeHint() const
> +QSize ViewFinderQt::sizeHint() const
> {
> return size_.isValid() ? size_ : QSize(640, 480);
> }
> diff --git a/src/qcam/viewfinder_qt.h b/src/qcam/viewfinder_qt.h
> new file mode 100644
> index 0000000..ee2abab
> --- /dev/null
> +++ b/src/qcam/viewfinder_qt.h
> @@ -0,0 +1,67 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * viewfinder_qt.h - qcam - default Viewfinder for rendering by Qt
> + */
> +#ifndef __QCAM_VIEWFINDER_QT_H__
> +#define __QCAM_VIEWFINDER_QT_H__
> +
> +#include <stddef.h>
stddef.h doesn't seem to be needed.
> +
> +#include <QIcon>
> +#include <QImage>
> +#include <QList>
> +#include <QMutex>
> +#include <QSize>
> +#include <QWidget>
> +
> +#include <libcamera/buffer.h>
> +#include <libcamera/formats.h>
> +#include <libcamera/pixel_format.h>
> +
> +#include "format_converter.h"
> +#include "viewfinder.h"
> +
> +class QImage;
You can drop this, QImage is included.
> +
> +class ViewFinderQt : public QWidget, public ViewFinder
> +{
> + Q_OBJECT
> +
> +public:
> + ViewFinderQt(QWidget *parent);
> + ~ViewFinderQt();
> +
> + const QList<libcamera::PixelFormat> &nativeFormats() const override;
> +
> + int setFormat(const libcamera::PixelFormat &format, const QSize &size) override;
> + void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override;
> + void stop() override;
> +
> + QImage getCurrentImage() override;
> +
> +Q_SIGNALS:
> + void renderComplete(libcamera::FrameBuffer *buffer);
> +
> +protected:
> + void paintEvent(QPaintEvent *) override;
> + QSize sizeHint() const override;
> +
> +private:
> + FormatConverter converter_;
> +
> + libcamera::PixelFormat format_;
> + QSize size_;
> +
> + /* Camera stopped icon */
> + QSize vfSize_;
> + QIcon icon_;
> + QPixmap pixmap_;
> +
> + /* Buffer and render image */
> + libcamera::FrameBuffer *buffer_;
> + QImage image_;
> + QMutex mutex_; /* Prevent concurrent access to image_ */
> +};
Missing blank line.
With these small issues addressed,
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> +#endif /* __QCAM_VIEWFINDER_QT_H__ */
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list