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 727838 - Some grilo fixes
Some grilo fixes
Status: RESOLVED FIXED
Product: rhythmbox
Classification: Other
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: RhythmBox Maintainers
RhythmBox Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-04-08 15:04 UTC by Bastien Nocera
Modified: 2014-05-05 12:26 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
grilo: Unregister ignored sources (1.91 KB, patch)
2014-04-08 15:04 UTC, Bastien Nocera
committed Details | Review
grilo: Ignore video and image sources (1.58 KB, patch)
2014-04-08 15:05 UTC, Bastien Nocera
committed Details | Review
this might help (4.23 KB, patch)
2014-04-09 13:07 UTC, Jonathan Matthew
none Details | Review
somewhat better (6.18 KB, patch)
2014-04-11 12:24 UTC, Jonathan Matthew
committed Details | Review

Description Bastien Nocera 2014-04-08 15:04:52 UTC
There's still a problem with rhythmbox not supporting media items at the
root of the source, such as with the "Radio France" source in the latest
grilo-plugins.
Comment 1 Bastien Nocera 2014-04-08 15:04:55 UTC
Created attachment 273809 [details] [review]
grilo: Unregister ignored sources

Instead of letting them running and using resources, unregister
the sources that we ignore.
Comment 2 Bastien Nocera 2014-04-08 15:05:01 UTC
Created attachment 273810 [details] [review]
grilo: Ignore video and image sources

Instead of hard-coding many of the plugins, simply ignore sources
that don't support audio types.
Comment 3 Jonathan Matthew 2014-04-08 22:49:39 UTC
Review of attachment 273810 [details] [review]:

ok
Comment 4 Jonathan Matthew 2014-04-08 22:49:45 UTC
Review of attachment 273809 [details] [review]:

ok
Comment 5 Jonathan Matthew 2014-04-09 13:07:55 UTC
Created attachment 273890 [details] [review]
this might help

I can't test this at the moment (I don't have lua 5.2, so the lua-factory thing doesn't build, and I don't seem to have any other sources that put media in the root), but this might do roughly the right thing.

It's probably a good idea to add something to the browser pane representing the root, since otherwise if you select a child container there's no way to get back. I might look at that later on.
Comment 6 Bastien Nocera 2014-04-09 16:32:58 UTC
(In reply to comment #5)
> Created an attachment (id=273890) [details] [review]
> this might help
> 
> I can't test this at the moment (I don't have lua 5.2, so the lua-factory thing
> doesn't build, and I don't seem to have any other sources that put media in the
> root), but this might do roughly the right thing.
> 
> It's probably a good idea to add something to the browser pane representing the
> root, since otherwise if you select a child container there's no way to get
> back. I might look at that later on.

Much better, I actually see the items in the list though, as you mention, it would be good to hide the "browse" pane if there are no children to that list.

The other problem is that the "album art" is wrong except for the first played radio station, this is likely due to the radio stations lacking both artist and album values (they only have titles).

If that's any help, Fedora 20 has lua 5.2 builtin. My attempts at making it work with lua 5.1 failed as lua 5.1 is lacking a way to load libraries from the C api (we preload the grilo API for all the scripts).
Comment 7 Jonathan Matthew 2014-04-11 12:24:23 UTC
Created attachment 274087 [details] [review]
somewhat better

I've tried this out with a few things and seems okay. Rather than hiding the browser, I'm adding an item representing the root if we find any media in there in the initial browse. I don't have any sources that have media in the root and also a container hierarchy, but I think if that happens it should work.
Comment 8 Bastien Nocera 2014-04-11 13:08:10 UTC
That works well, thanks!
Comment 9 Jonathan Matthew 2014-04-11 14:16:52 UTC
Pushed as commit 75894d6; thanks for testing.
Comment 10 Jonathan Matthew 2014-05-05 12:26:31 UTC
other things pushed, closing