Unity 8

Merge lp:~aacid/unity8/preview_audio_playlist into lp:unity8

preview_audio_playlist
Merge into trunk

Proposed by Albert Astals Cid on 2016-02-01

Status:

Merged

Approved by:

Michał Sawicz on 2016-02-16

Approved revision:

2145

Merged at revision:

2200

Proposed branch:

lp:~aacid/unity8/preview_audio_playlist

Merge into:

lp:unity8

Diff against target:

52 lines (+18/-5)

2 files modified

qml/Dash/Previews/PreviewAudioPlayback.qml (+5/-1)
tests/qmltests/Dash/Previews/tst_PreviewAudioPlayback.qml (+13/-4)

To merge this branch:

bzr merge lp:~aacid/unity8/preview_audio_playlist

Undecided

Fix Committed

Link a bug report

Reviewer	Review Type	Date Requested	Status
Michał Sawicz	code		Approve on 2016-02-16
PS Jenkins bot	continuous-integration		Needs Fixing on 2016-02-15
Unity8 CI Bot	continuous-integration		Needs Fixing on 2016-02-15
Andrea Cimitan (community)		2016-02-01	Approve on 2016-02-01
Review via email: mp+284624@code.launchpad.net

Commit Message

Make the audio previews create a playlist

Description of the Change

* Are there any related MPs required for this MP to build/function as expected?
No

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

* Did you make sure that your branch does not contain spurious tags?
Yes

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

* If you changed the UI, has there been a design review?
N/A

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-02-01:

PASSED: Continuous integration, rev:2144
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/235/
Executed test runs:

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

review: Approve (continuous-integration)

Andrea Cimitan (cimi) wrote on 2016-02-01:

* 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.
green
* Did you make sure that the branch does not contain spurious tags?
yes

review: Approve

PS Jenkins bot (ps-jenkins) wrote on 2016-02-01:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/7184/rebuild

review: Needs Fixing (continuous-integration)

Michał Sawicz (saviq) wrote on 2016-02-11:

When you start playback from the second or further item, the playlist "wraps" in that the song you start playback with is always the first one in the playlist.

I'd say the playlist sent to media-hub should always be in the same order that the scope displays it.

review: Needs Fixing

Albert Astals Cid (aacid) wrote on 2016-02-15:

> When you start playback from the second or further item, the playlist "wraps"
> in that the song you start playback with is always the first one in the
> playlist.
>
> I'd say the playlist sent to media-hub should always be in the same order that
> the scope displays it.

That's not what UX wanted as far as i understood, have you confirmed this behaviour with them?

Albert Astals Cid (aacid) wrote on 2016-02-15:

> > When you start playback from the second or further item, the playlist
> "wraps"
> > in that the song you start playback with is always the first one in the
> > playlist.
> >
> > I'd say the playlist sent to media-hub should always be in the same order
> that
> > the scope displays it.
>
> That's not what UX wanted as far as i understood, have you confirmed this
> behaviour with them?

Actually it's on the bug description, so yes please speak with Patricia if you disagree with her design.

lp:~aacid/unity8/preview_audio_playlist updated on 2016-02-15

2145. By Albert Astals Cid on 2016-02-15: Playlist from 0 to last song

Albert Astals Cid (aacid) wrote on 2016-02-15:

Changed code since Patricia agreed with Saviq

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-02-15:

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

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2016-02-15:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/7291/rebuild

review: Needs Fixing (continuous-integration)

Michał Sawicz (saviq) on 2016-02-16:

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Albert Astals Cid

Mohammad Rakibul Hasan

Unity Team

 === modified file 'qml/Dash/Previews/PreviewAudioPlayback.qml'
 --- qml/Dash/Previews/PreviewAudioPlayback.qml	2016-01-18 17:08:29 +0000
 +++ qml/Dash/Previews/PreviewAudioPlayback.qml	2016-02-15 15:27:35 +0000
@@ -85,7 +85,11 @@
                                      DashAudioPlayer.play();
+                                 }
                              } else {
--                                DashAudioPlayer.playSource(sourceUrl);
++                                var playlist = [];
++                                for (var i = 0; i < trackRepeater.count; ++i) {
++                                    playlist.push(trackRepeater.itemAt(i).sourceUrl);
++                                }
++                                DashAudioPlayer.playSource(sourceUrl, playlist);
+                             }
+                         }
+                     }
 === modified file 'tests/qmltests/Dash/Previews/tst_PreviewAudioPlayback.qml'
 --- tests/qmltests/Dash/Previews/tst_PreviewAudioPlayback.qml	2015-12-01 14:02:21 +0000
 +++ tests/qmltests/Dash/Previews/tst_PreviewAudioPlayback.qml	2016-02-15 15:27:35 +0000
@@ -100,11 +100,15 @@
+             }
+         }
++        function checkPlayerUrls(modelFilename, playerUrl) {
++            var modelFilename = modelFilename.replace(/^.*[\\\/]/, '');
++            var playerFilename = playerUrl.toString().replace(/^.*[\\\/]/, '');
++
++            compare(modelFilename, playerFilename, "Player source is not set correctly.");
++        }
++
          function checkPlayerSource(index) {
--            var modelFilename = previewAudioPlayback.widgetData["tracks"][index]["source"].replace(/^.*[\\\/]/, '');
--            var playerFilename = DashAudioPlayer.currentSource.toString().replace(/^.*[\\\/]/, '');
--
--            compare(modelFilename, playerFilename, "Player source is not set correctly.");
++            checkPlayerUrls(previewAudioPlayback.widgetData["tracks"][index]["source"], DashAudioPlayer.currentSource);
+         }
          function test_playback() {
@@ -157,6 +161,11 @@
              tryCompare(audio, "playbackState", Audio.PlayingState);
              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));
++
              tryCompare(track0ProgressBar, "visible", false);
              tryCompare(track1ProgressBar, "visible", true);
              tryCompare(track2ProgressBar, "visible", false);