From 2083f7a805249490241bc07b25a33161dc0c6695 Mon Sep 17 00:00:00 2001 From: Nick Diego Yamane Date: Wed, 18 Jan 2017 13:33:17 -0400 Subject: [PATCH 1/2] QZXingFilter: Fix crash when destructing QZXingFilter QZXingFilterRunnable runs QFuture to perform decoder::decodeImage() and stores it in a class var of its owner (QZXingFilter), but when QZXingFilter is destructed it was not canceling that QFuture, making possible to the QFuture to try to access filter->decoder->decodeImage() after filter's are already destructed (dangling pointer). Issue detected when using QZXingFilter QML Element in an Page pushed in a StackView. When the page containing the QZXingFilter is poped from the StackView, (by default) the filter is unloaded/destructed, what makes the app crash without this patch. Signed-off-by: Nick Diego Yamane --- src/QZXingFilter.cpp | 22 ++++++++++++++++++---- src/QZXingFilter.h | 4 ++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/QZXingFilter.cpp b/src/QZXingFilter.cpp index 2a22562..1082fd1 100644 --- a/src/QZXingFilter.cpp +++ b/src/QZXingFilter.cpp @@ -39,6 +39,10 @@ QZXingFilter::QZXingFilter(QObject *parent) QZXingFilter::~QZXingFilter() { + if(!processThread.isFinished()) { + processThread.cancel(); + processThread.waitForFinished(); + } if(decoder_p) delete decoder_p; } @@ -82,6 +86,10 @@ QZXingFilterRunnable::QZXingFilterRunnable(QZXingFilter * filter) { } +QZXingFilterRunnable::~QZXingFilterRunnable() +{ + filter = nullptr; +} QVideoFrame QZXingFilterRunnable::run(QVideoFrame * input, const QVideoSurfaceFormat &surfaceFormat, RunFlags flags) { @@ -261,7 +269,7 @@ void QZXingFilterRunnable::processVideoFrameProbed(SimpleVideoFrame & videoFrame // const QString path = QStandardPaths::writableLocation(QStandardPaths::PicturesLocation) + "/qrtest/test_" + QString::number(i % 100) + ".png"; // qDebug() << "saving image" << i << "at:" << path << image.save(path); - QString tag = filter->decoder_p->decodeImage(image, image.width(), image.height()); + QString tag = decode(image); const bool tryHarder = filter->decoder_p->getTryHarder(); /// The frames we get from the camera may be reflected horizontally or vertically @@ -269,14 +277,20 @@ void QZXingFilterRunnable::processVideoFrameProbed(SimpleVideoFrame & videoFrame /// TODO: Maybe there is a better way to know this orientation beforehand? Or should we try decoding all of them? if (tag.isEmpty() && tryHarder) { image = image.mirrored(true, false); - tag = filter->decoder_p->decodeImage(image, image.width(), image.height()); + tag = decode(image); } if (tag.isEmpty() && tryHarder) { image = image.mirrored(false, true); - tag = filter->decoder_p->decodeImage(image, image.width(), image.height()); + tag = decode(image); } if (tag.isEmpty() && tryHarder) { image = image.mirrored(true, true); - tag = filter->decoder_p->decodeImage(image, image.width(), image.height()); + tag = decode(image); } } + +QString QZXingFilterRunnable::decode(const QImage &image) +{ + return (filter != nullptr) ? + filter->decoder_p->decodeImage(image, image.width(), image.height()) : QString(); +} diff --git a/src/QZXingFilter.h b/src/QZXingFilter.h index cefef5b..e21e610 100644 --- a/src/QZXingFilter.h +++ b/src/QZXingFilter.h @@ -103,10 +103,14 @@ class QZXingFilterRunnable : public QObject, public QVideoFilterRunnable public: explicit QZXingFilterRunnable(QZXingFilter * filter); + virtual ~QZXingFilterRunnable(); /// This method is called whenever we get a new frame. It runs in the UI thread. QVideoFrame run(QVideoFrame * input, const QVideoSurfaceFormat &surfaceFormat, RunFlags flags); void processVideoFrameProbed(SimpleVideoFrame & videoFrame, const QRect& captureRect); + private: + QString decode(const QImage &image); + private: QZXingFilter * filter; }; From 557a52b17daf6e673917c50756a258d73a3dd227 Mon Sep 17 00:00:00 2001 From: Nick Diego Yamane Date: Wed, 18 Jan 2017 19:12:45 -0400 Subject: [PATCH 2/2] QZXingFilter: Enforce captureRect validation captureRect is accessible and modifyable via QML, so that it can take arbitrary values, what can make QZXingFilter to crash the application. This issue was experienced when running code similar to QZXingLive example on an android device, where captureRect is bound based on VideoOutput's contentRect and sourceRect, when QML Engine is calculating layout for the pages it sets temp values to width/height of elements (including VideoOutput) what causes captureRect to be set to invalid values for the QZXingFilter's perspective, such as negative values for x and y. Signed-off-by: Nick Diego Yamane --- src/QZXingFilter.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/QZXingFilter.cpp b/src/QZXingFilter.cpp index 1082fd1..a506608 100644 --- a/src/QZXingFilter.cpp +++ b/src/QZXingFilter.cpp @@ -126,6 +126,11 @@ QVideoFrame QZXingFilterRunnable::run(QVideoFrame * input, const QVideoSurfaceFo return * input; } +static bool isRectValid(const QRect& rect) +{ + return rect.x() > 0 && rect.y() > 0 && rect.isValid(); +} + static QImage rgbDataToGrayscale(const uchar* data, const int width, const int height, const int alpha, const int red, const int green, const int blue, @@ -134,10 +139,10 @@ static QImage rgbDataToGrayscale(const uchar* data, const int width, const int h { const int stride = (alpha < 0) ? 3 : 4; - const int startX = captureRect.x(); - const int startY = captureRect.y(); - const int targetWidth = captureRect.isNull() ? width : captureRect.width(); - const int targetHeight = captureRect.isNull() ? height : captureRect.height(); + const int startX = isRectValid(captureRect) ? captureRect.x() : 0; + const int startY = isRectValid(captureRect) ? captureRect.y() : 0; + const int targetWidth = isRectValid(captureRect) ? captureRect.width() : width; + const int targetHeight = isRectValid(captureRect) ? captureRect.height() : height; const int endX = width - startX - targetWidth; const int skipX = (endX + startX) * stride;