GNOME Bugzilla – Bug 555823
BBC plugin
Last modified: 2011-04-06 14:20:46 UTC
This is a source plugin for content made available by the BBC, kindly sponsored by the BBC and Canonical. It's still quite basic, but functional. (Note: this has nothing to do with the iPlayer, it mostly just makes content available in totem which is already available in some form or other on the BBC website now, at least for the time being; server-side things are also still work-in-progress, so expect the occasional hiccups and problems with the content in the feed.) I'll post patches against the 2.24 branch here until I think it's ready to go into trunk.
Created attachment 120353 [details] [review] patch against totem 2.24.2
That looks fine to use, but could you please remove all the apt code in there. If anything, we should be using the PackageKit-gnome D-Bus API to ask for things like that. Having apt code in there is out of the question. Feel free to commit to trunk once you've removed the apt/gdbm code
Well, I'm not particularly attached to that code, but it's not a dependency for non-apt systems and only used at runtime if actually supported. The idea is to only show episodes/brands for which decoders could theoretically be installed. I tried to write it in a way that would make it easy to add hooks for rpm or packageKit too. I believe there's no unified system for this kind of thing yet, and I think it's a useful and not very intrusive feature, and the code is encapsulated in a separate and very very small module. In any case, I intend to polish things up a bit more before committing it to trunk. Just posting it here so interested people can easily test it and track progress.
Created attachment 120375 [details] [review] updated patch (fixes logic error which filters out all brands under certain circumstances)
There is a unified thing for this sort of thing using PackageKit, and it's distro neutral and working now. Ubuntu are even shipping the PackageKit apt backend that supports installing codecs in this way. This works from any gstreamer application, and not just totem, and does not rely on static lists of codecs->packages. I really think we need to work on getting a simple DBUS method in PackageKit to do this with one DBUS call from the totem plugin, rather than the thousands of ubuntu specific code you propose. It really would be a few lines of code to the PackageKit sesson helper to add: IsCodecAvailable(gstreamer-provide-name) Or you could even do it using packagekit-glib (or the python binding) in about 10 lines of code with anything > 0.2.2. e.g. list=what_provides(CODEC, provide, ~installed) -- and using the raw DBUS interface wouldn't be that tricky either. Richard.
There's a system example here: http://www.packagekit.org/files/system.c and a session example here: http://www.packagekit.org/files/session.c You can get compile instructions here: http://www.packagekit.org/pk-faq.html#session-system
> I really think we need to work on getting a simple DBUS method in > PackageKit to do this with one DBUS call from the totem plugin, > rather than the thousands of ubuntu specific code you propose. I was proposing to include ~50 lines ("thousands", eh?) of optional code which exists *now* and adds some real benefit to a lot of users *right now* (ie. debian and ubuntu users). Until a better and more generic solution is found. Not more, not less. This seemed a useful trade-off to me. I was not suggesting that this is the right long-term solution, or trying to make a political statement of any sort. It might make sense to add API for this kind of thing to GStreamer directly (which could work similar to how the install codec stuff works now, ie. the implementation would be provided by an external hook script/program which then queries packageKit via d-bus or whatever else).
There is a generic solution, it's called PackageKit. Putting lots and lots of code into totem means that we have to do the same thing for any other media player (KDE or GNOME) that wants to do the same thing. You can use PackageKit *right now* and produce similar results to what you did in your patch, only making it compatible with foresight, fedora, ubuntu, mandriva and others with little effort on your part.
Created attachment 121203 [details] [review] same as above, but adds delayed downloading of content list and a genre tree
Would a generic solution also include pushing the functionality into GStreamer? Then GStreamer could utilise on PackageKit on the Unix/Linux platforms and on Windows / Mac OS X do whatever is appropriate. If you are not willing to have shortcuts in at the moment, why not go for the full solution (via GStreamer)?
> Would a generic solution also include pushing the functionality into GStreamer? > Then GStreamer could utilise on PackageKit on the Unix/Linux platforms and on > Windows / Mac OS X do whatever is appropriate. Yes, that's my plan at least. > If you are not willing to have shortcuts in at the moment, why not go for the > full solution (via GStreamer)? There isn't really a reason to block on that stuff though, yhe plugin is perfectly usable without this, it's just much nicer with it. I plan on committing it to trunk soonish, then we can go from there.
(In reply to comment #11) > There isn't really a reason to block on that stuff though, yhe plugin is > perfectly usable without this It does not work on Fedora, SUSE, Foresight or Solaris. It only works on Debian based systems. Might I suggest the fact that Canonical paid for this feature might infer some bias? > it's just much nicer with it. I plan on committing it to trunk soonish I think you have to get Bastien to ack it before you do that... Richard.
> It does not work on Fedora, SUSE, Foresight or Solaris. It only works on Debian > based systems. What I meant is: the plugin is perfectly usable if I remove the apt-specific stuff (provided some of the required codecs are installed). Is that not actually the case? It did work on Fedora at some point. > I think you have to get Bastien to ack it before you do that... What I meant is: I'll commit it without the apt-specific stuff, which was ok-ed by Bastien in comment #2, if I'm not mistaken.
(In reply to comment #13) > What I meant is: I'll commit it without the apt-specific stuff, which was ok-ed > by Bastien in comment #2, if I'm not mistaken. Right, I agree with that. It's just your last patch (comment 9) still had the APT bits left in.
BTW, before I forget. I don't want to see action_set_mrl_and_play coming back in the API, you should use add_to_playlist_and_play().
And please don't forget about i18n: https://bugs.launchpad.net/ubuntu/+source/totem/+bug/329573
What's the status of this?
Still waiting on a cleanup.
Tim, any news on this?
(In reply to comment #12) > It does not work on Fedora, SUSE, Foresight or Solaris. It only works on Debian > based systems. Might I suggest the fact that Canonical paid for this feature > might infer some bias? Using PackageKit, OTOH, would mean it would only be accessible on RedHat-based systems. Might I suggest the fact that Red Hat paid for this development might infer some bias?
(In reply to comment #20) > Using PackageKit, OTOH, would mean it would only be accessible on RedHat-based > systems. Bttzzz. Please see here: http://www.packagekit.org/pk-users.html > Might I suggest the fact that Red Hat paid for this development might infer > some bias? Red Hat do not pay for development of PackageKit. I started PackageKit before I was working at Red Hat, and there are plenty of other developers who contribute to PackageKit who work for the other distros. Please also see the apt and aptcc backends, and the progress of integrating a debconf frontend in PackageKit. Please stop trolling.
(In reply to comment #20) > (In reply to comment #12) > > It does not work on Fedora, SUSE, Foresight or Solaris. It only works on Debian > > based systems. Might I suggest the fact that Canonical paid for this feature > > might infer some bias? > > Using PackageKit, OTOH, would mean it would only be accessible on RedHat-based > systems. When trolling, at least try harder. The current code makes it only accessible on Debian-based system, might that have something to do with Canonical funding the project? (the answer is yes, by the way) > Might I suggest the fact that Red Hat paid for this development might infer > some bias?
Guys, there is nothing left to debate here. The apt-specific code will be removed before merging this (and the functionality will ultimately be implemented in GStreamer - distros can then implement that using whatever they prefer as backend, like they do now with the easy codec installer stuff).
ping. Can we see this committed? Does anything need modifying to be accepted?
Created attachment 164580 [details] [review] BBC plugin updated to latest git master
Created attachment 164582 [details] Example source file Note that there is no content available at the moment due to a server error with the content index: http://open.bbc.co.uk/rad/uriplay/availablecontent I have tested this with the attached hacked cache file (copied to ~/.cache/totem/plugins/bbc, open the first entry)
Created attachment 164584 [details] [review] BBC plugin updated to latest git master Had some old code in configure.in
(In reply to comment #24) > ping. Can we see this committed? Does anything need modifying to be accepted? Yes, as I mentioned, it still needs porting to something distribution agnostic for the codecs discovery. And you didn't test it at all, given that Python plugins are disabled in master (the Python support needs porting to PyGI and introspection).
The last patch should be distribution agnostic. I wasn't able to test it in git master as I don't have a GTK3 setup yet. I tested it against 2.30.2 for the correctness of the changes to the patch. If it is just as broken as the existing Python plugins can we commit this to get some momentum on this issue?
Issue mentioned weren't fixed, and we want to use Grilo for our media sources, so dup'ing. *** This bug has been marked as a duplicate of bug 628648 ***