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

[B. Clausius]

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.

[Eduardo Mucelli Rezende Oliveira (eduardo-mucelli)]

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

[lp:~barcc/repeat-one-song/one-second-issue]

14. By B. Clausius on 2010-10-28

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

15. By B. Clausius on 2010-10-28

Merge from trunk

[B. Clausius (barcc)]

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

[Eduardo Mucelli Rezende Oliveira (eduardo-mucelli)]

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.

[Eduardo Mucelli Rezende Oliveira (eduardo-mucelli)]

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

review: Approve

[B. Clausius (barcc)]

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

[lp:~barcc/repeat-one-song/one-second-issue]

16. By B. Clausius on 2010-11-03

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

[Eduardo Mucelli Rezende Oliveira (eduardo-mucelli)]

Nice workaround.

[Eduardo Mucelli Rezende Oliveira (eduardo-mucelli)]

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

review: Approve

=== modified file '__init__.py'
--- __init__.py	2010-10-27 20:49:54 +0000
+++ __init__.py	2010-11-03 18:11:53 +0000
@@ -44,6 +44,10 @@
         </ui>
         """
 
+ONE_SONG_STATE_NORMAL = 0
+ONE_SONG_STATE_EOS = 1
+ONE_SONG_STATE_REPEAT = 2
+
 class RepeatOneSong (rb.Plugin):
 
     def switch_repeat_status(self, control):
@@ -51,6 +55,20 @@
         # mas a boa pratica eh colocar as acoes em um grupo e busca-la com o get_action
         action = self.action_group.get_action('RepeatOne')                              # seleciona a acao de repetir a musica atual
         self.repeat = action.get_active()                                               # indica se o botao esta marcado, ou desmarcado
+        player = self.shell.props.shell_player
+        if self.repeat:
+            shuffle,self.repeat_all = player.get_playback_state()
+            player.set_playback_state(shuffle, 1)
+        else:
+            shuffle,repeat_all = player.get_playback_state()
+            player.set_playback_state(shuffle, self.repeat_all)
+            
+    def switch_repeat_all_status(self, action):
+        if not action.get_active() and self.repeat:
+            self.repeat = False
+            self.repeat_all = 0
+            action_repeat_one = self.action_group.get_action('RepeatOne')
+            action_repeat_one.set_active(False)
 
     def load_local_language(self):
         """Carrega a traducao para o idioma local caso exista. Caso contrario, o ingles sera usado"""
@@ -91,6 +109,9 @@
         manager.insert_action_group(self.action_group, 0)
         self.uid = manager.add_ui_from_string(ui_str)                                                           # mescla xml definido com o do RB
         manager.ensure_update()
+        
+        action = manager.get_action('/ToolBar/Repeat')
+        action.connect("activate", self.switch_repeat_all_status)
 
     def __init__(self):
         rb.Plugin.__init__(self)
@@ -101,17 +122,23 @@
         self.duration = None
         self.repeat = False
 
-        self.number_of_repetitions = self.get_gconf_number_of_repetitions()                                    # inicializa com o valor do campo
+        self.number_of_repetitions = 0
+        self.one_song_state = ONE_SONG_STATE_NORMAL
           
         self.load_local_language()
         self.load_icon()                                                                                        # necessary load icon first ...
         self.generate_ui()                                                                                      # ... because it is used here 
 
         player = self.shell.props.shell_player
-        player.connect('elapsed-changed', self.on_elapsed_change)           # receives a signal every single second that the song is being played
         player.connect('playing-song-changed', self.on_song_change)         # mudou a musica. o player.do_next() invoca o on_elapsed_changed
+        player.props.player.connect('eos', self.on_gst_player_eos)
 
     def deactivate(self, shell):                                            # when unloading applet
+        if self.repeat:
+            player = self.shell.props.shell_player
+            shuffle,repeat_all = player.get_playback_state()
+            player.set_playback_state(shuffle, self.repeat_all)
+        
         for attr in (self.db, self.shell, self.duration, self.repeat):      # delete global attributes if possible
             if attr:
                 del attr
@@ -120,36 +147,27 @@
         manager.remove_action_group(self.action_group)
         manager.ensure_update()
 
-    def on_elapsed_change(self, player, time):
-        if (self.repeat):                                                   # se o botao de repetir foi acionado, ver switch_repeat_status
-            duration = player.get_playing_song_duration()					# the current song duration
-            if (duration > 0):												# in the exact moment the first song is played, the duration is -1
-                if (time >= duration - 1):									# one second before the song finishes
-                    if (self.number_of_repetitions > 0):                    # existem repeticoes para serem feitas
-                        self.increment_play_count(player.get_playing_entry()) # incrementa o contador do numero de vezes que a musica foi tocada
-                        player.set_playing_time(0)						    # restart the song
-                        self.number_of_repetitions -= 1                     # atualiza o numero da repeticao atual
-                    elif (self.number_of_repetitions == 0):                 # acabaram-se as repeticoes
-                        player.do_next()                                    # vai invocar o on_song_changed
-                    else:                                                   # == -1 repeticoes infinitas
-                       self.increment_play_count(player.get_playing_entry())
-                       player.set_playing_time(0)						    # restart the song
-
-    def increment_play_count(self, song):
-        current_count = self.db.entry_get (song, rhythmdb.PROP_PLAY_COUNT)
-        self.db.set (song, rhythmdb.PROP_PLAY_COUNT, current_count + 1)
-
+    def on_gst_player_eos(self, gst_player, stream_data, early):
+        if self.number_of_repetitions != 0 and self.repeat:
+            self.one_song_state = ONE_SONG_STATE_EOS
+            if self.number_of_repetitions > 0:
+                self.number_of_repetitions -= 1
+        
     def on_song_change(self, player, time):                                 # quando mudar a musica ...
-        self.number_of_repetitions = self.get_gconf_number_of_repetitions() # ... reinicia o numero de repeticoes
-
+        if self.one_song_state == ONE_SONG_STATE_EOS:
+            self.one_song_state = ONE_SONG_STATE_REPEAT
+            player.do_previous()
+        elif self.one_song_state == ONE_SONG_STATE_REPEAT:
+            self.one_song_state = ONE_SONG_STATE_NORMAL
+        else:
+            self.number_of_repetitions = self.get_gconf_number_of_repetitions()
+        
     def get_gconf_number_of_repetitions(self):                              # le do arquivo /apps/rhythmbox/plugins/repeat-one-song/gconf.xml ...
         client = gconf.client_get_default()
         try:
             key_value = client.get_value(gconf_keys['number_of_repetitions'])   # ... o valor do atributo number_of_repetitions
-            print ("[+] The gconf attribute number_of_repetitions was read successfully")
         except StandardError:
             key_value = -1
-            print ("[-] Problem to read gconf attribute number_of_repetitions")
         return key_value
 
     def create_configure_dialog(self, dialog=None):