[{"id":12465,"web_url":"https://patchwork.libcamera.org/comment/12465/","msgid":"<20200912012540.GL6808@pendragon.ideasonboard.com>","date":"2020-09-12T01:25:40","subject":"Re: [libcamera-devel] [PATCH v6 2/4] qcam: new viewfinder hierarchy","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Show,\n\nThank you for the patch.\n\nOn Fri, Sep 11, 2020 at 04:55:12PM +0800, Show Liu wrote:\n> Create viewfinder base class and rename the original viewfinder\n\ns/viewfinder/ViewFinder/\n\n> as QPainter-based ViewFinder\n> \n> Signed-off-by: Show Liu \n> ---\n> src/qcam/meson.build | 4 +-\n> src/qcam/viewfinder.h | 57 ++++-------------\n> .../{viewfinder.cpp => viewfinder_qt.cpp} | 24 +++----\n> src/qcam/viewfinder_qt.h | 64 +++++++++++++++++++\n> 4 files changed, 89 insertions(+), 60 deletions(-)\n> rename src/qcam/{viewfinder.cpp => viewfinder_qt.cpp} (86%)\n> create mode 100644 src/qcam/viewfinder_qt.h\n\nThis doesn't compile. The following patch fixes it.\n\ndiff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\nindex 612d978a73df..7406f0bd4512 100644\n--- a/src/qcam/main_window.cpp\n+++ b/src/qcam/main_window.cpp\n@@ -28,6 +28,7 @@\n #include \n\n #include \"dng_writer.h\"\n+#include \"viewfinder_qt.h\"\n\n using namespace libcamera;\n\n@@ -105,10 +106,11 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options)\n \tsetWindowTitle(title_);\n \tconnect(&titleTimer_, SIGNAL(timeout()), this, SLOT(updateTitle()));\n\n-\tviewfinder_ = new ViewFinder(this);\n-\tconnect(viewfinder_, &ViewFinder::renderComplete,\n+\tViewFinderQt *viewfinder = new ViewFinderQt(this);\n+\tconnect(viewfinder, &ViewFinderQt::renderComplete,\n \t\tthis, &MainWindow::queueRequest);\n-\tsetCentralWidget(viewfinder_);\n+\tviewfinder_ = viewfinder;\n+\tsetCentralWidget(viewfinder);\n \tadjustSize();\n\n \t/* Hotplug/unplug support */\n\nWith this,\n\nReviewed-by: Laurent Pinchart \n\nI'll apply the above diff when applying the patch, no need for a v7 just\nfor this.\n\n> diff --git a/src/qcam/meson.build b/src/qcam/meson.build\n> index e0c6f26..a4bad0a 100644\n> --- a/src/qcam/meson.build\n> +++ b/src/qcam/meson.build\n> @@ -6,12 +6,12 @@ qcam_sources = files([\n> 'format_converter.cpp',\n> 'main.cpp',\n> 'main_window.cpp',\n> - 'viewfinder.cpp',\n> + 'viewfinder_qt.cpp',\n> ])\n> \n> qcam_moc_headers = files([\n> 'main_window.h',\n> - 'viewfinder.h',\n> + 'viewfinder_qt.h',\n> ])\n> \n> qcam_resources = files([\n> diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h\n> index 26a1320..67da1df 100644\n> --- a/src/qcam/viewfinder.h\n> +++ b/src/qcam/viewfinder.h\n> @@ -2,70 +2,35 @@\n> /*\n> * Copyright (C) 2019, Google Inc.\n> *\n> - * viewfinder.h - qcam - Viewfinder\n> + * viewfinder.h - qcam - Viewfinder base class\n> */\n> #ifndef __QCAM_VIEWFINDER_H__\n> #define __QCAM_VIEWFINDER_H__\n> \n> -#include \n> -\n> -#include \n> -#include \n> #include \n> -#include \n> +#include \n> #include \n> -#include \n> \n> #include \n> -#include \n> -\n> -#include \"format_converter.h\"\n> -\n> -class QImage;\n> +#include \n> \n> struct MappedBuffer {\n> \tvoid *memory;\n> \tsize_t size;\n> };\n> \n> -class ViewFinder : public QWidget\n> +class ViewFinder\n> {\n> -\tQ_OBJECT\n> -\n> public:\n> -\tViewFinder(QWidget *parent);\n> -\t~ViewFinder();\n> -\n> -\tconst QList &nativeFormats() const;\n> -\n> -\tint setFormat(const libcamera::PixelFormat &format, const QSize &size);\n> -\tvoid render(libcamera::FrameBuffer *buffer, MappedBuffer *map);\n> -\tvoid stop();\n> -\n> -\tQImage getCurrentImage();\n> -\n> -Q_SIGNALS:\n> -\tvoid renderComplete(libcamera::FrameBuffer *buffer);\n> -\n> -protected:\n> -\tvoid paintEvent(QPaintEvent *) override;\n> -\tQSize sizeHint() const override;\n> -\n> -private:\n> -\tFormatConverter converter_;\n> +\tvirtual ~ViewFinder() {}\n> \n> -\tlibcamera::PixelFormat format_;\n> -\tQSize size_;\n> +\tvirtual const QList &nativeFormats() const = 0;\n> \n> -\t/* Camera stopped icon */\n> -\tQSize vfSize_;\n> -\tQIcon icon_;\n> -\tQPixmap pixmap_;\n> +\tvirtual int setFormat(const libcamera::PixelFormat &format, const QSize &size) = 0;\n> +\tvirtual void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) = 0;\n> +\tvirtual void stop() = 0;\n> \n> -\t/* Buffer and render image */\n> -\tlibcamera::FrameBuffer *buffer_;\n> -\tQImage image_;\n> -\tQMutex mutex_; /* Prevent concurrent access to image_ */\n> +\tvirtual QImage getCurrentImage() = 0;\n> };\n> \n> -#endif /* __QCAM_VIEWFINDER__ */\n> +#endif /* __QCAM_VIEWFINDER_H__ */\n> diff --git a/src/qcam/viewfinder.cpp b/src/qcam/viewfinder_qt.cpp\n> similarity index 86%\n> rename from src/qcam/viewfinder.cpp\n> rename to src/qcam/viewfinder_qt.cpp\n> index dcf8a15..e436714 100644\n> --- a/src/qcam/viewfinder.cpp\n> +++ b/src/qcam/viewfinder_qt.cpp\n> @@ -2,10 +2,10 @@\n> /*\n> * Copyright (C) 2019, Google Inc.\n> *\n> - * viewfinder.cpp - qcam - Viewfinder\n> + * viewfinder_qt.cpp - qcam - QPainter-based ViewFinder\n> */\n> \n> -#include \"viewfinder.h\"\n> +#include \"viewfinder_qt.h\"\n> \n> #include \n> #include \n> @@ -33,24 +33,24 @@ static const QMap nativeFormats\n> \t{ libcamera::formats::BGR888, QImage::Format_RGB888 },\n> };\n> \n> -ViewFinder::ViewFinder(QWidget *parent)\n> +ViewFinderQt::ViewFinderQt(QWidget *parent)\n> \t: QWidget(parent), buffer_(nullptr)\n> {\n> \ticon_ = QIcon(\":camera-off.svg\");\n> }\n> \n> -ViewFinder::~ViewFinder()\n> +ViewFinderQt::~ViewFinderQt()\n> {\n> }\n> \n> -const QList &ViewFinder::nativeFormats() const\n> +const QList &ViewFinderQt::nativeFormats() const\n> {\n> \tstatic const QList formats = ::nativeFormats.keys();\n> \treturn formats;\n> }\n> \n> -int ViewFinder::setFormat(const libcamera::PixelFormat &format,\n> -\t\t\t const QSize &size)\n> +int ViewFinderQt::setFormat(const libcamera::PixelFormat &format,\n> +\t\t\t const QSize &size)\n> {\n> \timage_ = QImage();\n> \n> @@ -78,7 +78,7 @@ int ViewFinder::setFormat(const libcamera::PixelFormat &format,\n> \treturn 0;\n> }\n> \n> -void ViewFinder::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)\n> +void ViewFinderQt::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)\n> {\n> \tif (buffer->planes().size() != 1) {\n> \t\tqWarning() << \"Multi-planar buffers are not supported\";\n> @@ -121,7 +121,7 @@ void ViewFinder::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)\n> \t\trenderComplete(buffer);\n> }\n> \n> -void ViewFinder::stop()\n> +void ViewFinderQt::stop()\n> {\n> \timage_ = QImage();\n> \n> @@ -133,14 +133,14 @@ void ViewFinder::stop()\n> \tupdate();\n> }\n> \n> -QImage ViewFinder::getCurrentImage()\n> +QImage ViewFinderQt::getCurrentImage()\n> {\n> \tQMutexLocker locker(&mutex_);\n> \n> \treturn image_.copy();\n> }\n> \n> -void ViewFinder::paintEvent(QPaintEvent *)\n> +void ViewFinderQt::paintEvent(QPaintEvent *)\n> {\n> \tQPainter painter(this);\n> \n> @@ -175,7 +175,7 @@ void ViewFinder::paintEvent(QPaintEvent *)\n> \tpainter.drawPixmap(point, pixmap_);\n> }\n> \n> -QSize ViewFinder::sizeHint() const\n> +QSize ViewFinderQt::sizeHint() const\n> {\n> \treturn size_.isValid() ? size_ : QSize(640, 480);\n> }\n> diff --git a/src/qcam/viewfinder_qt.h b/src/qcam/viewfinder_qt.h\n> new file mode 100644\n> index 0000000..d755428\n> --- /dev/null\n> +++ b/src/qcam/viewfinder_qt.h\n> @@ -0,0 +1,64 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * viewfinder_qt.h - qcam - QPainter-based ViewFinder\n> + */\n> +#ifndef __QCAM_VIEWFINDER_QT_H__\n> +#define __QCAM_VIEWFINDER_QT_H__\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +#include \n> +\n> +#include \"format_converter.h\"\n> +#include \"viewfinder.h\"\n> +\n> +class ViewFinderQt : public QWidget, public ViewFinder\n> +{\n> +\tQ_OBJECT\n> +\n> +public:\n> +\tViewFinderQt(QWidget *parent);\n> +\t~ViewFinderQt();\n> +\n> +\tconst QList &nativeFormats() const override;\n> +\n> +\tint setFormat(const libcamera::PixelFormat &format, const QSize &size) override;\n> +\tvoid render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override;\n> +\tvoid stop() override;\n> +\n> +\tQImage getCurrentImage() override;\n> +\n> +Q_SIGNALS:\n> +\tvoid renderComplete(libcamera::FrameBuffer *buffer);\n> +\n> +protected:\n> +\tvoid paintEvent(QPaintEvent *) override;\n> +\tQSize sizeHint() const override;\n> +\n> +private:\n> +\tFormatConverter converter_;\n> +\n> +\tlibcamera::PixelFormat format_;\n> +\tQSize size_;\n> +\n> +\t/* Camera stopped icon */\n> +\tQSize vfSize_;\n> +\tQIcon icon_;\n> +\tQPixmap pixmap_;\n> +\n> +\t/* Buffer and render image */\n> +\tlibcamera::FrameBuffer *buffer_;\n> +\tQImage image_;\n> +\tQMutex mutex_; /* Prevent concurrent access to image_ */\n> +};\n> +\n> +#endif /* __QCAM_VIEWFINDER_QT_H__ */","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 51E90BF01C\n\tfor ;\n\tSat, 12 Sep 2020 01:26:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CC33F62D5B;\n\tSat, 12 Sep 2020 03:26:11 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 41BDE62C43\n\tfor ;\n\tSat, 12 Sep 2020 03:26:10 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8835326B;\n\tSat, 12 Sep 2020 03:26:08 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"KWeAcCGo\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1599873968;\n\tbh=yQk5v1qZBd5MCRkwHz+KYzTZqTvwYsdE5NoTb0xMPak=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=KWeAcCGoGZMabIRTrvHWFQIWnNw+JOLkKEwkdD8tVKnjtB1n2lzcKC4B/ZP2X2tC6\n\t8BArZoczlFx1eNa4Pp4W3JAPszW7uPaFtkXRUsruQELCgMtKRY/d6zuUltfep3BpIi\n\t2jtE7ZWlYCYQCWpxLs+7H2971ENHACOaDg6+/rmc=","Date":"Sat, 12 Sep 2020 04:25:40 +0300","From":"Laurent Pinchart ","To":"Show Liu ","Message-ID":"<20200912012540.GL6808@pendragon.ideasonboard.com>","References":"<20200911085514.30021-1-show.liu@linaro.org>\n\t<20200911085514.30021-3-show.liu@linaro.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200911085514.30021-3-show.liu@linaro.org>","Subject":"Re: [libcamera-devel] [PATCH v6 2/4] qcam: new viewfinder hierarchy","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]