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 635394 - Add ability to get video URL from page URL, or <embed> URL
Add ability to get video URL from page URL, or <embed> URL
Status: RESOLVED FIXED
Product: grilo
Classification: Other
Component: core
git master
Other Linux
: Normal normal
: ---
Assigned To: Iago Toral
grilo-maint
Depends on:
Blocks:
 
 
Reported: 2010-11-20 23:57 UTC by Bastien Nocera
Modified: 2010-12-13 12:43 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch set for grilo core adding the feature (tgz) (5.65 KB, application/x-compressed-tar)
2010-12-03 10:05 UTC, Iago Toral
  Details
Patch set for grilo plugins adding support for this feature in the Youtube plugin (tgz) (2.35 KB, application/x-compressed-tar)
2010-12-03 10:07 UTC, Iago Toral
  Details
core: Add support for two new operations in GrlMediaSource (7.55 KB, patch)
2010-12-03 11:24 UTC, Iago Toral
none Details | Review
core: Implemented grl_multiple_get_media_from_site(). (4.85 KB, patch)
2010-12-03 11:24 UTC, Iago Toral
none Details | Review
core: Do not use and idle destroy function for media_from_site, since (3.10 KB, patch)
2010-12-03 11:24 UTC, Iago Toral
none Details | Review
core: Add a list of metadata keys as input parameter for (4.72 KB, patch)
2010-12-03 11:25 UTC, Iago Toral
none Details | Review
core: Added flags input parameter to media_from_site() APIs. (4.74 KB, patch)
2010-12-03 11:25 UTC, Iago Toral
none Details | Review
youtube: Implemented test_media_from_site (3.13 KB, patch)
2010-12-03 11:26 UTC, Iago Toral
none Details | Review
youtube: Implemented get_media_from_site (4.23 KB, patch)
2010-12-03 11:26 UTC, Iago Toral
none Details | Review
youtube: Use keys from GrlMediaSourceMediaFromSiteSpec to build (970 bytes, patch)
2010-12-03 11:26 UTC, Iago Toral
none Details | Review

Description Bastien Nocera 2010-11-20 23:57:17 UTC
This would be useful for Totem integration as it would allow adding hooks to web browser to load a particular video on supported video sites.

For example:
- Open a YouTube video in web browser
- Click "Open in Totem"
- Totem checks whether any plugins can claim the URL, gets Grilo to resolve it to a full video URL
- Watch video in a real movie player :)
Comment 1 Iago Toral 2010-11-21 12:15:05 UTC
Sounds like an interesting use case.

I guess for this we would need new API in Grilo to expose this feature to applications, something like this maybe:

GrlMedia * grl_media_source_get_media_from_site_uri (const gchar *web_uri);

The implementation for this would have to go through all supported plugins and check for the one that supports 'web_uri'. Of course we need plugin API for plugins to support this. Maybe something like this:

GrlMedia * get_media_from_site_uri (const gchar *web_uri).
Would return NULL if web_uri is not supported.

If plugins do not implement this API, a default implementation returning NULL should be provided by the base class.
Comment 2 Iago Toral 2010-12-02 08:58:24 UTC
Working on this, I have a patch almost ready, just need to do some testing.
Comment 3 Bastien Nocera 2010-12-02 12:09:01 UTC
I have actually implemented this in totem-pl-parser, using the "quvi" library. We're sorting out licensing now, and it should be ready to use.

The good side of quvi is that the "hacky" parts are written in lua, which means it's easy to hack without needing recompilation.

See bug 635709.
Comment 4 Iago Toral 2010-12-03 10:05:17 UTC
Created attachment 175763 [details]
Patch set for grilo core adding the feature (tgz)

I think this feature is good to have in Grilo nonetheless.

These are the patches for grilo core, they add:

grl_multiple_get_media_from_site: Goes through all sources that support media resolution from site url, selects the one that supports the specific site url and then requests the GrlMedia.

grl_media_source_test_media_from_site: Allows the client to check if a certain media source supports resolution for a specific url (this is fast and synchronous and does not imply any network I/O).

grl_media_source_get_media_from_site: Requests the creation of a GrlMedia from a site url, this is asynchronous and would involve network I/O.

Things missing:
  - No support for full resolution mode, although clients can do that once they get the media. I could add support for that too later anyway.
  - No cancellation support. I could add that later.

I'll be attaching another patch set for grilo-plugins adding support for this in the Youtube plugin.
Comment 5 Iago Toral 2010-12-03 10:07:37 UTC
Created attachment 175764 [details]
Patch set for grilo plugins adding support for this feature in the Youtube plugin (tgz)
Comment 6 Víctor Manuel Jáquez Leal 2010-12-03 10:49:51 UTC
Iago,

