GNOME Bugzilla – Bug 776668
double click on seek bar pauses the song
Last modified: 2017-04-13 10:18:53 UTC
double clicking on the seek/progress bar pauses the playing song. I don't know if this is a feature or a bug. For me it seems more like a bug.
Created attachment 343553 [details] [review] Prevent pause on clicking progress scale multiple Progress scale's button press event was getting called multiple times before a single button release event was called. This messed up the last state of the player, and caused it to pause. This might be a bug with how gtk/gdk handles events. Introduced a boolean to sync those event. I think this surely is an inconvenience. Because many times user will press progress scale multiple times while moving it, and if the track gets paused, then user will have to play it again.
Created attachment 344982 [details] [review] player.py : Prevent pause when multiple click on progress Make sure the event is just a single click before pausing the song. 'button-press-event' gets called multiple times before a 'button-release-event' is called if the user has double/triple clicked.
Review of attachment 344982 [details] [review]: This solution works and is much cleaner than the first patch: kudos. However, I was actually looking at why we even have a callback for on_button_press event, apparently it is so the songs is paused otherwise the button will reset to it's state in the playing position (probably b/c of the progressbar being updated). If I look at the same thing in eg. totem then we can just seek through the song and hear the sound at that point in time, the music keeps playing. I think this is much more desirable behaviour than pausing on seek. I think we need to investigate & go that route.
*** Bug 776667 has been marked as a duplicate of this bug. ***
Created attachment 345954 [details] [review] Prevent pause when seeking Removed lastState and prevent pause on seekbar button press event.
Created attachment 345955 [details] [review] Simultaneously play music when seeking Connect to 'change-value' of the progress bar.
Created attachment 345962 [details] [review] player.py : Simultaneously play music when seeking Patch over Prevent pause when seeking. It instantly plays when seeking. Does not use time_out of button-press button-release events of progress bar.
Created attachment 346000 [details] [review] player.py: Simultaneously play music when seeking Connect to 'change-value' of the progress bar. Along with smooth simultaneous playing, this patch also adds a minimum seek such that seeking with infinitesimal amounts does not stutter the song.
Review of attachment 345954 [details] [review]: lgtm
Review of attachment 346000 [details] [review]: I'm not 100% sure the code on removing the mini-stutter is really needed and not overcomplicating things. Other than that and the comments it's looking good. ::: gnomemusic/player.py @@ +81,3 @@ nextTrack = None timeout = None + seek_timeout = None _seek_timeout Also I don't think this should be a class variable, but should be just declared in __init__. @@ +827,3 @@ + self._old_progress_scale_value = 0.0 + + def _on_progress_scale_seek(self, scale, scroll_type, value): I think a docstring explaining what is going on in here is useful. @@ +834,3 @@ + self._old_progress_scale_value = self.progressScale.get_value() + + def on_progress_scale_change_value(): Although this is internal, I don't think it's a good idea to name it the same as a main class function.
Created attachment 347178 [details] [review] player.py : Simultaneously play music when seeking Connect to 'change-value' of the progress bar. Only after a seek has become stable, we play the song from its location.
Review of attachment 347178 [details] [review]: lgtm the only minor thing is that the calling arguments for functions aren't in the docstrings (see other docstrings in the project)..
Nice patch, thanks. Applied.
Created attachment 348787 [details] [review] player: Fix stuttering when seeking Old progress scale value was not updating causing minor stuttering. Update its value when a seek is finished. Marinus Schraal: I missed a line in the previous patch :x
Comment on attachment 348787 [details] [review] player: Fix stuttering when seeking My bad, missed this earlier. Attachment 348787 [details] pushed as 14f0175 - player: Fix stuttering when seeking