Merge lp:~vthompson/music-app/fixes-1234990 into lp:music-app/trusty

Proposed by Victor Thompson
Status: Merged
Approved by: Daniel Holm
Approved revision: 241
Merged at revision: 234
Proposed branch: lp:~vthompson/music-app/fixes-1234990
Merge into: lp:music-app/trusty
Diff against target: 399 lines (+112/-89)
8 files modified
LibraryListModel.qml (+4/-0)
MusicNowPlaying.qml (+1/-0)
MusicStart.qml (+1/-0)
debian/changelog (+14/-2)
manifest.json (+1/-1)
music-app.qml (+34/-38)
tests/autopilot/music_app/emulators.py (+6/-0)
tests/autopilot/music_app/tests/test_music.py (+51/-48)
To merge this branch: bzr merge lp:~vthompson/music-app/fixes-1234990
Reviewer Review Type Date Requested Status
Daniel Holm Approve
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Sergio Schvezov (community) Abstain
Review via email: mp+192771@code.launchpad.net

Commit message

Remove timer used to load the models.

Description of the change

Remove timer used to load the models. Instead, depend on the Grilo plugin's onFinished signal. On my desktop with 17.4 GB of music this reduces the time it takes the Grilo plugin to load from 17 seconds to 7 seconds.

real 0m17.079s
user 0m17.124s
sys 0m0.943s

real 0m7.069s
user 0m5.032s
sys 0m0.714s

Similarly, from the time to launch the app to when the listviews and other items in the app are loaded the time went down from 43 seconds to 34 seconds. When the Songs tab is
not loaded upon application initialization, but instead when the tab is selected this
time goes down to 6 seconds.

real 0m43.194s
user 0m41.096s
sys 0m1.306s

real 0m34.542s
user 0m32.103s
sys 0m1.418s

real 0m6.314s
user 0m4.516s
sys 0m0.709s

Additionally, a subjective test on a Nexus 4 running the latest (r5) 14.04 build saw the app start up time go from ~7 seconds to 4 or 5 seconds. When the Songs tab is not loaded upon application initialization this goes down to about 2 seconds.

To post a comment you must log in.
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
231. By Victor Thompson

Do not populate Songs tab until the tab is selected.

Revision history for this message
Victor Thompson (vthompson) wrote :

Avoiding loading the Songs tab's model until it is selected GREATLY reduces the overall load time. In my previous run the 34 second load time was reduced to around 7 seconds. The added delay when selecting the Songs tab the first time is relatively minimal.

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
232. By Victor Thompson

Do not populate Songs tab until the tab is selected, fixed.

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
233. By Victor Thompson

Change one of the conditions.

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
234. By Victor Thompson

Load Artist and Album models when tab is selected.

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
235. By Victor Thompson

resolve AP test failures

236. By Victor Thompson

oops

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
237. By Victor Thompson

Add blank line

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
238. By Victor Thompson

Increment revision in anticipation of OTA upgrade

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
239. By Victor Thompson

Still load songs tab, this reduces the speed improvements of this branch but a little--but the overall increas is still significant.

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Sergio Schvezov (sergiusens) wrote :

can you bump the manifest.json version entry once prepared to release as well?

review: Abstain
240. By Victor Thompson

Update version in manifest.json

241. By Victor Thompson

Fix AP tests--should not need to explicitly show toolbar.

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Victor Thompson (vthompson) wrote :

I think what we have now should be what lands. The work to only load the Songs tab's model until it was clicked had 2 downfalls 1) clicking a song from the dash didn't play when the app was already loaded and 2) the toolbar was not automatically being shown. The time savings of avoiding populating this model is something we should look into further, however.

Revision history for this message
Daniel Holm (danielholm) wrote :

