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 705159 - Only retrieve metadata once at start-up
Only retrieve metadata once at start-up
Status: RESOLVED FIXED
Product: sound-juicer
Classification: Applications
Component: general
git master
Other All
: Normal normal
: ---
Assigned To: Sound Juicer Maintainers
Sound Juicer Maintainers
Depends on:
Blocks: 734603
 
 
Reported: 2013-07-30 17:37 UTC by Phillip Wood
Modified: 2014-08-15 13:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Only retrieve metadata once at start-up (1.33 KB, patch)
2013-07-30 17:37 UTC, Phillip Wood
needs-work Details | Review
Add utility functions for boolean actions. (3.45 KB, patch)
2014-08-07 11:03 UTC, Phillip Wood
committed Details | Review
Give re-read action state (2.01 KB, patch)
2014-08-07 11:03 UTC, Phillip Wood
committed Details | Review
Don't retrieve metadata twice at start up (1.20 KB, patch)
2014-08-07 11:03 UTC, Phillip Wood
committed Details | Review

Description Phillip Wood 2013-07-30 17:37:39 UTC
reread_cd was getting called twice at start-up if there was a disc in
the drive, once from set_device and then again from
media_added_cb. Fix this by removing the call from set_device.
Comment 1 Phillip Wood 2013-07-30 17:37:41 UTC
Created attachment 250492 [details] [review]
Only retrieve metadata once at start-up
Comment 2 Christophe Fergeau 2013-08-08 15:11:00 UTC
Review of attachment 250492 [details] [review]:

I would not remove the call from set_device actually, as when the cd reader that is being used is changed in the preferences, then set_device() is called from device_changed_cb(), and in that case we need to reread the CD.
Comment 3 Christophe Fergeau 2013-08-08 15:11:28 UTC
Review of attachment 250492 [details] [review]:

I would not remove the call from set_device actually, as when the cd reader that is being used is changed in the preferences, then set_device() is called from device_changed_cb(), and in that case we need to reread the CD.
Comment 4 Phillip Wood 2013-08-16 09:15:51 UTC
I'm weary of removing it and breaking something but set_device calls set_drive_from_device which calls

g_signal_connect (monitor, "medium-added", G_CALLBACK (media_added_cb), NULL);
g_signal_connect (monitor, "medium-removed", G_CALLBACK (media_removed_cb), NULL);

The 'medium-added' signal triggers the second call to reread_cd at start up so wont it do the same thing when the drive is changed? If so it would still be safe to remove the call to reread_cd from set_device as the reread would still be triggered. Unfortunately I don't have a second CD drive to test it.
Comment 5 Christophe Fergeau 2013-08-19 12:13:05 UTC
For some reason, I remember checking that medium-added code and deciding the monitoring was per CD reader, not global. Looking again at that code, it's indeed global, not per CD reader, so this should work out. I should be able to test it with USB readers at work, I'll try to do that this week.
Comment 6 Christophe Fergeau 2013-08-22 13:59:22 UTC
Now I remember why I ruled out media monitoring to do the right thing. The setup I had in mind was 2 CD readers, each already containing a CD. When you change the CD reader in the preferences, the UI does not get updated after your changes as this refresh is done through reread_cd. I've tested this with actual readers.
Comment 7 Phillip Wood 2014-08-07 11:03:19 UTC
Created attachment 282769 [details] [review]
Add utility functions for boolean actions.

Avoid dealing with GVariants in the main code by adding getter and
setter.

This raises the required GLib version to 2.38 for
g_action_parse_detailed_name().
Comment 8 Phillip Wood 2014-08-07 11:03:24 UTC
Created attachment 282770 [details] [review]
Give re-read action state

Make re-read a boolean action so we can tell if a re-read is currently
in progress.
Comment 9 Phillip Wood 2014-08-07 11:03:28 UTC
Created attachment 282771 [details] [review]
Don't retrieve metadata twice at start up

When sound-juicer starts up it calls reread_cb() twice, once from
set_device() and once from media_added_cb(). Add a check to
media_added_cb() to see if the metadata is already being retrieved
before calling reread_cd(). If the user changes the device while
retrieving metadata we still start a new request for the new drive. It
would be nice to make metadata retrieval cancellable in the future.
Comment 10 Phillip Wood 2014-08-15 13:31:42 UTC
Attachment 282769 [details] pushed as 8dd1c00 - Add utility functions for boolean actions.
Attachment 282770 [details] pushed as f9a94b7 - Give re-read action state
Attachment 282771 [details] pushed as 8654a16 - Don't retrieve metadata twice at start up