Merge lp:~fboucault/camera-app/hook_up_hdr into lp:camera-app

[Florian Boucault]

Enabled HDR feature:
- linked it up to backend (qtubuntu-camera)
- fixed up overlay UI for icon less options

[Florian Boucault (fboucault)]

Mostly good to go apart from that we need to define a better enum value for HDR than QCameraExposure::ExposureModeVendor

review: Needs Fixing

[Florian Boucault (fboucault)]

This can be merged independently but the feature will not work until the following MR has landed: https://code.launchpad.net/~fboucault/qtubuntu-camera/hdr_scene_mode/+merge/225993

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:303
http://jenkins.qa.ubuntu.com/job/camera-app-ci/230/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-utopic-amd64-ci/46
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-utopic-armhf-ci/46
        deb: http://jenkins.qa.ubuntu.com/job/camera-app-utopic-armhf-ci/46/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-utopic-i386-ci/46
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-utopic-touch/166
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/1492
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-runner-mako/314
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-click-builder-utopic-armhf/239
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/9572
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/1248
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/1677
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/1677/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/camera-app-ci/230/rebuild

review: Approve (continuous-integration)

[lp:~fboucault/camera-app/hook_up_hdr]

304. By Florian Boucault on 2014-07-10

Merged from trunk.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:305
http://jenkins.qa.ubuntu.com/job/camera-app-ci/231/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-utopic-amd64-ci/47
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-utopic-armhf-ci/47
        deb: http://jenkins.qa.ubuntu.com/job/camera-app-utopic-armhf-ci/47/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-utopic-i386-ci/47
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-utopic-touch/169/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/1522
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-runner-mako/317/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-click-builder-utopic-armhf/243
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/9621
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/1275
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/1709
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/1709/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/camera-app-ci/231/rebuild

review: Needs Fixing (continuous-integration)

[lp:~fboucault/camera-app/hook_up_hdr]

305. By Florian Boucault on 2014-07-10

Define a custom ExposureMode for HDR.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:305
http://jenkins.qa.ubuntu.com/job/camera-app-ci/232/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-utopic-amd64-ci/48
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-utopic-armhf-ci/48
        deb: http://jenkins.qa.ubuntu.com/job/camera-app-utopic-armhf-ci/48/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/camera-app-utopic-i386-ci/48
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-utopic-touch/172
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/1544
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-runner-mako/320
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-click-builder-utopic-armhf/251
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/9672
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/1294
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/1734
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/1734/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/camera-app-ci/232/rebuild

review: Approve (continuous-integration)

[Olivier Tilloy (osomon)]

Code changes look good to me, I haven’t tested them functionally though, as packages for qtubuntu-camera failed to build in silo 12.

One minor question:

10 +// Definition of this enum value is duplicated in qtubuntu-camera
11 +static const QCameraExposure::ExposureMode ExposureHdr = static_cast<QCameraExposure::ExposureMode>(QCameraExposure::ExposureModeVendor + 1);

Couldn’t we patch QtMultimedia to add this value to the QCameraExposure::ExposureMode enum? Or, if this doesn’t make sense, doesn’t qtubuntu-camera expose public headers that we could refer to instead of duplicating the definition of this custom value (which eventually will bite us when they get out of sync)?

[Florian Boucault (fboucault)]

There are valid packages for qtubuntu-media in https://code.launchpad.net/~fboucault/qtubuntu-camera/hdr_scene_mode/+merge/225993
The build had to be restarted in silo 12 after the libhybris changes landed in the image but it has not. Packages in MR are identical though.

About the question, I am not confident this patch would be accepted upstream. Also we cannot make the app depend on qtubuntu-camera which will only ever exist where we use Android (phone/tablet) (and the app should work on the desktop).

[Olivier Tilloy (osomon)]

OK, that makes sense.

review: Approve (code)

=== modified file 'CameraApp/advancedcamerasettings.cpp'
--- CameraApp/advancedcamerasettings.cpp	2014-06-30 07:28:08 +0000
+++ CameraApp/advancedcamerasettings.cpp	2014-07-10 17:01:25 +0000
@@ -25,6 +25,10 @@
 #include <QtMultimedia/QMediaService>
 #include <QtMultimedia/QVideoDeviceSelectorControl>
 #include <QtMultimedia/QCameraFlashControl>