Lovely work. It is in fact visible faster!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'LibraryListModel.qml'
2--- LibraryListModel.qml 2013-10-17 23:18:52 +0000
3+++ LibraryListModel.qml 2013-10-30 12:14:27 +0000
4@@ -214,4 +214,8 @@
5 worker.sendMessage({'add': add, 'model': libraryModel})
6 }
7 }
8+
9+ function clear() {
10+ worker.sendMessage({'clear': true, 'model': libraryModel})
11+ }
12 }
13
14=== modified file 'MusicNowPlaying.qml'
15--- MusicNowPlaying.qml 2013-10-17 01:58:15 +0000
16+++ MusicNowPlaying.qml 2013-10-30 12:14:27 +0000
17@@ -812,6 +812,7 @@
18 }
19
20 MouseArea {
21+ objectName: "nowPlayingBackButtonObject"
22 anchors.fill: parent
23
24 onClicked: {
25
26=== modified file 'MusicStart.qml'
27--- MusicStart.qml 2013-10-22 04:56:27 +0000
28+++ MusicStart.qml 2013-10-30 12:14:27 +0000
29@@ -221,6 +221,7 @@
30 id: genreDelegate
31 Item {
32 id: genreItem
33+ objectName: "genreItemObject"
34 height: units.gu(20)
35 width: units.gu(20)
36 UbuntuShape {
37
38=== modified file 'debian/changelog'
39--- debian/changelog 2013-10-14 16:28:17 +0000
40+++ debian/changelog 2013-10-30 12:14:27 +0000
41@@ -1,14 +1,26 @@
42+music-app (1.1) raring; urgency=low
43+
44+ * Blurred album art added to the now playing queue
45+ * Solid labels on cover art
46+ * Test shuffle and mp3 playback
47+ * Enable translations
48+ * Add recent albums and playlists to Music tab
49+ * Introduce user metrics of songs played
50+ * Optimize music library loading time
51+
52+ -- Victor Thompson <victor.thompson@gmail.com> Tue, 29 Oct 2013 20:16:34 -0500
53+
54 music-app (1.0) raring; urgency=low
55
56 * Finalize changes before Saucy 13.10 release.
57
58- -- Victor Thompson <victor.thompson@gmail.com> Thur, 10 Oct 2013 04:21:11 -0500
59+ -- Victor Thompson <victor.thompson@gmail.com> Thu, 10 Oct 2013 04:21:11 -0500
60
61 music-app (0.9) raring; urgency=low
62
63 * A LOT of changes.
64
65- -- Daniel Holm <d.holmen@gmail.com> Thur, 10 Oct 2013 00:39:50 +0200
66+ -- Daniel Holm <d.holmen@gmail.com> Thu, 10 Oct 2013 00:39:50 +0200
67
68 music-app (0.8) saucy; urgency=low
69
70
71=== modified file 'manifest.json'
72--- manifest.json 2013-10-28 17:24:52 +0000
73+++ manifest.json 2013-10-30 12:14:27 +0000
74@@ -12,5 +12,5 @@
75 "maintainer": "Ubuntu App Cats <ubuntu-touch-coreapps@lists.launchpad.net>",
76 "name": "com.ubuntu.music",
77 "title": "music",
78- "version": "1.0"
79+ "version": "1.1"
80 }
81
82=== modified file 'music-app.qml'
83--- music-app.qml 2013-10-30 00:51:47 +0000
84+++ music-app.qml 2013-10-30 12:14:27 +0000
85@@ -217,7 +217,7 @@
86
87 // VARIABLES
88 property string musicName: i18n.tr("Music")
89- property string appVersion: '1.0'
90+ property string appVersion: '1.1'
91 property bool isPlaying: false
92 property bool hasRecent: !Library.isRecentEmpty()
93 property bool random: false
94@@ -652,25 +652,24 @@
95 onBaseMediaChanged: refresh();
96
97 onFinished: {
98- griloModel.loaded = true
99- }
100- }
101-
102- onCountChanged: {
103- if (count > 0) {
104- timer.start()
105- for (var i = timer.counted; i < griloModel.count; i++)
106+ for (var i = 0; i < griloModel.count; i++)
107 {
108- var file = griloModel.get(i).url.toString()
109+ var media = griloModel.get(i)
110+ var file = media.url.toString()
111 if (file.indexOf("file://") == 0)
112 {
113 file = file.slice(7, file.length)
114 }
115- //console.log("Artist:"+ griloModel.get(i).artist + ", Album:"+griloModel.get(i).album + ", Title:"+griloModel.get(i).title + ", File:"+file + ", Cover:"+griloModel.get(i).thumbnail + ", Number:"+griloModel.get(i).trackNumber + ", Genre:"+griloModel.get(i).genre);
116- Library.setMetadata(file, griloModel.get(i).title, griloModel.get(i).artist, griloModel.get(i).album, griloModel.get(i).thumbnail, griloModel.get(i).year, griloModel.get(i).trackNumber, griloModel.get(i).duration, griloModel.get(i).genre)
117+ //console.log("Artist:"+ media.artist + ", Album:"+media.album + ", Title:"+media.title + ", File:"+file + ", Cover:"+media.thumbnail + ", Number:"+media.trackNumber + ", Genre:"+media.genre);
118+ Library.setMetadata(file, media.title, media.artist, media.album, media.thumbnail, media.year, media.trackNumber, media.duration, media.genre)
119 }
120+ Library.writeDb()
121+ recentModel.filterRecent()
122+ genreModel.filterGenres()
123+ libraryModel.populate()
124+ loading.visible = false
125+ griloModel.loaded = true
126 }
127-
128 }
129 }
130
131@@ -773,31 +772,6 @@
132 id: undo
133 }
134
135- Timer {
136- id: timer
137- interval: 200; repeat: true
138- running: false
139- triggeredOnStart: false
140- property int counted: 0
141-
142- onTriggered: {
143- console.log("Counted: " + counted)
144- console.log("griloModel.count: " + griloModel.count)
145- if (counted === griloModel.count) {
146- console.log("MOVING ON")
147- Library.writeDb()
148- libraryModel.populate()
149- albumModel.filterAlbums()
150- artistModel.filterArtists()
151- recentModel.filterRecent()
152- genreModel.filterGenres()
153- timer.stop()
154- loading.visible = false
155- }
156- counted = griloModel.count
157- }
158- }
159-
160 // Blurred background
161 BlurredBackground {
162 }
163@@ -933,10 +907,17 @@
164
165 // Second tab is arists
166 Tab {
167+ property bool populated: false
168 id: artistsTab
169 objectName: "artiststab"
170 anchors.fill: parent
171 title: i18n.tr("Artists")
172+ onVisibleChanged: {
173+ if (visible && !populated && griloModel.loaded) {
174+ artistModel.filterArtists()
175+ populated = true
176+ }
177+ }
178
179 // tab content
180 page: MusicArtists {
181@@ -946,10 +927,17 @@
182
183 // third tab is albums
184 Tab {
185+ property bool populated: false
186 id: albumsTab
187 objectName: "albumstab"
188 anchors.fill: parent
189 title: i18n.tr("Albums")
190+ onVisibleChanged: {
191+ if (visible && !populated && griloModel.loaded) {
192+ albumModel.filterAlbums()
193+ populated = true
194+ }
195+ }
196
197 // Tab content begins here
198 page: MusicAlbums {
199@@ -959,10 +947,18 @@
200
201 // fourth tab is all songs
202 Tab {
203+ property bool populated: false
204 id: tracksTab
205 objectName: "trackstab"
206 anchors.fill: parent
207 title: i18n.tr("Songs")
208+ // TODO: offloading this revents file arguments from working
209+ /* onVisibleChanged: {
210+ if (visible && !populated && griloModel.loaded) {
211+ libraryModel.populate()
212+ populated = true
213+ }
214+ } */
215
216 // Tab content begins here
217 page: MusicTracks {
218
219=== modified file 'tests/autopilot/music_app/emulators.py'
220--- tests/autopilot/music_app/emulators.py 2013-10-15 07:07:12 +0000
221+++ tests/autopilot/music_app/emulators.py 2013-10-30 12:14:27 +0000
222@@ -87,3 +87,9 @@
223 def get_spinner(self):
224 return self.select_single("ActivityIndicator",
225 objectName="LoadingSpinner")
226+
227+ def get_first_genre_item(self):
228+ return self.select_single("*", objectName="genreItemObject")
229+
230+ def get_back_button(self):
231+ return self.select_single("*", objectName="nowPlayingBackButtonObject")
232
233=== modified file 'tests/autopilot/music_app/tests/test_music.py'
234--- tests/autopilot/music_app/tests/test_music.py 2013-10-17 22:05:30 +0000
235+++ tests/autopilot/music_app/tests/test_music.py 2013-10-30 12:14:27 +0000
236@@ -31,62 +31,69 @@
237 """ tests if the music library is populated from our
238 fake mediascanner database"""
239
240+ # populate queue
241+ first_genre_item = self.main_view.get_first_genre_item()
242+ self.pointing_device.click_object(first_genre_item)
243+
244 title = lambda: self.main_view.currentTracktitle
245 artist = lambda: self.main_view.currentArtist
246 self.assertThat(title,
247 Eventually(Equals("Foss Yeaaaah! (Radio Edit)")))
248 self.assertThat(artist, Eventually(Equals("Benjamin Kerensa")))
249
250- def test_play_pause(self):
251+ def test_play_pause_library(self):
252 """ Test playing and pausing a track (Music Library must exist) """
253
254+ # populate queue
255+ first_genre_item = self.main_view.get_first_genre_item()
256+ self.pointing_device.click_object(first_genre_item)
257+
258+ # click back button
259+ back_button = self.main_view.get_back_button()
260+ self.pointing_device.click_object(back_button)
261+
262 self.main_view.show_toolbar()
263-
264 playbutton = self.main_view.get_play_button()
265
266- """ Track is not playing"""
267- self.assertThat(self.main_view.isPlaying, Eventually(Equals(False)))
268- self.pointing_device.click_object(playbutton)
269-
270- """ Track is playing"""
271- self.assertThat(self.main_view.isPlaying, Eventually(Equals(True)))
272-
273- """ Track is not playing"""
274- self.pointing_device.click_object(playbutton)
275- self.assertThat(self.main_view.isPlaying, Eventually(Equals(False)))
276+ """ Track is playing"""
277+ self.assertThat(self.main_view.isPlaying, Eventually(Equals(True)))
278+ self.pointing_device.click_object(playbutton)
279+
280+ """ Track is not playing"""
281+ self.assertThat(self.main_view.isPlaying, Eventually(Equals(False)))
282+
283+ """ Track is playing"""
284+ self.pointing_device.click_object(playbutton)
285+ self.assertThat(self.main_view.isPlaying, Eventually(Equals(True)))
286
287 def test_play_pause_now_playing(self):
288 """ Test playing and pausing a track (Music Library must exist) """
289
290- self.main_view.show_toolbar()
291-
292- # switch to the now playing page
293- label = self.main_view.get_player_control_title()
294- self.pointing_device.click_object(label)
295+ # populate queue
296+ first_genre_item = self.main_view.get_first_genre_item()
297+ self.pointing_device.click_object(first_genre_item)
298
299 self.assertThat(self.main_view.get_now_playing_play_button,
300 Eventually(NotEquals(None)))
301 playbutton = self.main_view.get_now_playing_play_button()
302
303- """ Track is not playing"""
304- self.assertThat(self.main_view.isPlaying, Eventually(Equals(False)))
305- self.pointing_device.click_object(playbutton)
306-
307- """ Track is playing"""
308- self.assertThat(self.main_view.isPlaying, Eventually(Equals(True)))
309-
310- """ Track is not playing"""
311- self.pointing_device.click_object(playbutton)
312- self.assertThat(self.main_view.isPlaying, Eventually(Equals(False)))
313+ """ Track is playing"""
314+ self.assertThat(self.main_view.isPlaying, Eventually(Equals(True)))
315+ self.pointing_device.click_object(playbutton)
316+
317+ """ Track is not playing"""
318+ self.assertThat(self.main_view.isPlaying, Eventually(Equals(False)))
319+
320+ """ Track is playing"""
321+ self.pointing_device.click_object(playbutton)
322+ self.assertThat(self.main_view.isPlaying, Eventually(Equals(True)))
323
324 def test_next(self):
325 """ Test going to next track (Music Library must exist) """
326
327- self.main_view.show_toolbar()
328-
329- # switch to the now playing page
330- label = self.main_view.get_player_control_title()
331- self.pointing_device.click_object(label)
332+ # populate queue
333+ first_genre_item = self.main_view.get_first_genre_item()
334+ self.pointing_device.click_object(first_genre_item)
335
336 forwardbutton = self.main_view.get_forward_button()
337
338@@ -96,8 +103,8 @@
339 Eventually(Equals("Foss Yeaaaah! (Radio Edit)")))
340 self.assertThat(artist, Eventually(Equals("Benjamin Kerensa")))
341
342- """ Track is not playing"""
343- self.assertThat(self.main_view.isPlaying, Equals(False))
344+ """ Track is playing"""
345+ self.assertThat(self.main_view.isPlaying, Equals(True))
346 self.pointing_device.click_object(forwardbutton)
347
348 """ Track is playing"""
349@@ -109,11 +116,9 @@
350 """ Test going to previous track, last item must be an MP3
351 (Music Library must exist) """
352
353- self.main_view.show_toolbar()
354-
355- # switch to the now playing page
356- label = self.main_view.get_player_control_title()
357- self.pointing_device.click_object(label)
358+ # populate queue
359+ first_genre_item = self.main_view.get_first_genre_item()
360+ self.pointing_device.click_object(first_genre_item)
361
362 self.assertThat(self.main_view.get_repeat_button,
363 Eventually(NotEquals(None)))
364@@ -129,8 +134,8 @@
365 Eventually(Equals("Foss Yeaaaah! (Radio Edit)")))
366 self.assertThat(artist, Eventually(Equals("Benjamin Kerensa")))
367
368- """ Track is not playing, repeat is off"""
369- self.assertThat(self.main_view.isPlaying, Equals(False))
370+ """ Track is playing, repeat is off"""
371+ self.assertThat(self.main_view.isPlaying, Equals(True))
372 self.pointing_device.click_object(repeatbutton)
373 self.pointing_device.click_object(previousbutton)
374
375@@ -142,11 +147,9 @@
376 def test_shuffle(self):
377 """ Test shuffle (Music Library must exist) """
378
379- self.main_view.show_toolbar()
380-
381- # switch to the now playing page
382- label = self.main_view.get_player_control_title()
383- self.pointing_device.click_object(label)
384+ # populate queue
385+ first_genre_item = self.main_view.get_first_genre_item()
386+ self.pointing_device.click_object(first_genre_item)
387
388 self.assertThat(self.main_view.get_shuffle_button,
389 Eventually(NotEquals(None)))
390@@ -166,8 +169,8 @@
391 Eventually(Equals("Foss Yeaaaah! (Radio Edit)")))
392 self.assertThat(artist, Eventually(Equals("Benjamin Kerensa")))
393
394- """ Track is not playing, shuffle is turned on"""
395- self.assertThat(self.main_view.isPlaying, Equals(False))
396+ """ Track is playing, shuffle is turned on"""
397+ self.assertThat(self.main_view.isPlaying, Equals(True))
398 self.pointing_device.click_object(shufflebutton)
399 self.assertThat(self.main_view.random, Eventually(Equals(True)))
400

Subscribers

People subscribed via source and target branches

to status/vote changes: