AVFoundation: correctly set the activeFormat on the AVCaptureDevice.

According to the AVCaptureDevice documentation, the device must be
locked before starting the capture session to prevent the activeFormat
from being overridden. We now do that, but only if we explicitly set
an activeFormat. Otherwise the device is not locked, which allows
the session to find an appropriate format for the capture device.
The device is also locked when enabling video capture, as doing so
might also reset the activeFormat.

Task-number: QTBUG-49170
Change-Id: I75478fd4bbfec96cd2abd2c3ae2951088b38978e
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@theqtcompany.com>

7 files changed, 62 insertions, 38 deletions

diff --git a/src/plugins/avfoundation/camera/avfcamerasession.h b/src/plugins/avfoundation/camera/avfcamerasession.h
index 2b322cfaf..1be847b7c 100644
--- a/src/plugins/avfoundation/camera/avfcamerasession.h
+++ b/src/plugins/avfoundation/camera/avfcamerasession.h
@@ -102,8 +102,8 @@ Q_SIGNALS:
 private:
     static void updateCameraDevices();
     void attachVideoInputDevice();
-    void applyImageEncoderSettings();
-    void applyViewfinderSettings();
+    bool applyImageEncoderSettings();
+    bool applyViewfinderSettings();
 
     static int m_defaultCameraIndex;
     static QList<AVFCameraInfo> m_cameraDevices;
diff --git a/src/plugins/avfoundation/camera/avfcamerasession.mm b/src/plugins/avfoundation/camera/avfcamerasession.mm
index 5bf841bec..4ede4a514 100644
--- a/src/plugins/avfoundation/camera/avfcamerasession.mm
+++ b/src/plugins/avfoundation/camera/avfcamerasession.mm
@@ -285,10 +285,23 @@ void AVFCameraSession::setState(QCamera::State newState)
         Q_EMIT readyToConfigureConnections();
         m_defaultCodec = 0;
         defaultCodec();
-        applyImageEncoderSettings();
-        applyViewfinderSettings();
+
+        bool activeFormatSet = applyImageEncoderSettings();
+        activeFormatSet |= applyViewfinderSettings();
+
         [m_captureSession commitConfiguration];
+
+        if (activeFormatSet) {
+            // According to the doc, the capture device must be locked before
+            // startRunning to prevent the format we set to be overriden by the
+            // session preset.
+            [videoCaptureDevice() lockForConfiguration:nil];
+        }
+
         [m_captureSession startRunning];
+
+        if (activeFormatSet)
+            [videoCaptureDevice() unlockForConfiguration];
     }
 
     if (oldState == QCamera::ActiveState) {
@@ -355,13 +368,15 @@ void AVFCameraSession::attachVideoInputDevice()
     }
 }
 
-void AVFCameraSession::applyImageEncoderSettings()
+bool AVFCameraSession::applyImageEncoderSettings()
 {
     if (AVFImageEncoderControl *control = m_service->imageEncoderControl())
-        control->applySettings();
+        return control->applySettings();
+
+    return false;
 }
 
-void AVFCameraSession::applyViewfinderSettings()
+bool AVFCameraSession::applyViewfinderSettings()
 {
     if (AVFCameraViewfinderSettingsControl2 *vfControl = m_service->viewfinderSettingsControl2()) {
         QCameraViewfinderSettings vfSettings(vfControl->requestedSettings());
@@ -372,12 +387,13 @@ void AVFCameraSession::applyViewfinderSettings()
             if (!imageResolution.isNull() && imageResolution.isValid()) {
                 vfSettings.setResolution(imageResolution);
                 vfControl->setViewfinderSettings(vfSettings);
-                return;
             }
         }
 
-        vfControl->applySettings();
+        return vfControl->applySettings();
     }
+
+    return false;
 }
 
 void AVFCameraSession::addProbe(AVFMediaVideoProbeControl *probe)
diff --git a/src/plugins/avfoundation/camera/avfcameraviewfindersettingscontrol.h b/src/plugins/avfoundation/camera/avfcameraviewfindersettingscontrol.h
index 8d9b88c01..9a5bbd5de 100644
--- a/src/plugins/avfoundation/camera/avfcameraviewfindersettingscontrol.h
+++ b/src/plugins/avfoundation/camera/avfcameraviewfindersettingscontrol.h
@@ -76,7 +76,7 @@ private:
     AVCaptureDeviceFormat *findBestFormatMatch(const QCameraViewfinderSettings &settings) const;
     QVector<QVideoFrame::PixelFormat> viewfinderPixelFormats() const;
     bool convertPixelFormatIfSupported(QVideoFrame::PixelFormat format, unsigned &avfFormat) const;
-    void applySettings();
+    bool applySettings();
     QCameraViewfinderSettings requestedSettings() const;
 
     AVCaptureConnection *videoConnection() const;
