Merge lp:~ahayzen/music-app/queue-refactor into lp:music-app/trusty

[Andrew Hayzen]

- trackQueue is now used for the queue, with currentIndex as the current
position in the trackQueue
- LibraryListModel can now saves the current query
- addQueueFromModel has been added to build the trackQueue from a
LibraryListModel
- trackClicked has been refactored
- updateMeta has been added to update the metadata of the current track and is trigged onSourceChange from the player
- currentCover is now set from Library.hasCover so it works with all
pages/models
- playing-list.js has been removed as it is not used anymore
- Various code cleanups

Major changes to the queuing system, playingList has been removed and the whole queue works through the model trackQueue.
The current query of the LibraryListModel is saved and can then be queried to load the trackQueue, this fixes bug 1204293.
The updateMeta is triggered onSourceChange of the player and is retrieved from the trackQueue and database, hopefully resolving bug 1200393.

Please test this merge thoroughly to ensure no regressions.

[John Kim (kotux)]

Hello,

How can I test this feature on my Nexus 7 grouper? I installed today's build, but the music app doesn't function properly. For instance, when I open the music app, I scroll through the songs, but the moment I tap a song or a button (play or fastforward), the app freezes. It does this regardless of how many times I force quit the app.

Andrew Hayzen <email address hidden>이 씀:
>Andrew Hayzen has proposed merging
>lp:~andrew-hayzen/music-app/queue-refactor into lp:music-app.
>
>Commit message:
>- trackQueue is now used for the queue, with currentIndex as the
>current
>position in the trackQueue
>- LibraryListModel can now saves the current query
>- addQueueFromModel has been added to build the trackQueue from a
>LibraryListModel
>- trackClicked has been refactored
>- updateMeta has been added to update the metadata of the current track
>and is trigged onSourceChange from the player
>- currentCover is now set from Library.hasCover so it works with all
>pages/models
>- playing-list.js has been removed as it is not used anymore
>- Various code cleanups
>
>Requested reviews:
> Music App Developers (music-app-dev)
>Related bugs:
>Bug #1200393 in Ubuntu Music App: "Shuffling doesn't display the
>correct song"
> https://bugs.launchpad.net/music-app/+bug/1200393
>Bug #1204293 in Ubuntu Music App: "PlayingList sometimes isn't fully
>populated causing Shuffle not to work correctly"
> https://bugs.launchpad.net/music-app/+bug/1204293
>
>For more details, see:
>https://code.launchpad.net/~andrew-hayzen/music-app/queue-refactor/+merge/177491
>
>Major changes to the queuing system, playingList has been removed and
>the whole queue works through the model trackQueue.
>The current query of the LibraryListModel is saved and can then be
>queried to load the trackQueue, this fixes bug 1204293.
>The updateMeta is triggered onSourceChange of the player and is
>retrieved from the trackQueue and database, hopefully resolving bug
>1200393.
>
>Please test this merge thoroughly to ensure no regressions.
>
>--
>https://code.launchpad.net/~andrew-hayzen/music-app/queue-refactor/+merge/177491
>You are subscribed to branch lp:music-app.

--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:80
http://91.189.93.70:8080/job/music-app-ci/67/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/music-app-precise-amd64-ci/61
    SUCCESS: http://91.189.93.70:8080/job/music-app-quantal-amd64-ci/67
    SUCCESS: http://91.189.93.70:8080/job/music-app-raring-amd64-ci/67
    SUCCESS: http://91.189.93.70:8080/job/music-app-saucy-amd64-ci/61

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/music-app-ci/67/rebuild

review: Approve (continuous-integration)

[Victor Thompson (vthompson)]

review: Approve

[Daniel Holm (danielholm)]

Impressive work, Andrew!

John (thinkndev) I created a bleeding edge PPA that we could use to submit the absolutely latest changes to and that can be used on the device: https://code.launchpad.net/~danielholm/+archive/music-app-bleeding

review: Approve

