Merge lp:~phablet-team/unity8/sms-notif-lp1544477 into lp:unity8

Proposed by Alfonso Sanchez-Beato on 2016-08-24
Status: Merged
Approved by: Albert Astals Cid on 2016-08-26
Approved revision: 2604
Merged at revision: 2601
Proposed branch: lp:~phablet-team/unity8/sms-notif-lp1544477
Merge into: lp:unity8
Diff against target: 205 lines (+73/-34)
3 files modified
plugins/Dash/DashAudioPlayer.qml (+45/-21)
qml/Components/NotificationAudio.qml (+20/-3)
tests/qmltests/Dash/Previews/tst_PreviewAudioPlayback.qml (+8/-10)
To merge this branch: bzr merge lp:~phablet-team/unity8/sms-notif-lp1544477
Reviewer Review Type Date Requested Status
Albert Astals Cid (community) 2016-08-24 Approve on 2016-08-26
Unity8 CI Bot continuous-integration Needs Fixing on 2016-08-26
Review via email: mp+303791@code.launchpad.net

Commit message

Make sure we emit sounds when taking a screenshot even after media-hub
has restarted (LP: #1544477)

Description of the change

Make sure we emit sounds when taking a screenshot even after media-hub
has restarted (LP: #1544477)

To post a comment you must log in.
2600. By Alfonso Sanchez-Beato on 2016-08-24

Add reference to QtMultimedia bug

Albert Astals Cid (aacid) wrote :

Isn't this something we can fix in qtmultimedia? Or is it just easier to wokraround here?

If so I guess we need a similar fix in ./plugins/Dash/DashAudioPlayer.qml ?

review: Needs Information

It is quite difficult to solve in qtmultimedia in a general way. It is also unclear whether upstream would take those patches or not. Anyway, I have opened bug #1616425 to track this as we might try that in the future.

Yes, probably plugins/Dash/DashAudioPlayer.qml needs the fix too.

2601. By Alfonso Sanchez-Beato on 2016-08-25

Make sure we can play music from the scopes after media-hub restarts.

Branch refreshed after fixing DashAudioPlayer.qml

Unity8 CI Bot (unity8-ci-bot) wrote :

FAILED: Continuous integration, rev:2601
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2032/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/2666
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/1463
    FAILURE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/1463/console
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=yakkety,testname=qmluitests.sh/1463
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/2694
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2567
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2567
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2567
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2561
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2561/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2561
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2561/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2561
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2561/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2561
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2561/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2561
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2561/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2561
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2561/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2561
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2561/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2561
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2561/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2561
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2561/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2032/rebuild

review: Needs Fixing (continuous-integration)
2602. By Alfonso Sanchez-Beato on 2016-08-26

Adapt unit test to DashAudioPlayer changes

2603. By Alfonso Sanchez-Beato on 2016-08-26

One more fix for PreviewAudioPlayback test

2604. By Alfonso Sanchez-Beato on 2016-08-26

Change verify to compare in test

Albert Astals Cid (aacid) wrote :

 * Did you perform an exploratory manual test run of the code change and any related functionality?
Yes

 * Did CI run pass? If not, please explain why.
Waiting for it before top-approving, but passes fine locally

review: Approve
Unity8 CI Bot (unity8-ci-bot) wrote :

FAILED: Continuous integration, rev:2603
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2041/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/2681
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/1473
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/1473
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=yakkety,testname=qmluitests.sh/1473
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/2709
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2582
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2582
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2582
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2575
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2575/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2575
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2575/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2575
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2575/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2575
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2575/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2575
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2575/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2575
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2575/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2575
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2575/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2575
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2575/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2575
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2575/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2041/rebuild

review: Needs Fixing (continuous-integration)
Albert Astals Cid (aacid) wrote :

 * Did you perform an exploratory manual test run of the code change and any related functionality?
Yes

 * Did CI run pass? If not, please explain why.
Almost, known unstable unrelated tests failed

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/Dash/DashAudioPlayer.qml'
2--- plugins/Dash/DashAudioPlayer.qml 2015-12-22 09:12:32 +0000
3+++ plugins/Dash/DashAudioPlayer.qml 2016-08-26 08:08:57 +0000
4@@ -20,16 +20,22 @@
5 import Dash 0.1
6
7 QtObject {
8- readonly property real progress: audio.position / audio.duration
9- readonly property bool playing: audio.playbackState === Audio.PlayingState
10- readonly property bool paused: audio.playbackState === Audio.PausedState
11- readonly property bool stopped: audio.playbackState === Audio.StoppedState
12- readonly property alias position: audio.position
13- readonly property url currentSource: audio.playlist.currentItemSource
14+ id: root
15+ readonly property real progress: priv.audio ? priv.audio.position / priv.audio.duration : 0.0
16+ readonly property bool playing: priv.audio ? priv.audio.playbackState === Audio.PlayingState : false
17+ readonly property bool paused: priv.audio ? priv.audio.playbackState === Audio.PausedState : false
18+ readonly property bool stopped: priv.audio ? priv.audio.playbackState === Audio.StoppedState : true
19+ readonly property int position: priv.audio ? priv.audio.position : 0
20+ readonly property url currentSource: priv.audio ? priv.audio.playlist.currentItemSource : ""
21+ readonly property Playlist playlist: priv.audio ? priv.audio.playlist : null
22
23 function playSource(newSource, newPlaylist) {
24+ if (!priv.audio) {
25+ console.info("DashAudioPlayer: creating player");
26+ priv.audio = priv.audioComponent.createObject(root);
27+ }
28 stop();
29- audio.playlist.clear();
30+ priv.audio.playlist.clear();
31 if (newPlaylist) {
32 // Look for newSource in newPlaylist
33 var sourceIndex = -1;
34@@ -48,35 +54,53 @@
35 for (var i in newPlaylist) {
36 urls.push(newPlaylist[i]);
37 }
38- audio.playlist.addItems(urls);
39- audio.playlist.currentIndex = sourceIndex;
40+ priv.audio.playlist.addItems(urls);
41+ priv.audio.playlist.currentIndex = sourceIndex;
42 } else {
43- audio.playlist.addItem(newSource);
44- audio.playlist.currentIndex = 0;
45+ priv.audio.playlist.addItem(newSource);
46+ priv.audio.playlist.currentIndex = 0;
47 }
48 play();
49 }
50
51 function stop() {
52- audio.stop();
53+ if (priv.audio) {
54+ priv.audio.stop();
55+ }
56 }
57
58 function play() {
59- audio.play();
60+ if (priv.audio) {
61+ priv.audio.play();
62+ }
63 }
64
65 function pause() {
66- audio.pause();
67+ if (priv.audio) {
68+ priv.audio.pause();
69+ }
70 }
71
72- property QtObject audio: Audio {
73- id: audio
74- objectName: "audio"
75- playlist: Playlist {
76- objectName: "playlist"
77+ property QtObject priv: QtObject {
78+ id: priv
79+ property Audio audio: null
80+ property Component audioComponent: Component {
81+ Audio {
82+ playlist: Playlist {
83+ objectName: "playlist"
84+ }
85+ /* Remove player in case of error so it gets recreated next time
86+ * we need it. Happens if backend media player restarted, for
87+ * instance. qtmultimedia should probably handle this
88+ * transparently (LP: #1616425).
89+ */
90+ onError: {
91+ console.warn("DashAudioPlayer: error event (" +
92+ priv.audio.errorString + "), destroying");
93+ priv.audio.destroy();
94+ }
95+ }
96 }
97-
98- onErrorStringChanged: console.warn("Dash Audio player error:", errorString)
99 }
100
101 function lengthToString(s) {
102
103=== modified file 'qml/Components/NotificationAudio.qml'
104--- qml/Components/NotificationAudio.qml 2016-04-29 20:06:54 +0000
105+++ qml/Components/NotificationAudio.qml 2016-08-26 08:08:57 +0000
106@@ -23,6 +23,10 @@
107 readonly property var playbackState: priv.audio ? priv.audio.playbackState : 0
108
109 function play() {
110+ if (!priv.audio) {
111+ console.info("NotificationAudio: creating player");
112+ priv.audio = priv.audioComponent.createObject(root);
113+ }
114 if (priv.audio) {
115 priv.audio.play();
116 }
117@@ -35,9 +39,22 @@
118
119 QtObject {
120 id: priv
121- property var audio: Audio {
122- source: root.source
123- audioRole: MediaPlayer.NotificationRole
124+ property Audio audio: null
125+ property Component audioComponent: Component {
126+ Audio {
127+ source: root.source
128+ audioRole: MediaPlayer.NotificationRole
129+ /* Remove player in case of error so it gets recreated next time
130+ * we need it. Happens if backend media player restarted, for
131+ * instance. qtmultimedia should probably handle this
132+ * transparently (LP: #1616425).
133+ */
134+ onError: {
135+ console.warn("NotificationAudio: error event (" +
136+ priv.audio.errorString + "), destroying");
137+ priv.audio.destroy();
138+ }
139+ }
140 }
141 }
142 }
143
144=== modified file 'tests/qmltests/Dash/Previews/tst_PreviewAudioPlayback.qml'
145--- tests/qmltests/Dash/Previews/tst_PreviewAudioPlayback.qml 2016-02-15 15:27:09 +0000
146+++ tests/qmltests/Dash/Previews/tst_PreviewAudioPlayback.qml 2016-08-26 08:08:57 +0000
147@@ -127,8 +127,6 @@
148 var track1PlayButton = findChild(track1Item, "playButton");
149 var track2PlayButton = findChild(track2Item, "playButton");
150
151- var audio = DashAudioPlayer.audio;
152-
153 // All progress bars must be hidden in the beginning
154 compare(track0ProgressBar.visible, false);
155 compare(track1ProgressBar.visible, false);
156@@ -137,7 +135,7 @@
157 // Playing track 0 should make progress bar 0 visible
158 mouseClick(track0PlayButton);
159
160- tryCompare(audio, "playbackState", Audio.PlayingState);
161+ compare(DashAudioPlayer.playing, true);
162 checkPlayerSource(0);
163
164 tryCompare(track0ProgressBar, "visible", true);
165@@ -146,25 +144,25 @@
166
167 // Clicking the button again should pause it. The progress bar should stay visible
168 mouseClick(track0PlayButton);
169- tryCompare(audio, "playbackState", Audio.PausedState);
170+ compare(DashAudioPlayer.paused, true);
171 checkPlayerSource(0);
172 tryCompare(track0ProgressBar, "visible", true);
173
174 // Continue playback
175 mouseClick(track0PlayButton);
176- tryCompare(audio, "playbackState", Audio.PlayingState);
177+ compare(DashAudioPlayer.playing, true);
178 checkPlayerSource(0);
179
180 // Playing track 1 should make progress bar 1 visible and hide progress bar 0 again
181 mouseClick(track1PlayButton);
182
183- tryCompare(audio, "playbackState", Audio.PlayingState);
184+ compare(DashAudioPlayer.playing, true);
185 checkPlayerSource(1);
186
187 // Check the playlist is song 0, 1, 2
188- checkPlayerUrls(tracksModel2["tracks"][0].source, audio.playlist.itemSource(0));
189- checkPlayerUrls(tracksModel2["tracks"][1].source, audio.playlist.itemSource(1));
190- checkPlayerUrls(tracksModel2["tracks"][2].source, audio.playlist.itemSource(2));
191+ checkPlayerUrls(tracksModel2["tracks"][0].source, DashAudioPlayer.playlist.itemSource(0));
192+ checkPlayerUrls(tracksModel2["tracks"][1].source, DashAudioPlayer.playlist.itemSource(1));
193+ checkPlayerUrls(tracksModel2["tracks"][2].source, DashAudioPlayer.playlist.itemSource(2));
194
195 tryCompare(track0ProgressBar, "visible", false);
196 tryCompare(track1ProgressBar, "visible", true);
197@@ -173,7 +171,7 @@
198 // Playing track 2 should make progress bar 1 visible and hide progress bar 0 again
199 mouseClick(track2PlayButton);
200
201- tryCompare(audio, "playbackState", Audio.PlayingState);
202+ compare(DashAudioPlayer.playing, true);
203 checkPlayerSource(2);
204
205 tryCompare(track0ProgressBar, "visible", false);

Subscribers

People subscribed via source and target branches