diff --git a/src/plugins/avfoundation/camera/avfcameraviewfindersettingscontrol.mm b/src/plugins/avfoundation/camera/avfcameraviewfindersettingscontrol.mm
index 95fadb816..3c20801e5 100644
--- a/src/plugins/avfoundation/camera/avfcameraviewfindersettingscontrol.mm
+++ b/src/plugins/avfoundation/camera/avfcameraviewfindersettingscontrol.mm
@@ -384,11 +384,6 @@ QCameraViewfinderSettings AVFCameraViewfinderSettingsControl2::viewfinderSetting
 
 void AVFCameraViewfinderSettingsControl2::setViewfinderSettings(const QCameraViewfinderSettings &settings)
 {
-    if (settings.isNull()) {
-        qDebugCamera() << Q_FUNC_INFO << "empty viewfinder settings";
-        return;
-    }
-
     if (m_settings == settings)
         return;
 
@@ -454,7 +449,7 @@ bool AVFCameraViewfinderSettingsControl2::CVPixelFormatFromQtFormat(QVideoFrame:
 AVCaptureDeviceFormat *AVFCameraViewfinderSettingsControl2::findBestFormatMatch(const QCameraViewfinderSettings &settings) const
 {
     AVCaptureDevice *captureDevice = m_service->session()->videoCaptureDevice();
-    if (!captureDevice)
+    if (!captureDevice || settings.isNull())
         return nil;
 
 #if QT_MAC_PLATFORM_SDK_EQUAL_OR_ABOVE(__MAC_10_7, __IPHONE_7_0)
@@ -553,32 +548,30 @@ bool AVFCameraViewfinderSettingsControl2::convertPixelFormatIfSupported(QVideoFr
     return found;
 }
 
-void AVFCameraViewfinderSettingsControl2::applySettings()
+bool AVFCameraViewfinderSettingsControl2::applySettings()
 {
-    if (m_settings.isNull())
-        return;
-
     if (m_service->session()->state() != QCamera::LoadedState &&
         m_service->session()->state() != QCamera::ActiveState) {
-        return;
+        return false;
     }
 
     AVCaptureDevice *captureDevice = m_service->session()->videoCaptureDevice();
     if (!captureDevice)
-        return;
+        return false;
+
+    bool activeFormatChanged = false;
 
-    NSMutableDictionary *videoSettings = [NSMutableDictionary dictionaryWithCapacity:1];
 #if QT_MAC_PLATFORM_SDK_EQUAL_OR_ABOVE(__MAC_10_7, __IPHONE_7_0)
     AVCaptureDeviceFormat *match = findBestFormatMatch(m_settings);
     if (match) {
         if (match != captureDevice.activeFormat) {
             const AVFConfigurationLock lock(captureDevice);
-            if (!lock) {
+            if (lock) {
+                captureDevice.activeFormat = match;
+                activeFormatChanged = true;
+            } else {
                 qDebugCamera() << Q_FUNC_INFO << "failed to lock for configuration";
-                return;
             }
-
-            captureDevice.activeFormat = match;
         }
     } else {
         qDebugCamera() << Q_FUNC_INFO << "matching device format not found";
@@ -617,6 +610,7 @@ void AVFCameraViewfinderSettingsControl2::applySettings()
         }
 
         if (avfPixelFormat != 0) {
+            NSMutableDictionary *videoSettings = [NSMutableDictionary dictionaryWithCapacity:1];
             [videoSettings setObject:[NSNumber numberWithUnsignedInt:avfPixelFormat]
                                       forKey:(id)kCVPixelBufferPixelFormatTypeKey];
 
@@ -625,6 +619,8 @@ void AVFCameraViewfinderSettingsControl2::applySettings()
     }
 
     qt_set_framerate_limits(captureDevice, videoConnection(), m_settings);
+
+    return activeFormatChanged;
 }
 
 QCameraViewfinderSettings AVFCameraViewfinderSettingsControl2::requestedSettings() const
diff --git a/src/plugins/avfoundation/camera/avfimageencodercontrol.h b/src/plugins/avfoundation/camera/avfimageencodercontrol.h
index c805b3e21..d3af77ffd 100644
--- a/src/plugins/avfoundation/camera/avfimageencodercontrol.h
+++ b/src/plugins/avfoundation/camera/avfimageencodercontrol.h
@@ -68,7 +68,7 @@ private:
     AVFCameraService *m_service;
     QImageEncoderSettings m_settings;
 
-    void applySettings();
+    bool applySettings();
     bool videoCaptureDeviceIsValid() const;
 };
 
diff --git a/src/plugins/avfoundation/camera/avfimageencodercontrol.mm b/src/plugins/avfoundation/camera/avfimageencodercontrol.mm
index a5e9c0e09..e2eb0bd01 100644
--- a/src/plugins/avfoundation/camera/avfimageencodercontrol.mm
+++ b/src/plugins/avfoundation/camera/avfimageencodercontrol.mm
@@ -168,40 +168,40 @@ QImageEncoderSettings AVFImageEncoderControl::imageSettings() const
 
 void AVFImageEncoderControl::setImageSettings(const QImageEncoderSettings &settings)
 {
-    if (m_settings == settings || settings.isNull())
+    if (m_settings == settings)
         return;
 
     m_settings = settings;
     applySettings();
 }
 
-void AVFImageEncoderControl::applySettings()
+bool AVFImageEncoderControl::applySettings()
 {
     if (!videoCaptureDeviceIsValid())
-        return;
+        return false;
 
     AVFCameraSession *session = m_service->session();
     if (!session || (session->state() != QCamera::ActiveState
         && session->state() != QCamera::LoadedState)) {
-        return;
+        return false;
     }
 
     if (!m_service->imageCaptureControl()
         || !m_service->imageCaptureControl()->stillImageOutput()) {
         qDebugCamera() << Q_FUNC_INFO << "no still image output";
-        return;
+        return false;
     }
 
     if (m_settings.codec().size()
         && m_settings.codec() != QLatin1String("jpeg")) {
         qDebugCamera() << Q_FUNC_INFO << "unsupported codec:" << m_settings.codec();
-        return;
+        return false;
     }
 
     QSize res(m_settings.resolution());
     if (res.isNull()) {
         qDebugCamera() << Q_FUNC_INFO << "invalid resolution:" << res;
-        return;
+        return false;
     }
 
     if (!res.isValid()) {
@@ -209,9 +209,11 @@ void AVFImageEncoderControl::applySettings()
         // Here we could choose the best format available, but
         // activeFormat is already equal to 'preset high' by default,
         // which is good enough, otherwise we can end in some format with low framerates.
-        return;
+        return false;
     }
 
+    bool activeFormatChanged = false;
+
 #if QT_MAC_PLATFORM_SDK_EQUAL_OR_ABOVE(__MAC_10_7, __IPHONE_7_0)
     if (QSysInfo::MacintoshVersion >= qt_OS_limit(QSysInfo::MV_10_7, QSysInfo::MV_IOS_7_0)) {
         AVCaptureDevice *captureDevice = m_service->session()->videoCaptureDevice();
@@ -220,16 +222,17 @@ void AVFImageEncoderControl::applySettings()
 
         if (!match) {
             qDebugCamera() << Q_FUNC_INFO << "unsupported resolution:" << res;
-            return;
+            return false;
         }
 
         if (match != captureDevice.activeFormat) {
             const AVFConfigurationLock lock(captureDevice);
             if (!lock) {
                 qDebugCamera() << Q_FUNC_INFO << "failed to lock for configuration";
-                return;
+                return false;
             }
             captureDevice.activeFormat = match;
+            activeFormatChanged = true;
         }
 
 #if defined(Q_OS_IOS) && QT_IOS_PLATFORM_SDK_EQUAL_OR_ABOVE(__IPHONE_8_0)
@@ -247,6 +250,8 @@ void AVFImageEncoderControl::applySettings()
 #endif
         // TODO: resolution without capture device format ...
     }
+
+    return activeFormatChanged;
 }
 
 bool AVFImageEncoderControl::videoCaptureDeviceIsValid() const
diff --git a/src/plugins/avfoundation/camera/avfmediarecordercontrol.mm b/src/plugins/avfoundation/camera/avfmediarecordercontrol.mm
index 115d70864..1b6e23ee5 100644
--- a/src/plugins/avfoundation/camera/avfmediarecordercontrol.mm
+++ b/src/plugins/avfoundation/camera/avfmediarecordercontrol.mm
@@ -37,6 +37,7 @@
 #include "avfcameraservice.h"
 #include "avfcameracontrol.h"
 #include "avfaudioinputselectorcontrol.h"
+#include "avfcamerautility.h"
 
 #include <QtCore/qurl.h>
 #include <QtCore/qfileinfo.h>
@@ -330,6 +331,9 @@ void AVFMediaRecorderControl::setupSessionForCapture()
             && m_cameraControl->captureMode().testFlag(QCamera::CaptureVideo)
             && m_session->state() != QCamera::UnloadedState) {
 
+        // Lock the video capture device to make sure the active format is not reset
+        const AVFConfigurationLock lock(m_session->videoCaptureDevice());
+
         // Add audio input
         // Allow recording even if something wrong happens with the audio input initialization
         AVCaptureDevice *audioDevice = m_audioInputControl->createCaptureDevice();
@@ -361,6 +365,9 @@ void AVFMediaRecorderControl::setupSessionForCapture()
                && (!m_cameraControl->captureMode().testFlag(QCamera::CaptureVideo)
                    || m_session->state() == QCamera::UnloadedState)) {
 
+        // Lock the video capture device to make sure the active format is not reset
+        const AVFConfigurationLock lock(m_session->videoCaptureDevice());
+
         [captureSession removeOutput:m_movieOutput];
 
         if (m_audioInput) {