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

[Alfonso Sanchez-Beato]

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

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

[lp:~phablet-team/unity8/sms-notif-lp1544477]

2600. By Alfonso Sanchez-Beato on 2016-08-24

Add reference to QtMultimedia bug

[Albert Astals Cid (aacid)]

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

[Alfonso Sanchez-Beato (alfonsosanchezbeato)]

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.

[lp:~phablet-team/unity8/sms-notif-lp1544477]

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

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

[Alfonso Sanchez-Beato (alfonsosanchezbeato)]

Branch refreshed after fixing DashAudioPlayer.qml

[Unity8 CI Bot (unity8-ci-bot)]

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)

[lp:~phablet-team/unity8/sms-notif-lp1544477]

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)]

 * 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)]

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)]

 * 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

=== modified file 'plugins/Dash/DashAudioPlayer.qml'
--- plugins/Dash/DashAudioPlayer.qml	2015-12-22 09:12:32 +0000
+++ plugins/Dash/DashAudioPlayer.qml	2016-08-26 08:08:57 +0000
@@ -20,16 +20,22 @@
 import Dash 0.1
 
 QtObject {
-    readonly property real progress: audio.position / audio.duration
-    readonly property bool playing: audio.playbackState === Audio.PlayingState
-    readonly property bool paused: audio.playbackState === Audio.PausedState
-    readonly property bool stopped: audio.playbackState === Audio.StoppedState
-    readonly property alias position: audio.position
-    readonly property url currentSource: audio.playlist.currentItemSource
+    id: root
+    readonly property real progress: priv.audio ? priv.audio.position / priv.audio.duration : 0.0
+    readonly property bool playing: priv.audio ? priv.audio.playbackState === Audio.PlayingState : false
+    readonly property bool paused: priv.audio ? priv.audio.playbackState === Audio.PausedState : false
+    readonly property bool stopped: priv.audio ? priv.audio.playbackState === Audio.StoppedState : true
+    readonly property int position: priv.audio ? priv.audio.position : 0
+    readonly property url currentSource: priv.audio ? priv.audio.playlist.currentItemSource : ""
+    readonly property Playlist playlist: priv.audio ? priv.audio.playlist : null
 
     function playSource(newSource, newPlaylist) {
+        if (!priv.audio) {
+            console.info("DashAudioPlayer: creating player");
+            priv.audio = priv.audioComponent.createObject(root);
+        }
         stop();
-        audio.playlist.clear();
+        priv.audio.playlist.clear();
         if (newPlaylist) {
             // Look for newSource in newPlaylist
             var sourceIndex = -1;
@@ -48,35 +54,53 @@
             for (var i in newPlaylist) {
                 urls.push(newPlaylist[i]);
             }
-            audio.playlist.addItems(urls);
-            audio.playlist.currentIndex = sourceIndex;
+            priv.audio.playlist.addItems(urls);
+            priv.audio.playlist.currentIndex = sourceIndex;
         } else {
-            audio.playlist.addItem(newSource);
-            audio.playlist.currentIndex = 0;
+            priv.audio.playlist.addItem(newSource);
+            priv.audio.playlist.currentIndex = 0;
         }
         play();
     }
 
     function stop() {
-        audio.stop();
+        if (priv.audio) {
+            priv.audio.stop();
+        }
     }
 
     function play() {
-        audio.play();
+        if (priv.audio) {
+            priv.audio.play();
+        }
     }
 
     function pause() {
-        audio.pause();
+        if (priv.audio) {
+            priv.audio.pause();
+        }
     }
 
-    property QtObject audio: Audio {
-        id: audio
-        objectName: "audio"
-        playlist: Playlist {
-            objectName: "playlist"
+    property QtObject priv: QtObject {
+        id: priv
+        property Audio audio: null
+        property Component audioComponent: Component {
+            Audio {
+                playlist: Playlist {
+                    objectName: "playlist"
+                }
+                /* Remove player in case of error so it gets recreated next time
+                 * we need it. Happens if backend media player restarted, for
+                 * instance. qtmultimedia should probably handle this
+                 * transparently (LP: #1616425).
+                 */
+                onError: {
+                    console.warn("DashAudioPlayer: error event (" +
+                                  priv.audio.errorString + "), destroying");
+                    priv.audio.destroy();
+                }
+            }
         }
-
-        onErrorStringChanged: console.warn("Dash Audio player error:", errorString)
     }
 
     function lengthToString(s) {

=== modified file 'qml/Components/NotificationAudio.qml'
--- qml/Components/NotificationAudio.qml	2016-04-29 20:06:54 +0000
+++ qml/Components/NotificationAudio.qml	2016-08-26 08:08:57 +0000
@@ -23,6 +23,10 @@
     readonly property var playbackState: priv.audio ? priv.audio.playbackState : 0
 
     function play() {
+        if (!priv.audio) {
+            console.info("NotificationAudio: creating player");
+            priv.audio = priv.audioComponent.createObject(root);
+        }
         if (priv.audio) {
             priv.audio.play();
         }
@@ -35,9 +39,22 @@
 
     QtObject {
         id: priv
-        property var audio: Audio {
-            source: root.source
-            audioRole: MediaPlayer.NotificationRole
+        property Audio audio: null
+        property Component audioComponent: Component {
+            Audio {
+                source: root.source
+                audioRole: MediaPlayer.NotificationRole
+                /* Remove player in case of error so it gets recreated next time
+                 * we need it. Happens if backend media player restarted, for
+                 * instance. qtmultimedia should probably handle this
+                 * transparently (LP: #1616425).
+                 */
+                onError: {
+                    console.warn("NotificationAudio: error event (" +
+                                  priv.audio.errorString + "), destroying");
+                    priv.audio.destroy();
+                }
+            }
         }
     }
 }

=== modified file 'tests/qmltests/Dash/Previews/tst_PreviewAudioPlayback.qml'
--- tests/qmltests/Dash/Previews/tst_PreviewAudioPlayback.qml	2016-02-15 15:27:09 +0000
+++ tests/qmltests/Dash/Previews/tst_PreviewAudioPlayback.qml	2016-08-26 08:08:57 +0000
@@ -127,8 +127,6 @@
             var track1PlayButton = findChild(track1Item, "playButton");
             var track2PlayButton = findChild(track2Item, "playButton");
 
-            var audio = DashAudioPlayer.audio;
-
             // All progress bars must be hidden in the beginning
             compare(track0ProgressBar.visible, false);
             compare(track1ProgressBar.visible, false);
@@ -137,7 +135,7 @@
             // Playing track 0 should make progress bar 0 visible
             mouseClick(track0PlayButton);
 
-            tryCompare(audio, "playbackState", Audio.PlayingState);
+            compare(DashAudioPlayer.playing, true);
             checkPlayerSource(0);
 
             tryCompare(track0ProgressBar, "visible", true);
@@ -146,25 +144,25 @@
 
             // Clicking the button again should pause it. The progress bar should stay visible
             mouseClick(track0PlayButton);
-            tryCompare(audio, "playbackState", Audio.PausedState);
+            compare(DashAudioPlayer.paused, true);
             checkPlayerSource(0);
             tryCompare(track0ProgressBar, "visible", true);
 
             // Continue playback
             mouseClick(track0PlayButton);
-            tryCompare(audio, "playbackState", Audio.PlayingState);
+            compare(DashAudioPlayer.playing, true);
             checkPlayerSource(0);
 
             // Playing track 1 should make progress bar 1 visible and hide progress bar 0 again
             mouseClick(track1PlayButton);
 
-            tryCompare(audio, "playbackState", Audio.PlayingState);
+            compare(DashAudioPlayer.playing, true);
             checkPlayerSource(1);
 
             // Check the playlist is song 0, 1, 2
-            checkPlayerUrls(tracksModel2["tracks"][0].source, audio.playlist.itemSource(0));
-            checkPlayerUrls(tracksModel2["tracks"][1].source, audio.playlist.itemSource(1));
-            checkPlayerUrls(tracksModel2["tracks"][2].source, audio.playlist.itemSource(2));
+            checkPlayerUrls(tracksModel2["tracks"][0].source, DashAudioPlayer.playlist.itemSource(0));
+            checkPlayerUrls(tracksModel2["tracks"][1].source, DashAudioPlayer.playlist.itemSource(1));
+            checkPlayerUrls(tracksModel2["tracks"][2].source, DashAudioPlayer.playlist.itemSource(2));
 
             tryCompare(track0ProgressBar, "visible", false);
             tryCompare(track1ProgressBar, "visible", true);
@@ -173,7 +171,7 @@
             // Playing track 2 should make progress bar 1 visible and hide progress bar 0 again
             mouseClick(track2PlayButton);
 
-            tryCompare(audio, "playbackState", Audio.PlayingState);
+            compare(DashAudioPlayer.playing, true);
             checkPlayerSource(2);
 
             tryCompare(track0ProgressBar, "visible", false);