Merge lp:~barcc/repeat-one-song/one-second-issue into lp:repeat-one-song/trunk

Proposed by B. Clausius
Status: Merged
Approved by: Eduardo Mucelli Rezende Oliveira
Approved revision: 15
Merged at revision: 16
Proposed branch: lp:~barcc/repeat-one-song/one-second-issue
Merge into: lp:repeat-one-song/trunk
Diff against target: 122 lines (+43/-25)
1 file modified
__init__.py (+43/-25)
To merge this branch: bzr merge lp:~barcc/repeat-one-song/one-second-issue
Reviewer Review Type Date Requested Status
Eduardo Mucelli Rezende Oliveira Approve
B. Clausius (community) Needs Fixing
Review via email: mp+39285@code.launchpad.net

Description of the change

This change solves this issues:
 * last second from a song is cut off
 * if the song duration is wrong (too long) the song is not repeated at all
 * the play count is not incremented
Unfortunately this introduces another issue: If a song is playing and the user selects an other song, the plugin gets confused. Somehow in the playing-song-changed signal there should be differentiated between user initiated and end of song.

To post a comment you must log in.
Revision history for this message
Eduardo Mucelli Rezende Oliveira (eduardo-mucelli) wrote :

First of all, thanks for you effort to make this plugin better. My first approach when I started it was to call the do_previous with the same kind of "state machine" you did in the callback method on_song_change. Consequently, the erroneous behavior which happens when the user changes a song by himself happened to me, the same one that you pointed out -- and I totally agree with you about that there should be a way to differentiate its sources.

The last second cut off I guess is not a big deal, but I agree it is a problem. The play count can be fixed using rhythmdb, and I'd not realized until the moment, thanks again.

One awesome thing that the do_previous approach (I guess it) has, is the possibility to fix the crossfader problem reported by another user. I was wondering that last night.

Anyway, I'm really restricted about the time, and I will check everything out to answer your merge proposal.

Thanks B Clausius

14. By B. Clausius

Use the end-of-stream signal to initiate repetition.

15. By B. Clausius

Merge from trunk

Revision history for this message
B. Clausius (barcc) wrote :

I found out, how to connect to the end-of-stream signal. I think this fixes all issues mentioned above.

Revision history for this message
Eduardo Mucelli Rezende Oliveira (eduardo-mucelli) wrote :

You're right, B Clausius. Anyway, over here, the on_gst_player_eos callback is not feeding the method with 4 parameters, but with 3. I defined 0 to the "early" parameter in order to it works. Works like a charm, congratulations.

Revision history for this message
Eduardo Mucelli Rezende Oliveira (eduardo-mucelli) wrote :

Thanks for your efforts. I will change the "early" parameter as I commented and pack it as version 0.0.6.

review: Approve
Revision history for this message
B. Clausius (barcc) wrote :

There is a bug in my code, if the song is the last song in the list, it does not work. To do a "do_previous" there must have been a next song of course. I will look at it.

review: Needs Fixing
16. By B. Clausius

Activate repeat when activating repeat-one to ensure that a next song is available even for the last song in a list.

Revision history for this message
Eduardo Mucelli Rezende Oliveira (eduardo-mucelli) wrote :

Nice workaround.

Revision history for this message
Eduardo Mucelli Rezende Oliveira (eduardo-mucelli) wrote :