+#include <QtMultimedia/QCameraExposureControl>
+
+// Definition of this enum value is duplicated in qtubuntu-camera
+static const QCameraExposure::ExposureMode ExposureHdr = static_cast<QCameraExposure::ExposureMode>(QCameraExposure::ExposureModeVendor + 1);
 
 AdvancedCameraSettings::AdvancedCameraSettings(QObject *parent) :
     QObject(parent),
@@ -33,7 +37,8 @@
     m_camera(0),
     m_deviceSelector(0),
     m_viewFinderControl(0),
-    m_cameraFlashControl(0)
+    m_cameraFlashControl(0),
+    m_cameraExposureControl(0)
 {
 }
 
@@ -134,6 +139,19 @@
     return flashControl;
 }
 
+QCameraExposureControl* AdvancedCameraSettings::exposureControlFromCamera(QCamera *camera) const
+{
+    QMediaControl *control = mediaControlFromCamera(camera, QCameraExposureControl_iid);
+    QCameraExposureControl *exposureControl = qobject_cast<QCameraExposureControl*>(control);
+
+    if (exposureControl == 0) {
+        qWarning() << "No exposure control support";
+    }
+
+    return exposureControl;
+}
+
+
 QObject* AdvancedCameraSettings::camera() const
 {
     return m_cameraObject;
@@ -181,9 +199,18 @@
     }
 
     m_cameraFlashControl = flashControlFromCamera(m_camera);
+    m_cameraExposureControl = exposureControlFromCamera(m_camera);
+
+    if (m_cameraExposureControl) {
+        QObject::connect(m_cameraExposureControl,
+                         SIGNAL(actualValueChanged(int)),
+                         this, SLOT(onExposureValueChanged(int)));
+    }
 
     Q_EMIT resolutionChanged();
     Q_EMIT hasFlashChanged();
+    Q_EMIT hasHdrChanged();
+    Q_EMIT hdrEnabledChanged();
 }
 
 void AdvancedCameraSettings::onCameraStateChanged()
@@ -228,3 +255,42 @@
         return false;
     }
 }
+
+bool AdvancedCameraSettings::hasHdr() const
+{
+    if (m_cameraExposureControl) {
+        bool continuous;
+        if (m_cameraExposureControl->isParameterSupported(QCameraExposureControl::ExposureMode)) {
+            QVariantList range = m_cameraExposureControl->supportedParameterRange(QCameraExposureControl::ExposureMode, &continuous);
+            return range.contains(QVariant::fromValue(ExposureHdr));
+        }
+    } else {
+        return false;
+    }
+}
+
+bool AdvancedCameraSettings::hdrEnabled() const
+{
+    if (m_cameraExposureControl) {
+        QVariant exposureMode = m_cameraExposureControl->actualValue(QCameraExposureControl::ExposureMode);
+        return exposureMode.value<QCameraExposure::ExposureMode>() == ExposureHdr;
+    } else {
+        return false;
+    }
+}
+
+void AdvancedCameraSettings::setHdrEnabled(bool enabled)
+{
+    if (m_cameraExposureControl) {
+        QVariant exposureMode = enabled ? QVariant::fromValue(ExposureHdr)
+                                        : QVariant::fromValue(QCameraExposure::ExposureAuto);
+        m_cameraExposureControl->setValue(QCameraExposureControl::ExposureMode, exposureMode);
+    }
+}
+
+void AdvancedCameraSettings::onExposureValueChanged(int parameter)
+{
+    if (parameter == QCameraExposureControl::ExposureMode) {
+        Q_EMIT hdrEnabledChanged();
+    }
+}

=== modified file 'CameraApp/advancedcamerasettings.h'
--- CameraApp/advancedcamerasettings.h	2014-06-17 07:40:12 +0000
+++ CameraApp/advancedcamerasettings.h	2014-07-10 17:01:25 +0000
@@ -24,6 +24,7 @@
 #include <QtMultimedia/QCamera>
 #include <QtMultimedia/QVideoDeviceSelectorControl>
 #include <QtMultimedia/QCameraViewfinderSettingsControl>
+#include <QtMultimedia/QCameraExposureControl>
 #include <QtMultimedia/QMediaControl>
 
 class QCameraControl;
@@ -37,6 +38,8 @@
                 NOTIFY activeCameraIndexChanged)
     Q_PROPERTY (QSize resolution READ resolution NOTIFY resolutionChanged)
     Q_PROPERTY (bool hasFlash READ hasFlash NOTIFY hasFlashChanged)
+    Q_PROPERTY (bool hdrEnabled READ hdrEnabled WRITE setHdrEnabled NOTIFY hdrEnabledChanged)
+    Q_PROPERTY (bool hasHdr READ hasHdr NOTIFY hasHdrChanged)
 
 public:
     explicit AdvancedCameraSettings(QObject *parent = 0);
