After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 705855 - Crash when attempting to get properties over DBus
Crash when attempting to get properties over DBus
Status: RESOLVED FIXED
Product: gnome-music
Classification: Applications
Component: general
unspecified
Other Linux
: Normal major
: 3.10
Assigned To: Arnel Borja
gnome-music-maint
Depends on:
Blocks:
 
 
Reported: 2013-08-12 15:27 UTC by Vadim Rutkovsky
Modified: 2013-09-02 16:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
mpris: Don't check if has next and previous track if there's no playlist (1.81 KB, patch)
2013-08-19 01:43 UTC, Arnel Borja
none Details | Review
player: Has next and previous track should be false if there's no playlist (1.48 KB, patch)
2013-08-19 02:18 UTC, Arnel Borja
committed Details | Review
mpris: Use Songs view if has no playlist (2.36 KB, patch)
2013-09-02 16:06 UTC, Arnel Borja
committed Details | Review
view: Make adding rows to models atomic (3.97 KB, patch)
2013-09-02 16:06 UTC, Arnel Borja
committed Details | Review
player: Store current track as row reference (5.37 KB, patch)
2013-09-02 16:06 UTC, Arnel Borja
committed Details | Review
player: Sync buttons when playlist is modified (2.10 KB, patch)
2013-09-02 16:07 UTC, Arnel Borja
committed Details | Review
view: Fix crash when artist is unknown (1.92 KB, patch)
2013-09-02 16:07 UTC, Arnel Borja
committed Details | Review

Description Vadim Rutkovsky 2013-08-12 15:27:39 UTC
From https://github.com/gnome-prototypes-team/gnome-music/issues/164


When nothing is playing, attempts to get any MPRISv2 property return:

org.freedesktop.DBus.Python.AttributeError: Traceback (most recent call last):
  • File "/usr/lib/python3.3/site-packages/dbus/service.py", line 707 in _message_cb
    retval = candidate_method(self, *args, **keywords)
  • File "/usr/lib/python3.3/site-packages/gnomemusic/mpris.py", line 184 in Get
    return self.GetAll(interface_name)[property_name]
  • File "/usr/lib/python3.3/site-packages/gnomemusic/mpris.py", line 217 in GetAll
    'CanGoNext': self.player.has_next(),
  • File "/usr/lib/python3.3/site-packages/gnomemusic/player.py", line 174 in has_next
    tmp = self.currentTrack.copy()
AttributeError: 'NoneType' object has no attribute 'copy'

Comment 1 Arnel Borja 2013-08-12 15:50:45 UTC
Hm, right, we are supposed to select a default playlist here - that's how other players handle this. Maybe we should set Songs view as default.
Comment 2 Vadim Rutkovsky 2013-08-12 16:12:33 UTC
+1 to comment #2, I think we should switch to Songs view then and start playback.

On the other hand, current implementation will not play all the songs there - as we need to load them all via Load More.
Comment 3 Mantas Mikulėnas (grawity) 2013-08-12 17:18:02 UTC
(In reply to comment #2)
> +1 to comment #2, I think we should switch to Songs view then and start
> playback.

Starting playback whenever someone wants to retrieve the status properties sounds a bit unusual...
Comment 4 Arnel Borja 2013-08-19 01:43:08 UTC
Created attachment 252172 [details] [review]
mpris: Don't check if has next and previous track if there's no playlist

This prevents a crash when there is no selected playlist yet and an MPRIS client
tries to get properties of org.mpris.MediaPlayer2.Player interface.
Comment 5 Arnel Borja 2013-08-19 01:49:21 UTC
(In reply to comment #2)
> +1 to comment #2, I think we should switch to Songs view then and start
> playback.
> 
> On the other hand, current implementation will not play all the songs there -
> as we need to load them all via Load More.

I attached a partial fix for this so that gnome-music won't crash when there's no selected playlist yet. Could you please check if it works? I'll probably commit this before the release today though - I don't want this crash to be in our first release.

(In reply to comment #3)
> Starting playback whenever someone wants to retrieve the status properties
> sounds a bit unusual...

He probably meant to play a song when Play() is called in MPRIS.
Comment 6 Arnel Borja 2013-08-19 02:18:32 UTC
Created attachment 252173 [details] [review]
player: Has next and previous track should be false if there's no playlist

This prevents a crash when there is no selected playlist yet and an MPRIS
client tries to get properties of org.mpris.MediaPlayer2.Player interface.
Comment 7 Arnel Borja 2013-08-19 02:20:20 UTC
Comment on attachment 252173 [details] [review]
player: Has next and previous track should be false if there's no playlist

Argh, accidentally pushed it when I was supposed to attach it here.

This fix is much better, the other one only handles mpris.
Comment 8 Arnel Borja 2013-09-01 15:33:36 UTC
Branch for switching to Songs view then starting playback:
https://github.com/gnome-prototypes-team/gnome-music/tree/default-playlist
Comment 9 Arnel Borja 2013-09-02 16:06:35 UTC
Created attachment 253853 [details] [review]
mpris: Use Songs view if has no playlist

Select Songs view as playlist if there's none and Play is called.
Comment 10 Arnel Borja 2013-09-02 16:06:47 UTC
Created attachment 253854 [details] [review]
view: Make adding rows to models atomic

This ensures that when row-inserted signal is emitted, the row already
have values for all columns.
Comment 11 Arnel Borja 2013-09-02 16:06:57 UTC
Created attachment 253855 [details] [review]
player: Store current track as row reference

Ensures that currentTrack iter won't get invalidated if a value changes
in its row in the model.
Comment 12 Arnel Borja 2013-09-02 16:07:08 UTC
Created attachment 253856 [details] [review]
player: Sync buttons when playlist is modified

Sync prev and next buttons when a track gets added or removed in the
current playlist.
Comment 13 Arnel Borja 2013-09-02 16:07:14 UTC
Created attachment 253857 [details] [review]
view: Fix crash when artist is unknown
Comment 14 Arnel Borja 2013-09-02 16:21:19 UTC
Comment on attachment 253853 [details] [review]
mpris: Use Songs view if has no playlist

Committed as 227ec60.
Comment 15 Arnel Borja 2013-09-02 16:21:35 UTC
Comment on attachment 253854 [details] [review]
view: Make adding rows to models atomic

Committed as cd32ba0.
Comment 16 Arnel Borja 2013-09-02 16:21:56 UTC
Comment on attachment 253855 [details] [review]
player: Store current track as row reference

Committed as e1a311f.
Comment 17 Arnel Borja 2013-09-02 16:22:16 UTC
Comment on attachment 253856 [details] [review]
player: Sync buttons when playlist is modified

Committed as 25cfb09.
Comment 18 Arnel Borja 2013-09-02 16:22:37 UTC
Comment on attachment 253857 [details] [review]
view: Fix crash when artist is unknown

Committed as 9aa86fd.