AVFImageCaptureControl - do not block the capture's callback

It's possible to initiate an asynchronous image capture
(via AVCaptureStillImageOutput object) and immediately
stop AVCaptureSession. In this case image capture's callback
can potentially block forever, waiting for a semaphore: (if)
no new frames arrive after 'stop' (on whatever thread
AVFoundation/GDC chooses), this semaphore is never released.
To avoid this we try to acquire a semaphore with a (reasonable)
timeout and report an error in case of failure. To make sure
we are not leaking a semaphore and not creating a danling pointer,
we use a QSharedPointer now.

Task-number: QTBUG-61367
Change-Id: I208cd463f843bc807b53b23ac9651aab0382775a
Reviewed-by: Christian Stromme <christian.stromme@qt.io>
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>

2 files changed, 23 insertions, 7 deletions

diff --git a/src/plugins/avfoundation/camera/avfimagecapturecontrol.h b/src/plugins/avfoundation/camera/avfimagecapturecontrol.h
index a0f3cf8ba..0671034e3 100644
--- a/src/plugins/avfoundation/camera/avfimagecapturecontrol.h
+++ b/src/plugins/avfoundation/camera/avfimagecapturecontrol.h
@@ -44,6 +44,7 @@
 
 #include <QtCore/qqueue.h>
 #include <QtCore/qsemaphore.h>
+#include <QtCore/qsharedpointer.h>
 #include <QtMultimedia/qcameraimagecapturecontrol.h>
 #include "avfcamerasession.h"
 #include "avfstoragelocation.h"
@@ -56,7 +57,7 @@ Q_OBJECT
 public:
     struct CaptureRequest {
         int captureId;
-        QSemaphore *previewReady;
+        QSharedPointer<QSemaphore> previewReady;
     };
 
     AVFImageCaptureControl(AVFCameraService *service, QObject *parent = 0);
diff --git a/src/plugins/avfoundation/camera/avfimagecapturecontrol.mm b/src/plugins/avfoundation/camera/avfimagecapturecontrol.mm
index b59aa7bfd..2a654c94f 100644
--- a/src/plugins/avfoundation/camera/avfimagecapturecontrol.mm
+++ b/src/plugins/avfoundation/camera/avfimagecapturecontrol.mm
@@ -119,7 +119,7 @@ int AVFImageCaptureControl::capture(const QString &fileName)
 
     qDebugCamera() << "Capture image to" << actualFileName;
 
-    CaptureRequest request = { m_lastCaptureId, new QSemaphore };
+    CaptureRequest request = { m_lastCaptureId, QSharedPointer<QSemaphore>::create()};
     m_requestsMutex.lock();
     m_captureRequests.enqueue(request);
     m_requestsMutex.unlock();
@@ -127,10 +127,6 @@ int AVFImageCaptureControl::capture(const QString &fileName)
     [m_stillImageOutput captureStillImageAsynchronouslyFromConnection:m_videoConnection
                         completionHandler: ^(CMSampleBufferRef imageSampleBuffer, NSError *error) {
 
-        // Wait for the preview to be generated before saving the JPEG
-        request.previewReady->acquire();
-        delete request.previewReady;
-
         if (error) {
             QStringList messageParts;
             messageParts << QString::fromUtf8([[error localizedDescription] UTF8String]);
@@ -144,7 +140,18 @@ int AVFImageCaptureControl::capture(const QString &fileName)
                                       Q_ARG(int, request.captureId),
                                       Q_ARG(int, QCameraImageCapture::ResourceError),
                                       Q_ARG(QString, errorMessage));
-        } else {
+            return;
+        }
+
+        // Wait for the preview to be generated before saving the JPEG.
+        // It is possible to stop camera immediately after trying to capture an
+        // image; this can result in a blocked callback's thread, waiting for a
+        // new viewfinder's frame to arrive/semaphore to be released. It is also
+        // unspecified on which thread this callback gets executed, (probably it's
+        // not the same thread that initiated a capture and stopped the camera),
+        // so we cannot reliably check the camera's status. Instead, we wait
+        // with a timeout and treat a failure to acquire a semaphore as an error.
+        if (request.previewReady->tryAcquire(1, 1000)) {
             qDebugCamera() << "Image capture completed:" << actualFileName;
 
             NSData *nsJpgData = [AVCaptureStillImageOutput jpegStillImageNSDataRepresentation:imageSampleBuffer];
@@ -169,6 +176,14 @@ int AVFImageCaptureControl::capture(const QString &fileName)
                                           Q_ARG(int, QCameraImageCapture::ResourceError),
                                           Q_ARG(QString, errorMessage));
             }
+        } else {
+            const QLatin1String errorMessage("Image capture failed: timed out waiting"
+                                             " for a preview frame.");
+            qDebugCamera() << errorMessage;
+            QMetaObject::invokeMethod(this, "error", Qt::QueuedConnection,
+                                      Q_ARG(int, request.captureId),
+                                      Q_ARG(int, QCameraImageCapture::ResourceError),
+                                      Q_ARG(QString, errorMessage));
         }
     }];