Please use git-bz[1] or post the patches to the mailing list, because tarballs are rather difficult to review and comment.

A quick glance shows that some commit logs are bad formatted and seems to me that "core: Do not use and idle destroy function for media_from_site..." is unrelated to this bug.

1. http://blog.fishsoup.net/2008/11/16/git-bz-bugzilla-subcommand-for-git/
Comment 7 Iago Toral 2010-12-03 10:59:00 UTC
that(In reply to comment #6)
> Iago,
> 
> Please use git-bz[1] or post the patches to the mailing list, because tarballs
> are rather difficult to review and comment.

Ok, will do.

> A quick glance shows that some commit logs are bad formatted and seems to me
> that "core: Do not use and idle destroy function for media_from_site..." is
> unrelated to this bug.
> 
> 1. http://blog.fishsoup.net/2008/11/16/git-bz-bugzilla-subcommand-for-git/

commit logs are well formatted, at least according to my git log output.
And that patch is related to the bug, maybe it could be  squashed, but it is related, it is part of the implementation.
Comment 8 Iago Toral 2010-12-03 11:24:50 UTC
Created attachment 175768 [details] [review]
core: Add support for two new operations in GrlMediaSource

- test_media_from_site
      - get_media_from_site
Comment 9 Iago Toral 2010-12-03 11:24:55 UTC
Created attachment 175769 [details] [review]
core: Implemented grl_multiple_get_media_from_site().

This function goes through all sources that implement GRL_OP_MEDIA_FROM_SITE
looking for the first one capable of instantiating a GrlMedia object that
represents the media resource exposed by certain site URI. Basically, it is
a convenience function to exploit grl_media_source_get_media_from_site().
Comment 10 Iago Toral 2010-12-03 11:24:59 UTC
Created attachment 175770 [details] [review]
core: Do not use and idle destroy function for media_from_site, since

the implementation on the plugin side is likely to be asynchronous,
use a relay callback instead.
Comment 11 Iago Toral 2010-12-03 11:25:04 UTC
Created attachment 175771 [details] [review]
core: Add a list of metadata keys as input parameter for

the media_from_site API.
Comment 12 Iago Toral 2010-12-03 11:25:08 UTC
Created attachment 175772 [details] [review]
core: Added flags input parameter to media_from_site() APIs.
Comment 13 Iago Toral 2010-12-03 11:26:01 UTC
Created attachment 175773 [details] [review]
youtube: Implemented test_media_from_site
Comment 14 Iago Toral 2010-12-03 11:26:06 UTC
Created attachment 175774 [details] [review]
youtube: Implemented get_media_from_site
Comment 15 Iago Toral 2010-12-03 11:26:11 UTC
Created attachment 175775 [details] [review]
youtube: Use keys from GrlMediaSourceMediaFromSiteSpec to build

GrlMedia instances in media_from_site().
Comment 16 Iago Toral 2010-12-13 11:57:12 UTC
Ok, after discussing this feature in the mailing list and with Juan we decided to rename this to media_from_uri.

For more generic situations where users may be interested in generic metadata => GrlMedia transformations, grl_media_source_query() would be the way to go for now. 

In the future, if generic metadata => GrlMedia conversions prove to have interesting use cases -other than the URI case- that we want to support we may consider a new query() API to make this type of use cases plugin-independent (at the moment query() is not plugin-independent).
Comment 17 Iago Toral 2010-12-13 12:00:18 UTC
Fixed in core. Commits:

3962e466cab4bbd9f137e043e2d52cf2e2f0ecd0
..
93d48590067290d4792f2285e0647df20debb247

Also, implemented this in the youtube plugin (grilo-plugins). Commits:

9ee4f3f9e6d31d749b1157c3cc141adc8a497c5a
20e2f13b756cc9ca62d6f6d68e0a2354e8cb8bba
Comment 18 Juan A. Suarez Romero 2010-12-13 12:43:53 UTC
(In reply to comment #16)
> Ok, after discussing this feature in the mailing list and with Juan we decided
> to rename this to media_from_uri.
> 

Just a comment to this. This media_from_uri() function expects a URI that depending on the plugin, can be the uri for the site or an uri pointing to the media element. It is expected that plugins document which kind of URIs are them able to reverse.

Thus, both this bug and bug #636693 can be fixed together (providing that plugins are able to provide a GrlMedia from these URIs).