=== modified file 'LibraryListModel.qml'
--- LibraryListModel.qml	2013-07-27 15:16:00 +0000
+++ LibraryListModel.qml	2013-07-29 22:57:27 +0000
@@ -23,6 +23,8 @@
 Item {
     property ListModel model : ListModel { id: libraryModel }
     property alias count: libraryModel.count
+    property var query: null
+    property var param: null
 
     WorkerScript {
          id: worker
@@ -51,6 +53,10 @@
         libraryModel.clear();
         console.log("called LibraryListModel::populate()")
 
+        // Save query for queue
+        query = Library.getAll
+        param = null
+
         var library = Library.getAll()
 
         for ( var key in library ) {
@@ -64,6 +70,10 @@
         libraryModel.clear();
         console.log("called LibraryListModel::filterArtists()")
 
+        // Save query for queue
+        query = Library.getArtists
+        param = null
+
         var library = Library.getArtists()
 
         for ( var key in library ) {
@@ -80,6 +90,10 @@
         // libraryModel.clear();
         console.log("called LibraryListModel::filterArtistTracks()")
 
+        // Save query for queue
+        query = Library.getArtistTracks
+        param = artist
+
         var library = Library.getArtistTracks(artist)
 
         for ( var key in library ) {
@@ -93,6 +107,10 @@
         libraryModel.clear();
         console.log("called LibraryListModel::filterAlbums()")
 
+        // Save query for queue
+        query = Library.getAlbums
+        param = null
+
         var library = Library.getAlbums()
 
         for ( var key in library ) {
@@ -109,6 +127,10 @@
         // libraryModel.clear();
         console.log("called LibraryListModel::filterAlbumTracks()")
 
+        // Save query for queue
+        query = Library.getAlbumTracks
+        param = album
+
         var library = Library.getAlbumTracks(album)
 
         for ( var key in library ) {
@@ -122,6 +144,10 @@
         // libraryModel.clear(); TODO: see filterAlbumTracks(album)
         console.log("called LibraryListModel::filterPlaylistTracks()")
 
+        // Save query for queue
+        query = Playlists.getPlaylistTracks
+        param = playlist
+
         var tracks = Playlists.getPlaylistTracks(playlist)
 
         for ( var key in tracks ) {

=== modified file 'LoginLastFM.qml'
--- LoginLastFM.qml	2013-07-09 19:35:57 +0000
+++ LoginLastFM.qml	2013-07-29 22:57:27 +0000
@@ -26,7 +26,6 @@
 import QtQuick.XmlListModel 2.0
 import "settings.js" as Settings
 import "meta-database.js" as Library
-import "playing-list.js" as PlayingList
 import "scrobble.js" as Scrobble
 
 // LastFM login dialog

=== modified file 'MusicAlbums.qml'
--- MusicAlbums.qml	2013-07-25 22:45:57 +0000
+++ MusicAlbums.qml	2013-07-29 22:57:27 +0000
@@ -25,7 +25,6 @@
 import QtQuick.LocalStorage 2.0
 import "settings.js" as Settings
 import "meta-database.js" as Library
-import "playing-list.js" as PlayingList
 import "playlists.js" as Playlists
 
 PageStack {
@@ -225,7 +224,7 @@
                                 focus = true
                             }
 
-                            trackClicked(file, index, albumTracksModel.model, albumtrackslist)
+                            trackClicked(albumTracksModel, index)  // play track
                         }
                     }
                     Component.onCompleted: {

=== modified file 'MusicArtists.qml'
--- MusicArtists.qml	2013-07-25 22:45:57 +0000
+++ MusicArtists.qml	2013-07-29 22:57:27 +0000
@@ -201,7 +201,7 @@
                                 focus = true
                             }
 
-                            trackClicked(file, index, artistTracksModel.model, artisttrackslist)
+                            trackClicked(artistTracksModel, index)  // play track
                         }
                     }
                     Component.onCompleted: {

=== modified file 'MusicPlaylists.qml'
--- MusicPlaylists.qml	2013-07-27 15:16:00 +0000
+++ MusicPlaylists.qml	2013-07-29 22:57:27 +0000
@@ -27,7 +27,6 @@
 import "settings.js" as Settings
 import "meta-database.js" as Library
 import "scrobble.js" as Scrobble
-import "playing-list.js" as PlayingList
 import "playlists.js" as Playlists
 
 PageStack {
@@ -234,9 +233,6 @@
             onCurrentIndexChanged: {
                 customdebug("tracklist.currentIndex = " + playlistslist.currentIndex)
             }
-            onModelChanged: {
-                customdebug("PlayingList cleared")
-            }
 
             Component {
                 id: playlistDelegate
@@ -377,9 +373,6 @@
             onCurrentIndexChanged: {
                 console.log("Tracks in playlist tracklist.currentIndex = " + playlistlist.currentIndex)
             }
-            onModelChanged: {
-                console.log("PlayingList cleared")
-            }
 
             Component {
                 id: playlisttrackDelegate
@@ -408,7 +401,7 @@
                         }
                         onClicked: {
                             customdebug("File: " + file) // debugger
-                            trackClicked(file, index, playlisttracksModel.model, playlistlist) // play track
+                            trackClicked(playlisttracksModel, index) // play track
                         }
                     }
                 }
@@ -485,7 +478,7 @@
                         }
                         onClicked: {
                             customdebug("File: " + file) // debugger
-                            trackClicked(file, index, trackQueue.model, queuelist) // play track
+                            trackClicked(trackQueue, index) // play track
                         }
 
                         /*onItemRemoved: {

=== modified file 'MusicSettings.qml'
--- MusicSettings.qml	2013-07-09 19:35:57 +0000
+++ MusicSettings.qml	2013-07-29 22:57:27 +0000
@@ -24,7 +24,6 @@
 import "scrobble.js" as Scrobble
 import "playlists.js" as Playlists
 import "meta-database.js" as Library
-import "playing-list.js" as PlayingList
 
 Dialog {
     id: root

=== modified file 'MusicTracks.qml'
--- MusicTracks.qml	2013-07-25 22:45:57 +0000
+++ MusicTracks.qml	2013-07-29 22:57:27 +0000
@@ -25,7 +25,6 @@
 import QtQuick.LocalStorage 2.0
 import "settings.js" as Settings
 import "meta-database.js" as Library
-import "playing-list.js" as PlayingList
 import "playlists.js" as Playlists
 
 
@@ -160,22 +159,15 @@
                                 focus = true
                             }
 
-                            trackClicked(file, index, libraryModel.model, tracklist)
+                            trackClicked(libraryModel, index)  // play track
                         }
                     }
                     Component.onCompleted: {
-                        if (PlayingList.size() === 0) {
-                            player.source = file
-                            currentModel = libraryModel.model
-                            currentListView = tracklist
-                            currentIndex = 0
+                        // Set first track as current track
+                        if (trackQueue.model.count === 0) {
+                            trackClicked(libraryModel, index, false)
                         }
 
-                        if (!PlayingList.contains(file)) {
-                            console.log("Adding file:" + file)
-                            PlayingList.addItem(file, itemnum)
-                            console.log(itemnum)
-                        }
                         console.log("Title:" + title + " Artist: " + artist)
                     }
                 }

=== modified file 'music-app.qml'
--- music-app.qml	2013-07-27 15:16:00 +0000
+++ music-app.qml	2013-07-29 22:57:27 +0000
@@ -26,7 +26,6 @@
 import QtQuick.XmlListModel 2.0
 import "settings.js" as Settings
 import "meta-database.js" as Library
-import "playing-list.js" as PlayingList
 import "scrobble.js" as Scrobble
 import "playlists.js" as Playlists
 
@@ -79,9 +78,7 @@
     property string musicName: i18n.tr("Music")
     property string musicDir: ""
     property string appVersion: '0.4.7'
-    property int playing: 0
     property bool isPlaying: false
-    property int itemnum: 0
     property bool random: false
     property bool scrobble: false
     property string lastfmusername
@@ -98,40 +95,19 @@
     property string currentTracktitle: ""
     property string currentFile: ""
     property int currentIndex: -1
-    property ListView currentListView: null
-    property ListModel currentModel: null  // Current model being used
-    property int currentModelCount: -1
+    property LibraryListModel currentModel: null  // Current model being used
+    property var currentQuery: null
+    property var currentParam: null
     property string currentCover: ""
     property string currentCoverSmall: currentCover === "" ?
                                            "images/cover_default_icon.png" :
-                                           "image://cover-art/" + currentFile
+                                           "image://cover-art/" + currentCover
     property string currentCoverFull: currentCover !== "" ?
-                                          "image://cover-art-full/" + currentFile :
+                                          "image://cover-art-full/" + currentCover :
                                           "images/cover_default.png"
 
     signal onPlayingTrackChange(string source)
 
-    onCurrentIndexChanged: {
-        // Index change most likely from next/pre button press or end of song
-
-        currentListView.currentIndex = currentIndex  // update highlight in listview
-
-        // Load metadata for the track
-        currentArtist = currentModel.get(currentIndex).artist
-        currentAlbum = currentModel.get(currentIndex).album
-        currentFile = currentModel.get(currentIndex).file
-        currentCover = currentModel.get(currentIndex).cover
-        currentTracktitle = currentModel.get(currentIndex).title
-    }
-
-    onCurrentModelChanged: {
-        PlayingList.rebuild(currentModel)
-    }
-
-    onCurrentModelCountChanged: {
-        PlayingList.rebuild(currentModel)
-    }
-
     // FUNCTIONS
 
     // Custom debug funtion that's easier to shut off
@@ -155,45 +131,36 @@
         if (random) {
             var now = new Date();
             var seed = now.getSeconds();
+
+            // trackQueue must be above 1 otherwise an infinite loop will occur
             do {
-                var num = (Math.floor((PlayingList.size()) * Math.random(seed)));
+                var num = (Math.floor((trackQueue.model.count) * Math.random(seed)));
                 console.log(num)
-                console.log(playing)
-            } while (num == playing && PlayingList.size() > 0)
-            player.source = Qt.resolvedUrl(PlayingList.getList()[num])
-            currentIndex = PlayingList.at(num)
-            playing = num
+            } while (num == currentIndex && trackQueue.model.count > 1)
+            currentIndex = num
+            player.source = Qt.resolvedUrl(trackQueue.model.get(num).file)
             console.log("MediaPlayer statusChanged, currentIndex: " + currentIndex)
         } else {
-            if ((playing < PlayingList.size() - 1 && direction === 1 )
-                    || (playing > 0 && direction === -1)) {
-                console.log("playing: " + playing)
-                console.log("filelistCount: " + currentModelCount)
-                console.log("PlayingList.size(): " + PlayingList.size())
-                playing += direction
-                if (playing === 0) {
-                    currentIndex = playing + (itemnum - PlayingList.size())
-                } else {
-                    currentIndex += direction
-                }
-                player.source = Qt.resolvedUrl(PlayingList.getList()[playing])
+            if ((currentIndex < trackQueue.model.count - 1 && direction === 1 )
+                    || (currentIndex > 0 && direction === -1)) {
+                console.log("currentIndex: " + currentIndex)
+                console.log("trackQueue.count: " + trackQueue.model.count)
+                currentIndex += direction
+                player.source = Qt.resolvedUrl(trackQueue.model.get(currentIndex).file)
             } else if(direction === 1) {
-                console.log("playing: " + playing)
-                console.log("filelistCount: " + currentModelCount)
-                console.log("PlayingList.size(): " + PlayingList.size())
-                playing = 0
-                currentIndex = playing + (currentModelCount - PlayingList.size())
-                player.source = Qt.resolvedUrl(PlayingList.getList()[playing])
+                console.log("currentIndex: " + currentIndex)
+                console.log("trackQueue.count: " + trackQueue.model.count)
+                currentIndex = 0
+                player.source = Qt.resolvedUrl(trackQueue.model.get(currentIndex).file)
             } else if(direction === -1) {
-                console.log("playing: " + playing)
-                console.log("filelistCount: " + currentModelCount)
-                console.log("PlayingList.size(): " + PlayingList.size())
-                playing = PlayingList.size() - 1
-                currentIndex = playing + (currentModelCount - PlayingList.size())
-                player.source = Qt.resolvedUrl(PlayingList.getList()[playing])
+                console.log("currentIndex: " + currentIndex)
+                console.log("trackQueue.count: " + trackQueue.model.count)
+                currentIndex = trackQueue.model.count - 1
+                player.source = Qt.resolvedUrl(trackQueue.model.get(currentIndex).file)
             }
             console.log("MediaPlayer statusChanged, currentIndex: " + currentIndex)
         }
+        player.stop()  // Add stop so that if same song is selected it restarts
         console.log("Playing: "+player.source)
         player.play()
         timestamp = new Date().getTime(); // contains current date and time in Unix time, used to scrobble
@@ -206,12 +173,49 @@
         }
     }
 
-    function trackClicked(file, index, libraryModel, listView)
-    {
+    // Add items from a stored query in libraryModel into the queue
+    function addQueueFromModel(libraryModel)
+    {
+        var items;
+
+        if (libraryModel.query === null)
+        {
+            return
+        }
+
+        if (libraryModel.param === null)
+        {
+            items = libraryModel.query()
+        }
+        else
+        {
+            items = libraryModel.query(libraryModel.param)
+        }
+
+        for (var key in items)
+        {
+            trackQueue.model.append(items[key])
+        }
+    }
+
+    function trackClicked(libraryModel, index, play)
+    {
+        if (play === undefined)
+        {
+            play = true
+        }
+
+        var file = libraryModel.model.get(index).file
+
         console.debug(player.source, Qt.resolvedUrl(file))
 
         if (player.source == Qt.resolvedUrl(file))  // same file different pages what should happen then?
         {
+            if (play === false)
+            {
+                return
+            }
+
             console.log("Is current track: "+player.playbackState)
 
             if (player.playbackState == MediaPlayer.PlayingState)
@@ -226,25 +230,63 @@
             return
         }
 
-        currentListView = listView
+        // Clear the play queue and load the new tracks - if not trackQueue
+        if (libraryModel !== trackQueue)
+        {
+            // Don't reload queue if model, query and parameters are the same
+            if (currentModel !== libraryModel ||
+                    currentQuery !== libraryModel.query ||
+                        currentParam !== libraryModel.param)
+            {
+                trackQueue.model.clear()
+                addQueueFromModel(libraryModel)
+            }
+        }
+
+        // Current index must be updated before player.source
         currentModel = libraryModel
-        currentModelCount = libraryModel.count
-        currentIndex = index  // update index after model so index can get info from model
+        currentQuery = libraryModel.query
+        currentParam = libraryModel.param
+        currentIndex = trackQueue.indexOf(file)
 
         console.log("Click of fileName: " + file)
 
-        player.stop()
+        if (play === true)
+        {
+            player.stop()
+        }
+
         player.source = Qt.resolvedUrl(file)
-        player.play()
 
-        playing = PlayingList.indexOf(file)
+        if (play === true)
+        {
+            player.play()
+        }
 
         console.log("Source: " + player.source.toString())
-        console.log("Index: " + index)
+        console.log("Index: " + currentIndex)
 
         return file
     }
 
+    function updateMeta()
+    {
+        // Load metadata for the track
+        currentArtist = trackQueue.model.get(currentIndex).artist
+        currentAlbum = trackQueue.model.get(currentIndex).album
+        currentTracktitle = trackQueue.model.get(currentIndex).title
+
+        // hasCover and currentCover require no file://
+        var file = currentFile
+
+        if (file.indexOf("file://") == 0)
+        {
+            file = file.slice(7, file.length)
+        }
+
+        currentCover = Library.hasCover(file) ? file : ""
+    }
+
     MediaPlayer {
         id: player
         objectName: "player"
@@ -257,7 +299,9 @@
         property string positionStr: "00:00"
 
         onSourceChanged: {
+            currentFile = source
             onPlayingTrackChange(source)
+            updateMeta()
         }
 
         onStatusChanged: {
@@ -459,8 +503,6 @@
                 libraryModel.populate()
                 albumModel.filterAlbums()
                 artistModel.filterArtists()
-                PlayingList.clear()
-                itemnum = 0
                 timer.stop()
             }
             counted = filelist.count

=== removed file 'playing-list.js'
--- playing-list.js	2013-07-18 21:57:52 +0000
+++ playing-list.js	1970-01-01 00:00:00 +0000
@@ -1,61 +0,0 @@
-/*
- * Copyright (C) 2013 Victor Thompson <[e-mail]>
- *                    Daniel Holm <[e-mail]>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; version 3.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- */
-
-.pragma library
-var myArray = new Array()
-var myLocation = new Array()
-
-function getList() {
-    return myArray
-}
-
-function at(index) {
-    return myLocation[index]
-}
-
-function addItem(item, index) {
-    myArray.push(item)
-    myLocation.push(index)
-}
-
-function contains(item) {
-    return myArray.indexOf(item) !== -1
-}
-
-function indexOf(item) {
-    return myArray.indexOf(item)
-}
-
-function size() {
-    return myArray.length
-}
-
-function clear() {
-    myArray = []
-    myLocation = []
-}
-
-function rebuild(currentModel)
-{
-    console.debug("Clearing playing list")
-    clear()
-
-    for (var i=0; i < currentModel.count; i++)
-    {
-        addItem(currentModel.get(i).file, i)
-    }
-}