Bug 485626 - f-spot screensaver repeats itself
f-spot screensaver repeats itself
Status: CLOSED NOTABUG
Product: Fedora
Classification: Fedora
Component: f-spot (Show other bugs)
10
All Linux
low Severity medium
: ---
: ---
Assigned To: Christian Krause
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-02-15 09:44 EST by Sjoerd Mullender
Modified: 2009-07-17 19:28 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-07-17 19:28:33 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)


External Trackers
Tracker ID Priority Status Summary Last Updated
Red Hat Bugzilla 512491 None None None Never

  None (edit)
Description Sjoerd Mullender 2009-02-15 09:44:32 EST
Description of problem:
I use f-spot as my screensaver.  I have some 16000 photos from which f-spot can choose, but I often see the same photos repeated during a screensaver session.  With 16000 photos to choose from, that should be impossible.

Looking at the code, I see that the algorithm is:
- get a list of photos to display (in src/Core.cs)
- call RandomSort to order them in a random manner (implemented in src/Photo.cs)
- go through the randomized list, displaying photos

The problem, I think, is with the implementation of RandomSort.  It seems to use a standard Sort method and gives it a comparison function that returns a random number in the range -5..5 each time it is called.  It seems to me this is exceedingly bad Computer Science.

First of all, the function should check whether the arguments are the same and if so, return 0, and if they are not equal, it should return a value != 0.  Also, it should implement a full order correctly (if it returns that A<B and B<C, it should also return that A<C).  None of this is happening.

Maybe related is that quite often f-spot stops displaying photos, produces a totally black screen and then resumes (or starts over).  Perhaps because it crashed and is then restarted?  If there are multiple copies of the same photo in the list of to-be-displayed photos, it can also happen there are photos missing in the list which might cause a crash.

Version-Release number of selected component (if applicable):
f-spot-0.5.0.3-2.fc10.x86_64

How reproducible:
It happens often when running for a prolonged time.

Steps to Reproduce:
1.Use f-spot as screensaver.
2.
3.
  
Actual results:
Photos get repeated during a screensaver session.

Expected results:
No repetition of photos until all photos have been displayed.

Additional info:
Comment 1 Sjoerd Mullender 2009-03-28 10:22:13 EDT
Although there are probably syntax and other errors in this code, this patch shows a way in which an array can be shuffled.  The idea is to select a random element from the list and store it at the end (exchanging with the element that was there), and then doing the same with the list shortened by one.  Repeat until done.
I'm assuming random.Next(0,i) returns a randomly chosen integer in the range 0..i inclusive.

--- Core.cs~    2008-07-15 12:06:37.000000000 +0200
+++ Core.cs     2009-03-24 16:28:12.000000000 +0100
@@ -174,7 +174,15 @@
                                window = new XScreenSaverSlide ();
                                SetStyle (window);
                                if (photos.Length > 0) {
-                                       Array.Sort (photos, new Photo.RandomSort
 ());
+                                       Random random = new Random ();
+                                       for (int i = photos.Length - 1; i > 0; i
--) {
+                                               int r = random.Next(0, i);
+                                               if (i != r) {
+                                                       Photo tmp = photos[r];
+                                                       photos[r] = photos[i];
+                                                       photos[i] = tmp;
+                                               }
+                                       }
                                        
                                        Gdk.Pixbuf black = new Gdk.Pixbuf (Gdk.C
olorspace.Rgb, false, 8, 1, 1);
                                        black.Fill (0x00000000);
Comment 2 Christian Krause 2009-07-17 19:28:33 EDT
Thank you very much for the bug report.

(In reply to comment #0)
> Description of problem:
> I use f-spot as my screensaver.  I have some 16000 photos from which f-spot can
> choose, but I often see the same photos repeated during a screensaver session. 
> With 16000 photos to choose from, that should be impossible.

I've seen the following:

A random order was created. All photos were displayed a couple of times using the same order. At some point the photos were permutated differently - just before this happens I've seen a black screen. But each permutation displayed all photos.

> Looking at the code, I see that the algorithm is:
> - get a list of photos to display (in src/Core.cs)
> - call RandomSort to order them in a random manner (implemented in
> src/Photo.cs)
> - go through the randomized list, displaying photos
> 
> The problem, I think, is with the implementation of RandomSort.  It seems to
> use a standard Sort method and gives it a comparison function that returns a
> random number in the range -5..5 each time it is called.  It seems to me this
> is exceedingly bad Computer Science.

No necessarly. Internally a quicksort is done using the comparison function. Since the qsort implementation does never copy elements at specific positions but only swaps items, I don't think it is possible to get duplicated entries in the Array or that entries are missing.

> First of all, the function should check whether the arguments are the same and
> if so, return 0, and if they are not equal, it should return a value != 0. 
> Also, it should implement a full order correctly (if it returns that A<B and
> B<C, it should also return that A<C).  None of this is happening.

These restrictions only apply if the compare function should really implement a stable sorting algorithm. ;-)
As far as I understand the qsort algorithm, the worst thing what could happen is that the sorting would not be good. Or in our case that the permutation would not be good. But doubled / missing photos should not be possible.

To be sure I've verified this with a small test program. ;-)

> Maybe related is that quite often f-spot stops displaying photos, produces a
> totally black screen and then resumes (or starts over).  Perhaps because it
> crashed and is then restarted?  If there are multiple copies of the same photo
> in the list of to-be-displayed photos, it can also happen there are photos
> missing in the list which might cause a crash.

I've only tested it with much less photos. Most likely the following happens on your side:

- f-spot starts as screensaver, permutates all photos, starts to display them
- before it can finish displaying all photos it is stopped and gets restarted automatically again and f-spot may display some of the previously seen photos again

So the remaining question is: Why is f-spot restarted when acting as screensaver? 

I've started gnome-screensaver manually with --no-daemon --debug and indeed, gnome-screensaver cycles the jobs every 10 minutes (since there _may_ be another screensaver selected as well).


[gs_manager_cycle] gs-manager.c:640 (00:41:32):  cycling jobs
[gs_job_stop] gs-job.c:483 (00:41:32):   stopping job
[gs_job_died] gs-job.c:128 (00:41:32):   Waiting on process 5729
[gs_job_died] gs-job.c:142 (00:41:32):   Job died
[gs_job_set_command] gs-job.c:193 (00:41:32):    Setting command for job: 'f-spot-screensaver'
[manager_maybe_start_job_for_window] gs-manager.c:210 (00:41:32):        Starting job for window

So I doubt there is anything which can be fixed in f-spot. I'll file a separate bug report for the gnome-screensaver component. You may add yourself to the CC list of the new bug if you like.

As a workaround you can set the "cycle_delay" of the gnome-screesaver's gconf options (/apps/gnome-screensaver/cycle_delay) to a very large value. This value defines the after how many minutes gnome-screensaver cycles to another screensaver (even if there is only one).

Note You need to log in before you can comment on or make changes to this bug.