@@ -46,6 +49,9 @@
     void setActiveCameraIndex(int index);
     QSize resolution() const;
     bool hasFlash() const;
+    bool hasHdr() const;
+    bool hdrEnabled() const;
+    void setHdrEnabled(bool enabled);
     void readCapabilities();
 
 Q_SIGNALS:
@@ -53,15 +59,19 @@
     void activeCameraIndexChanged();
     void resolutionChanged();
     void hasFlashChanged();
+    void hasHdrChanged();
+    void hdrEnabledChanged();
 
 private Q_SLOTS:
     void onCameraStateChanged();
+    void onExposureValueChanged(int parameter);
 
 private:
     QVideoDeviceSelectorControl* selectorFromCamera(QCamera *camera) const;
     QCameraViewfinderSettingsControl* viewfinderFromCamera(QCamera *camera) const;
     QCameraControl *camcontrolFromCamera(QCamera *camera) const;
     QCameraFlashControl* flashControlFromCamera(QCamera* camera) const;
+    QCameraExposureControl* exposureControlFromCamera(QCamera *camera) const;
     QCamera* cameraFromCameraObject(QObject* cameraObject) const;
     QMediaControl* mediaControlFromCamera(QCamera *camera, const char* iid) const;
 
@@ -72,6 +82,7 @@
     QCameraViewfinderSettingsControl* m_viewFinderControl;
     QCameraControl* m_cameraControl;
     QCameraFlashControl* m_cameraFlashControl;
+    QCameraExposureControl* m_cameraExposureControl;
 
 };
 

=== modified file 'OptionValueButton.qml'
--- OptionValueButton.qml	2014-06-26 11:38:24 +0000
+++ OptionValueButton.qml	2014-07-10 17:01:25 +0000
@@ -40,6 +40,7 @@
         width: height
         color: "white"
         opacity: optionValueButton.selected ? 1.0 : 0.5
+        visible: name !== ""
     }
 
     Label {

=== modified file 'ViewFinderOverlay.qml'
--- ViewFinderOverlay.qml	2014-07-02 15:43:31 +0000
+++ ViewFinderOverlay.qml	2014-07-10 17:01:25 +0000
@@ -56,6 +56,12 @@
         when: camera.captureMode == Camera.CaptureVideo
     }
 
+    Binding {
+        target: camera.advanced
+        property: "hdrEnabled"
+        value: settings.hdrEnabled
+    }
+
     Connections {
         target: camera.imageCapture
         onReadyChanged: {
@@ -155,11 +161,11 @@
                 id: hdrOptionsModel
 
                 property string settingsProperty: "hdrEnabled"
-                property string icon: "import-image"
+                property string icon: ""
                 property string label: "HDR"
                 property bool isToggle: true
                 property int selectedIndex: bottomEdge.indexForValue(hdrOptionsModel, settings.hdrEnabled)
-                property bool available: false
+                property bool available: camera.advanced.hasHdr
                 property bool visible: true
 
                 ListElement {
@@ -225,7 +231,7 @@
 
                 Repeater {
                     model: bottomEdge.options
-                    delegate: Icon {
+                    delegate: Item {
                         anchors {
                             top: parent.top
                             topMargin: units.gu(0.5)
@@ -233,10 +239,26 @@
                             bottomMargin: units.gu(0.5)
                         }
                         width: units.gu(2)
-                        color: "white"
+                        visible: modelData.available && modelData.visible ? (modelData.isToggle ? modelData.get(model.selectedIndex).value : true) : false
                         opacity: 0.5
-                        name: modelData.isToggle ? modelData.icon : modelData.get(model.selectedIndex).icon
-                        visible: modelData.available && modelData.visible ? (modelData.isToggle ? modelData.get(model.selectedIndex).value : true) : false
+
+                        Icon {
+                            id: indicatorIcon
+                            anchors.fill: parent
+                            color: "white"
+                            name: modelData.isToggle ? modelData.icon : modelData.get(model.selectedIndex).icon
+                            visible: name !== ""
+                        }
+
+                        Label {
+                            id: indicatorLabel
+                            anchors.fill: parent
+                            fontSize: "xx-small"
+                            color: "white"
+                            text: modelData.label
+                            verticalAlignment: Text.AlignVCenter
+                            visible: indicatorIcon.name === ""
+                        }
                     }
                 }
             }