I will pack it in the 0.0.6 and generate a Stable version with only the basic function.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file '__init__.py'
2--- __init__.py 2010-10-27 20:49:54 +0000
3+++ __init__.py 2010-11-03 18:11:53 +0000
4@@ -44,6 +44,10 @@
5 </ui>
6 """
7
8+ONE_SONG_STATE_NORMAL = 0
9+ONE_SONG_STATE_EOS = 1
10+ONE_SONG_STATE_REPEAT = 2
11+
12 class RepeatOneSong (rb.Plugin):
13
14 def switch_repeat_status(self, control):
15@@ -51,6 +55,20 @@
16 # mas a boa pratica eh colocar as acoes em um grupo e busca-la com o get_action
17 action = self.action_group.get_action('RepeatOne') # seleciona a acao de repetir a musica atual
18 self.repeat = action.get_active() # indica se o botao esta marcado, ou desmarcado
19+ player = self.shell.props.shell_player
20+ if self.repeat:
21+ shuffle,self.repeat_all = player.get_playback_state()
22+ player.set_playback_state(shuffle, 1)
23+ else:
24+ shuffle,repeat_all = player.get_playback_state()
25+ player.set_playback_state(shuffle, self.repeat_all)
26+
27+ def switch_repeat_all_status(self, action):
28+ if not action.get_active() and self.repeat:
29+ self.repeat = False
30+ self.repeat_all = 0
31+ action_repeat_one = self.action_group.get_action('RepeatOne')
32+ action_repeat_one.set_active(False)
33
34 def load_local_language(self):
35 """Carrega a traducao para o idioma local caso exista. Caso contrario, o ingles sera usado"""
36@@ -91,6 +109,9 @@
37 manager.insert_action_group(self.action_group, 0)
38 self.uid = manager.add_ui_from_string(ui_str) # mescla xml definido com o do RB
39 manager.ensure_update()
40+
41+ action = manager.get_action('/ToolBar/Repeat')
42+ action.connect("activate", self.switch_repeat_all_status)
43
44 def __init__(self):
45 rb.Plugin.__init__(self)
46@@ -101,17 +122,23 @@
47 self.duration = None
48 self.repeat = False
49
50- self.number_of_repetitions = self.get_gconf_number_of_repetitions() # inicializa com o valor do campo
51+ self.number_of_repetitions = 0
52+ self.one_song_state = ONE_SONG_STATE_NORMAL
53
54 self.load_local_language()
55 self.load_icon() # necessary load icon first ...
56 self.generate_ui() # ... because it is used here
57
58 player = self.shell.props.shell_player
59- player.connect('elapsed-changed', self.on_elapsed_change) # receives a signal every single second that the song is being played
60 player.connect('playing-song-changed', self.on_song_change) # mudou a musica. o player.do_next() invoca o on_elapsed_changed
61+ player.props.player.connect('eos', self.on_gst_player_eos)
62
63 def deactivate(self, shell): # when unloading applet
64+ if self.repeat:
65+ player = self.shell.props.shell_player
66+ shuffle,repeat_all = player.get_playback_state()
67+ player.set_playback_state(shuffle, self.repeat_all)
68+
69 for attr in (self.db, self.shell, self.duration, self.repeat): # delete global attributes if possible
70 if attr:
71 del attr
72@@ -120,36 +147,27 @@
73 manager.remove_action_group(self.action_group)
74 manager.ensure_update()
75
76- def on_elapsed_change(self, player, time):
77- if (self.repeat): # se o botao de repetir foi acionado, ver switch_repeat_status
78- duration = player.get_playing_song_duration() # the current song duration
79- if (duration > 0): # in the exact moment the first song is played, the duration is -1
80- if (time >= duration - 1): # one second before the song finishes
81- if (self.number_of_repetitions > 0): # existem repeticoes para serem feitas
82- self.increment_play_count(player.get_playing_entry()) # incrementa o contador do numero de vezes que a musica foi tocada
83- player.set_playing_time(0) # restart the song
84- self.number_of_repetitions -= 1 # atualiza o numero da repeticao atual
85- elif (self.number_of_repetitions == 0): # acabaram-se as repeticoes
86- player.do_next() # vai invocar o on_song_changed
87- else: # == -1 repeticoes infinitas
88- self.increment_play_count(player.get_playing_entry())
89- player.set_playing_time(0) # restart the song
90-
91- def increment_play_count(self, song):
92- current_count = self.db.entry_get (song, rhythmdb.PROP_PLAY_COUNT)
93- self.db.set (song, rhythmdb.PROP_PLAY_COUNT, current_count + 1)
94-
95+ def on_gst_player_eos(self, gst_player, stream_data, early):
96+ if self.number_of_repetitions != 0 and self.repeat:
97+ self.one_song_state = ONE_SONG_STATE_EOS
98+ if self.number_of_repetitions > 0:
99+ self.number_of_repetitions -= 1
100+
101 def on_song_change(self, player, time): # quando mudar a musica ...
102- self.number_of_repetitions = self.get_gconf_number_of_repetitions() # ... reinicia o numero de repeticoes
103-
104+ if self.one_song_state == ONE_SONG_STATE_EOS:
105+ self.one_song_state = ONE_SONG_STATE_REPEAT
106+ player.do_previous()
107+ elif self.one_song_state == ONE_SONG_STATE_REPEAT:
108+ self.one_song_state = ONE_SONG_STATE_NORMAL
109+ else:
110+ self.number_of_repetitions = self.get_gconf_number_of_repetitions()
111+
112 def get_gconf_number_of_repetitions(self): # le do arquivo /apps/rhythmbox/plugins/repeat-one-song/gconf.xml ...
113 client = gconf.client_get_default()
114 try:
115 key_value = client.get_value(gconf_keys['number_of_repetitions']) # ... o valor do atributo number_of_repetitions
116- print ("[+] The gconf attribute number_of_repetitions was read successfully")
117 except StandardError:
118 key_value = -1
119- print ("[-] Problem to read gconf attribute number_of_repetitions")
120 return key_value
121
122 def create_configure_dialog(self, dialog=None):

Subscribers

People subscribed via source and target branches