Merge lp:~fboucault/camera-app/hook_up_hdr into lp:camera-app
- hook_up_hdr
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Florian Boucault |
Approved revision: | 305 |
Merged at revision: | 307 |
Proposed branch: | lp:~fboucault/camera-app/hook_up_hdr |
Merge into: | lp:camera-app |
Diff against target: |
247 lines (+107/-7) 4 files modified
CameraApp/advancedcamerasettings.cpp (+67/-1) CameraApp/advancedcamerasettings.h (+11/-0) OptionValueButton.qml (+1/-0) ViewFinderOverlay.qml (+28/-6) |
To merge this branch: | bzr merge lp:~fboucault/camera-app/hook_up_hdr |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Olivier Tilloy | code | Approve | |
PS Jenkins bot | continuous-integration | Approve | |
Florian Boucault (community) | Needs Fixing | ||
Review via email:
|
Commit message
Enabled HDR feature:
- linked it up to backend (qtubuntu-camera)
- fixed up overlay UI for icon less options
Description of the change
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Florian Boucault (fboucault) wrote : | # |
This can be merged independently but the feature will not work until the following MR has landed: https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:303
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
- 304. By Florian Boucault
-
Merged from trunk.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:305
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
- 305. By Florian Boucault
-
Define a custom ExposureMode for HDR.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:305
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
Click here to trigger a rebuild:
http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Olivier Tilloy (osomon) wrote : | # |
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
Couldn’t we patch QtMultimedia to add this value to the QCameraExposure
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Florian Boucault (fboucault) wrote : | # |
There are valid packages for qtubuntu-media in https:/
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).
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Olivier Tilloy (osomon) wrote : | # |
OK, that makes sense.
Preview Diff
1 | === modified file 'CameraApp/advancedcamerasettings.cpp' | |||
2 | --- CameraApp/advancedcamerasettings.cpp 2014-06-30 07:28:08 +0000 | |||
3 | +++ CameraApp/advancedcamerasettings.cpp 2014-07-10 17:01:25 +0000 | |||
4 | @@ -25,6 +25,10 @@ | |||
5 | 25 | #include <QtMultimedia/QMediaService> | 25 | #include <QtMultimedia/QMediaService> |
6 | 26 | #include <QtMultimedia/QVideoDeviceSelectorControl> | 26 | #include <QtMultimedia/QVideoDeviceSelectorControl> |
7 | 27 | #include <QtMultimedia/QCameraFlashControl> | 27 | #include <QtMultimedia/QCameraFlashControl> |
8 | 28 | #include <QtMultimedia/QCameraExposureControl> | ||
9 | 29 | |||
10 | 30 | // Definition of this enum value is duplicated in qtubuntu-camera | ||
11 | 31 | static const QCameraExposure::ExposureMode ExposureHdr = static_cast<QCameraExposure::ExposureMode>(QCameraExposure::ExposureModeVendor + 1); | ||
12 | 28 | 32 | ||
13 | 29 | AdvancedCameraSettings::AdvancedCameraSettings(QObject *parent) : | 33 | AdvancedCameraSettings::AdvancedCameraSettings(QObject *parent) : |
14 | 30 | QObject(parent), | 34 | QObject(parent), |
15 | @@ -33,7 +37,8 @@ | |||
16 | 33 | m_camera(0), | 37 | m_camera(0), |
17 | 34 | m_deviceSelector(0), | 38 | m_deviceSelector(0), |
18 | 35 | m_viewFinderControl(0), | 39 | m_viewFinderControl(0), |
20 | 36 | m_cameraFlashControl(0) | 40 | m_cameraFlashControl(0), |
21 | 41 | m_cameraExposureControl(0) | ||
22 | 37 | { | 42 | { |
23 | 38 | } | 43 | } |
24 | 39 | 44 | ||
25 | @@ -134,6 +139,19 @@ | |||
26 | 134 | return flashControl; | 139 | return flashControl; |
27 | 135 | } | 140 | } |
28 | 136 | 141 | ||
29 | 142 | QCameraExposureControl* AdvancedCameraSettings::exposureControlFromCamera(QCamera *camera) const | ||
30 | 143 | { | ||
31 | 144 | QMediaControl *control = mediaControlFromCamera(camera, QCameraExposureControl_iid); | ||
32 | 145 | QCameraExposureControl *exposureControl = qobject_cast<QCameraExposureControl*>(control); | ||
33 | 146 | |||
34 | 147 | if (exposureControl == 0) { | ||
35 | 148 | qWarning() << "No exposure control support"; | ||
36 | 149 | } | ||
37 | 150 | |||
38 | 151 | return exposureControl; | ||
39 | 152 | } | ||
40 | 153 | |||
41 | 154 | |||
42 | 137 | QObject* AdvancedCameraSettings::camera() const | 155 | QObject* AdvancedCameraSettings::camera() const |
43 | 138 | { | 156 | { |
44 | 139 | return m_cameraObject; | 157 | return m_cameraObject; |
45 | @@ -181,9 +199,18 @@ | |||
46 | 181 | } | 199 | } |
47 | 182 | 200 | ||
48 | 183 | m_cameraFlashControl = flashControlFromCamera(m_camera); | 201 | m_cameraFlashControl = flashControlFromCamera(m_camera); |
49 | 202 | m_cameraExposureControl = exposureControlFromCamera(m_camera); | ||
50 | 203 | |||
51 | 204 | if (m_cameraExposureControl) { | ||
52 | 205 | QObject::connect(m_cameraExposureControl, | ||
53 | 206 | SIGNAL(actualValueChanged(int)), | ||
54 | 207 | this, SLOT(onExposureValueChanged(int))); | ||
55 | 208 | } | ||
56 | 184 | 209 | ||
57 | 185 | Q_EMIT resolutionChanged(); | 210 | Q_EMIT resolutionChanged(); |
58 | 186 | Q_EMIT hasFlashChanged(); | 211 | Q_EMIT hasFlashChanged(); |
59 | 212 | Q_EMIT hasHdrChanged(); | ||
60 | 213 | Q_EMIT hdrEnabledChanged(); | ||
61 | 187 | } | 214 | } |
62 | 188 | 215 | ||
63 | 189 | void AdvancedCameraSettings::onCameraStateChanged() | 216 | void AdvancedCameraSettings::onCameraStateChanged() |
64 | @@ -228,3 +255,42 @@ | |||
65 | 228 | return false; | 255 | return false; |
66 | 229 | } | 256 | } |
67 | 230 | } | 257 | } |
68 | 258 | |||
69 | 259 | bool AdvancedCameraSettings::hasHdr() const | ||
70 | 260 | { | ||
71 | 261 | if (m_cameraExposureControl) { | ||
72 | 262 | bool continuous; | ||
73 | 263 | if (m_cameraExposureControl->isParameterSupported(QCameraExposureControl::ExposureMode)) { | ||
74 | 264 | QVariantList range = m_cameraExposureControl->supportedParameterRange(QCameraExposureControl::ExposureMode, &continuous); | ||
75 | 265 | return range.contains(QVariant::fromValue(ExposureHdr)); | ||
76 | 266 | } | ||
77 | 267 | } else { | ||
78 | 268 | return false; | ||
79 | 269 | } | ||
80 | 270 | } | ||
81 | 271 | |||
82 | 272 | bool AdvancedCameraSettings::hdrEnabled() const | ||
83 | 273 | { | ||
84 | 274 | if (m_cameraExposureControl) { | ||
85 | 275 | QVariant exposureMode = m_cameraExposureControl->actualValue(QCameraExposureControl::ExposureMode); | ||
86 | 276 | return exposureMode.value<QCameraExposure::ExposureMode>() == ExposureHdr; | ||
87 | 277 | } else { | ||
88 | 278 | return false; | ||
89 | 279 | } | ||
90 | 280 | } | ||
91 | 281 | |||
92 | 282 | void AdvancedCameraSettings::setHdrEnabled(bool enabled) | ||
93 | 283 | { | ||
94 | 284 | if (m_cameraExposureControl) { | ||
95 | 285 | QVariant exposureMode = enabled ? QVariant::fromValue(ExposureHdr) | ||
96 | 286 | : QVariant::fromValue(QCameraExposure::ExposureAuto); | ||
97 | 287 | m_cameraExposureControl->setValue(QCameraExposureControl::ExposureMode, exposureMode); | ||
98 | 288 | } | ||
99 | 289 | } | ||
100 | 290 | |||
101 | 291 | void AdvancedCameraSettings::onExposureValueChanged(int parameter) | ||
102 | 292 | { | ||
103 | 293 | if (parameter == QCameraExposureControl::ExposureMode) { | ||
104 | 294 | Q_EMIT hdrEnabledChanged(); | ||
105 | 295 | } | ||
106 | 296 | } | ||
107 | 231 | 297 | ||
108 | === modified file 'CameraApp/advancedcamerasettings.h' | |||
109 | --- CameraApp/advancedcamerasettings.h 2014-06-17 07:40:12 +0000 | |||
110 | +++ CameraApp/advancedcamerasettings.h 2014-07-10 17:01:25 +0000 | |||
111 | @@ -24,6 +24,7 @@ | |||
112 | 24 | #include <QtMultimedia/QCamera> | 24 | #include <QtMultimedia/QCamera> |
113 | 25 | #include <QtMultimedia/QVideoDeviceSelectorControl> | 25 | #include <QtMultimedia/QVideoDeviceSelectorControl> |
114 | 26 | #include <QtMultimedia/QCameraViewfinderSettingsControl> | 26 | #include <QtMultimedia/QCameraViewfinderSettingsControl> |
115 | 27 | #include <QtMultimedia/QCameraExposureControl> | ||
116 | 27 | #include <QtMultimedia/QMediaControl> | 28 | #include <QtMultimedia/QMediaControl> |
117 | 28 | 29 | ||
118 | 29 | class QCameraControl; | 30 | class QCameraControl; |
119 | @@ -37,6 +38,8 @@ | |||
120 | 37 | NOTIFY activeCameraIndexChanged) | 38 | NOTIFY activeCameraIndexChanged) |
121 | 38 | Q_PROPERTY (QSize resolution READ resolution NOTIFY resolutionChanged) | 39 | Q_PROPERTY (QSize resolution READ resolution NOTIFY resolutionChanged) |
122 | 39 | Q_PROPERTY (bool hasFlash READ hasFlash NOTIFY hasFlashChanged) | 40 | Q_PROPERTY (bool hasFlash READ hasFlash NOTIFY hasFlashChanged) |
123 | 41 | Q_PROPERTY (bool hdrEnabled READ hdrEnabled WRITE setHdrEnabled NOTIFY hdrEnabledChanged) | ||
124 | 42 | Q_PROPERTY (bool hasHdr READ hasHdr NOTIFY hasHdrChanged) | ||
125 | 40 | 43 | ||
126 | 41 | public: | 44 | public: |
127 | 42 | explicit AdvancedCameraSettings(QObject *parent = 0); | 45 | explicit AdvancedCameraSettings(QObject *parent = 0); |
128 | @@ -46,6 +49,9 @@ | |||
129 | 46 | void setActiveCameraIndex(int index); | 49 | void setActiveCameraIndex(int index); |
130 | 47 | QSize resolution() const; | 50 | QSize resolution() const; |
131 | 48 | bool hasFlash() const; | 51 | bool hasFlash() const; |
132 | 52 | bool hasHdr() const; | ||
133 | 53 | bool hdrEnabled() const; | ||
134 | 54 | void setHdrEnabled(bool enabled); | ||
135 | 49 | void readCapabilities(); | 55 | void readCapabilities(); |
136 | 50 | 56 | ||
137 | 51 | Q_SIGNALS: | 57 | Q_SIGNALS: |
138 | @@ -53,15 +59,19 @@ | |||
139 | 53 | void activeCameraIndexChanged(); | 59 | void activeCameraIndexChanged(); |
140 | 54 | void resolutionChanged(); | 60 | void resolutionChanged(); |
141 | 55 | void hasFlashChanged(); | 61 | void hasFlashChanged(); |
142 | 62 | void hasHdrChanged(); | ||
143 | 63 | void hdrEnabledChanged(); | ||
144 | 56 | 64 | ||
145 | 57 | private Q_SLOTS: | 65 | private Q_SLOTS: |
146 | 58 | void onCameraStateChanged(); | 66 | void onCameraStateChanged(); |
147 | 67 | void onExposureValueChanged(int parameter); | ||
148 | 59 | 68 | ||
149 | 60 | private: | 69 | private: |
150 | 61 | QVideoDeviceSelectorControl* selectorFromCamera(QCamera *camera) const; | 70 | QVideoDeviceSelectorControl* selectorFromCamera(QCamera *camera) const; |
151 | 62 | QCameraViewfinderSettingsControl* viewfinderFromCamera(QCamera *camera) const; | 71 | QCameraViewfinderSettingsControl* viewfinderFromCamera(QCamera *camera) const; |
152 | 63 | QCameraControl *camcontrolFromCamera(QCamera *camera) const; | 72 | QCameraControl *camcontrolFromCamera(QCamera *camera) const; |
153 | 64 | QCameraFlashControl* flashControlFromCamera(QCamera* camera) const; | 73 | QCameraFlashControl* flashControlFromCamera(QCamera* camera) const; |
154 | 74 | QCameraExposureControl* exposureControlFromCamera(QCamera *camera) const; | ||
155 | 65 | QCamera* cameraFromCameraObject(QObject* cameraObject) const; | 75 | QCamera* cameraFromCameraObject(QObject* cameraObject) const; |
156 | 66 | QMediaControl* mediaControlFromCamera(QCamera *camera, const char* iid) const; | 76 | QMediaControl* mediaControlFromCamera(QCamera *camera, const char* iid) const; |
157 | 67 | 77 | ||
158 | @@ -72,6 +82,7 @@ | |||
159 | 72 | QCameraViewfinderSettingsControl* m_viewFinderControl; | 82 | QCameraViewfinderSettingsControl* m_viewFinderControl; |
160 | 73 | QCameraControl* m_cameraControl; | 83 | QCameraControl* m_cameraControl; |
161 | 74 | QCameraFlashControl* m_cameraFlashControl; | 84 | QCameraFlashControl* m_cameraFlashControl; |
162 | 85 | QCameraExposureControl* m_cameraExposureControl; | ||
163 | 75 | 86 | ||
164 | 76 | }; | 87 | }; |
165 | 77 | 88 | ||
166 | 78 | 89 | ||
167 | === modified file 'OptionValueButton.qml' | |||
168 | --- OptionValueButton.qml 2014-06-26 11:38:24 +0000 | |||
169 | +++ OptionValueButton.qml 2014-07-10 17:01:25 +0000 | |||
170 | @@ -40,6 +40,7 @@ | |||
171 | 40 | width: height | 40 | width: height |
172 | 41 | color: "white" | 41 | color: "white" |
173 | 42 | opacity: optionValueButton.selected ? 1.0 : 0.5 | 42 | opacity: optionValueButton.selected ? 1.0 : 0.5 |
174 | 43 | visible: name !== "" | ||
175 | 43 | } | 44 | } |
176 | 44 | 45 | ||
177 | 45 | Label { | 46 | Label { |
178 | 46 | 47 | ||
179 | === modified file 'ViewFinderOverlay.qml' | |||
180 | --- ViewFinderOverlay.qml 2014-07-02 15:43:31 +0000 | |||
181 | +++ ViewFinderOverlay.qml 2014-07-10 17:01:25 +0000 | |||
182 | @@ -56,6 +56,12 @@ | |||
183 | 56 | when: camera.captureMode == Camera.CaptureVideo | 56 | when: camera.captureMode == Camera.CaptureVideo |
184 | 57 | } | 57 | } |
185 | 58 | 58 | ||
186 | 59 | Binding { | ||
187 | 60 | target: camera.advanced | ||
188 | 61 | property: "hdrEnabled" | ||
189 | 62 | value: settings.hdrEnabled | ||
190 | 63 | } | ||
191 | 64 | |||
192 | 59 | Connections { | 65 | Connections { |
193 | 60 | target: camera.imageCapture | 66 | target: camera.imageCapture |
194 | 61 | onReadyChanged: { | 67 | onReadyChanged: { |
195 | @@ -155,11 +161,11 @@ | |||
196 | 155 | id: hdrOptionsModel | 161 | id: hdrOptionsModel |
197 | 156 | 162 | ||
198 | 157 | property string settingsProperty: "hdrEnabled" | 163 | property string settingsProperty: "hdrEnabled" |
200 | 158 | property string icon: "import-image" | 164 | property string icon: "" |
201 | 159 | property string label: "HDR" | 165 | property string label: "HDR" |
202 | 160 | property bool isToggle: true | 166 | property bool isToggle: true |
203 | 161 | property int selectedIndex: bottomEdge.indexForValue(hdrOptionsModel, settings.hdrEnabled) | 167 | property int selectedIndex: bottomEdge.indexForValue(hdrOptionsModel, settings.hdrEnabled) |
205 | 162 | property bool available: false | 168 | property bool available: camera.advanced.hasHdr |
206 | 163 | property bool visible: true | 169 | property bool visible: true |
207 | 164 | 170 | ||
208 | 165 | ListElement { | 171 | ListElement { |
209 | @@ -225,7 +231,7 @@ | |||
210 | 225 | 231 | ||
211 | 226 | Repeater { | 232 | Repeater { |
212 | 227 | model: bottomEdge.options | 233 | model: bottomEdge.options |
214 | 228 | delegate: Icon { | 234 | delegate: Item { |
215 | 229 | anchors { | 235 | anchors { |
216 | 230 | top: parent.top | 236 | top: parent.top |
217 | 231 | topMargin: units.gu(0.5) | 237 | topMargin: units.gu(0.5) |
218 | @@ -233,10 +239,26 @@ | |||
219 | 233 | bottomMargin: units.gu(0.5) | 239 | bottomMargin: units.gu(0.5) |
220 | 234 | } | 240 | } |
221 | 235 | width: units.gu(2) | 241 | width: units.gu(2) |
223 | 236 | color: "white" | 242 | visible: modelData.available && modelData.visible ? (modelData.isToggle ? modelData.get(model.selectedIndex).value : true) : false |
224 | 237 | opacity: 0.5 | 243 | opacity: 0.5 |
227 | 238 | name: modelData.isToggle ? modelData.icon : modelData.get(model.selectedIndex).icon | 244 | |
228 | 239 | visible: modelData.available && modelData.visible ? (modelData.isToggle ? modelData.get(model.selectedIndex).value : true) : false | 245 | Icon { |
229 | 246 | id: indicatorIcon | ||
230 | 247 | anchors.fill: parent | ||
231 | 248 | color: "white" | ||
232 | 249 | name: modelData.isToggle ? modelData.icon : modelData.get(model.selectedIndex).icon | ||
233 | 250 | visible: name !== "" | ||
234 | 251 | } | ||
235 | 252 | |||
236 | 253 | Label { | ||
237 | 254 | id: indicatorLabel | ||
238 | 255 | anchors.fill: parent | ||
239 | 256 | fontSize: "xx-small" | ||
240 | 257 | color: "white" | ||
241 | 258 | text: modelData.label | ||
242 | 259 | verticalAlignment: Text.AlignVCenter | ||
243 | 260 | visible: indicatorIcon.name === "" | ||
244 | 261 | } | ||
245 | 240 | } | 262 | } |
246 | 241 | } | 263 | } |
247 | 242 | } | 264 | } |
Mostly good to go apart from that we need to define a better enum value for HDR than QCameraExposure ::ExposureModeV endor