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 588377 - Don't block main thread in _seek()
Don't block main thread in _seek()
Status: RESOLVED FIXED
Product: totem
Classification: Core
Component: GStreamer backend
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Maintainer alias for GStreamer component of Totem
Maintainer alias for GStreamer component of Totem
Depends on:
Blocks:
 
 
Reported: 2009-07-12 15:54 UTC by Johannes Langlotz
Modified: 2010-08-05 09:35 UTC
See Also:
GNOME target: ---
GNOME version: 2.27/2.28


Attachments
Don't block main thread in _seek() (6.76 KB, patch)
2010-03-19 12:51 UTC, Bastien Nocera
none Details | Review
Don't block main thread in _seek() (6.79 KB, patch)
2010-03-19 18:40 UTC, Bastien Nocera
committed Details | Review

Description Johannes Langlotz 2009-07-12 15:54:37 UTC
Open a file with totem. Move the slider slowly while playing from the left to the right. While moving the slider you will see that the slider stops repeatedly for some ms. During these interruptions totem changes the position of the stream. I think the slider should have its own thread so that the movement is not interrupted by other code.

In Rythmbox there are no such interruptions. Rhythmbox will not change the position of the stream before you stop moving the slider.

This is not really a bug. It is rather not a nice behaviour.


This bug was originally reported at https://bugs.launchpad.net/ubuntu/+source/totem/+bug/323980
Comment 1 Bastien Nocera 2009-09-03 13:01:39 UTC
(In reply to comment #0)
> In Rythmbox there are no such interruptions. Rhythmbox will not change the
> position of the stream before you stop moving the slider.

Except that Rhythmbox doesn't play videos, and you do want scrubbing for videos.

It's most likely a problem with the source medium, or the computer being too slow.
Comment 2 Johannes Langlotz 2009-09-06 12:25:33 UTC
Totem also shows this behaviour while playing audio files.

The difference is that Rhythmbox will not change the
position of the stream before you stop moving the slider.
Comment 3 Johannes Langlotz 2009-09-06 12:32:38 UTC
I don't believe that this has something to do with the computer's speed. It's a Core Duo 2.2 GHz with 2 GB RAM and a 7200 rpm HD. Furthermore I tested it with the same audio file on the same computer and Rhythmbox doesn't have these interruptions. Perhaps Rhythmbox has its own thread for the GUI so that GUI is not freezed while waiting for the source medium.
Comment 4 Bastien Nocera 2010-03-19 12:50:50 UTC
Sigh. Rhythmbox DOES NOT DO SCRUBBING.

The problem is that when seeking we do a blocking gst_element_get_state(), which would make the seeking feel stuttery.
Comment 5 Bastien Nocera 2010-03-19 12:51:15 UTC
Created attachment 156552 [details] [review]
Don't block main thread in _seek()

Instead keep hold of the last seek request and apply it
when we've been told that the previous one finished, or
apply it straight away if the previous seek took longer than
1/10th of a second.
Comment 6 Bastien Nocera 2010-03-19 12:52:06 UTC
Johannes, could you please test the attached patch? It's against Totem master.
Comment 7 Sebastian Dröge (slomo) 2010-03-19 13:40:46 UTC
(In reply to comment #5)
> Created an attachment (id=156552) [details] [review]
> Don't block main thread in _seek()
> 
> Instead keep hold of the last seek request and apply it
> when we've been told that the previous one finished, or
> apply it straight away if the previous seek took longer than
> 1/10th of a second.

The delayed seek (after the previous one has finished) will be overwritten immediately by a new seek. Instead any new seeks should wait again on the delayed seek to finish or the timeout to happen.

Other than that the patch looks good
Comment 8 Bastien Nocera 2010-03-19 18:39:34 UTC
See also bug 613351
Comment 9 Bastien Nocera 2010-03-19 18:40:53 UTC
Created attachment 156575 [details] [review]
Don't block main thread in _seek()

Instead keep hold of the last seek request and apply it
when we've been told that the previous one finished, or
apply it straight away if the previous seek took longer than
1/10th of a second.
Comment 10 Bastien Nocera 2010-03-19 18:41:39 UTC
New patch fixes the bug Sebastian mentioned in comment 7.
Comment 11 Sebastian Dröge (slomo) 2010-03-20 07:10:01 UTC
Looks good to me :)
Comment 12 Sebastian Dröge (slomo) 2010-04-24 17:57:29 UTC
Any news on this for the 2.31 branch/master? ;)
Comment 13 Bastien Nocera 2010-04-24 18:04:02 UTC
master is still 2.30.x

Will probably branch after the next 2.30.x release.
Comment 14 Bastien Nocera 2010-04-27 11:45:54 UTC
Attachment 156575 [details] pushed as e55b44c - Don't block main thread in _seek()
Comment 15 Bastien Nocera 2010-08-05 09:35:53 UTC
BTW, it seems I forgot to push this onto master, and it only lived in the gnome-2-30 branch, which made merging a bit tedious.

Testing in the gnome-2-32 branch and/or master would be appreciated (to see if I didn't break anything with Alexander's "accurate" seeking patch).