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 706027 - grilo: don't hammer tracker with a query for every album
grilo: don't hammer tracker with a query for every album
Status: RESOLVED FIXED
Product: gnome-music
Classification: Applications
Component: general
unspecified
Other All
: High normal
: 3.10
Assigned To: Giovanni Campagna
gnome-music-maint
Depends on:
Blocks:
 
 
Reported: 2013-08-14 21:22 UTC by Giovanni Campagna
Modified: 2014-04-16 13:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
grilo: don't hammer tracker with a query for every album (5.79 KB, patch)
2013-08-14 21:22 UTC, Giovanni Campagna
none Details | Review
grilo: don't hammer tracker with a query for every album (5.78 KB, patch)
2013-08-15 19:38 UTC, Giovanni Campagna
needs-work Details | Review

Description Giovanni Campagna 2013-08-14 21:22:21 UTC
If tracker didn't give us a thumbnail the first time, it's because
it doesn't have one, so there is no point in querying again (especially
if we're not really querying for the thumbnail, but only for
artist and album fields, which we already know!). Instead, what
we want is to resolve the item using the other sources (GRL_RESOLVE_FULL),
in particular grl-local-metadata and grl-lastfm-albumart.
Comment 1 Giovanni Campagna 2013-08-14 21:22:24 UTC
Created attachment 251657 [details] [review]
grilo: don't hammer tracker with a query for every album
Comment 2 Giovanni Campagna 2013-08-15 19:38:03 UTC
Created attachment 251767 [details] [review]
grilo: don't hammer tracker with a query for every album

If tracker didn't give us a thumbnail the first time, it's because
it doesn't have one, so there is no point in querying again (especially
if we're not really querying for the thumbnail, but only for
artist and album fields, which we already know!). Instead, what
we want is to resolve the item using the other sources (GRL_RESOLVE_FULL),
in particular grl-local-metadata and grl-lastfm-albumart.
Comment 3 Vadim Rutkovsky 2013-08-18 08:47:07 UTC
Review of attachment 251767 [details] [review]:

Didn't test this, but looks good (and its a better solution then existing one)
Comment 4 Arnel Borja 2013-08-18 10:18:53 UTC
Seif is currently working in album art cache though, so I didn't commit it.
Comment 5 Arnel Borja 2013-08-18 16:48:53 UTC
Review of attachment 251767 [details] [review]:

When I select an album in the Albums view, a segmentation fault usually happens, otherwise wrong album information is shown. Sometimes when I play a song in the Songs view, a segmentation fault happens after a few seconds.

I think it is a good idea to resolve the album art on a fast query  - the album art in MPRIS is currently broken because the thumbnail uri is not being resolved.
Comment 6 Giovanni Campagna 2013-08-18 16:59:01 UTC
(In reply to comment #5)
> Review of attachment 251767 [details] [review]:
> 
> I think it is a good idea to resolve the album art on a fast query  - the album
> art in MPRIS is currently broken because the thumbnail uri is not being
> resolved.

I'm sorry but I don't follow here - the album art is not part of the fast result set, because, well, resolving it is not fast.
Comment 7 Arnel Borja 2013-08-18 17:12:29 UTC
(In reply to comment #6)
> I'm sorry but I don't follow here - the album art is not part of the fast
> result set, because, well, resolving it is not fast.

In grilo, album art and thumbnail is the same. I saw that you added it to the keys for "fast resolving" (without Grl.ResolutionFlags.FULL). It is grl-local-metadata that computes for the local thumbnail/album art, though it sometimes fails so the lookup code is in gnome-music too.
Comment 8 Giovanni Campagna 2013-08-18 17:18:11 UTC
(In reply to comment #7)
> (In reply to comment #6)
> > I'm sorry but I don't follow here - the album art is not part of the fast
> > result set, because, well, resolving it is not fast.
> 
> In grilo, album art and thumbnail is the same. I saw that you added it to the
> keys for "fast resolving" (without Grl.ResolutionFlags.FULL). It is
> grl-local-metadata that computes for the local thumbnail/album art, though it
> sometimes fails so the lookup code is in gnome-music too.

No, that's not what's happening.
Grl.ResolutionFlags.FULL means "ask other sources in addition to this one". We're not asking grl-local-metadata, we're asking grl-tracker, but with the flags, grilo tries all other sources that claim to resolve a key, if the first source did not find a value for it.
I removed the flag, so grl-local-metadata (which is slow, because it does IO on the disk and to gvfs) is out of the picture for the initial load of the data.

The key is added to the list because grl-tracker sometimes can give a value for it straight away, in which case we can take advantage of the join in the query and avoid more queries later.
Comment 9 Vadim Rutkovsky 2013-08-27 12:17:45 UTC
Review of attachment 251767 [details] [review]:

Agree with Arnel, I usually get:

Warning: g_value_set_object: assertion 'G_IS_OBJECT (v_object)' failed
  item = self._model.get_value(_iter, 5)

or simply None item. No idea how to fix this or why is this happening though
Comment 10 Giovanni Campagna 2013-08-27 12:24:03 UTC
(In reply to comment #9)
> Review of attachment 251767 [details] [review]:
> 
> Agree with Arnel, I usually get:
> 
> Warning: g_value_set_object: assertion 'G_IS_OBJECT (v_object)' failed
>   item = self._model.get_value(_iter, 5)
> 
> or simply None item. No idea how to fix this or why is this happening though

That warning means you're putting NULL for a GObject column in the model, which is not allowed.
Comment 11 Vadim Rutkovsky 2013-08-27 13:28:10 UTC
This is wierd. If I add
  self._model.get_value(_iter, 5)
after self._model.set the whole thing works fine. It seems that we got to 'confirm' that model has really set this value
Comment 12 Vadim Rutkovsky 2014-04-16 13:53:35 UTC
We've finally fixed this in https://git.gnome.org/browse/gnome-music/commit/?id=5f8afde8daa6ca8f4b4ff666d7aebf97e1d66334 and during albumart refactoring.

We're using a strategy similar to the one Giovanni proposed: first we try FAST_ONLY and then fallback to FULL