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 548316 - Add support for mp3 streaming in goear.com
Add support for mp3 streaming in goear.com
Status: RESOLVED OBSOLETE
Product: rhythmbox
Classification: Other
Component: Plugins (other)
HEAD
Other All
: Normal enhancement
: ---
Assigned To: Resapplet maintainers
RhythmBox Maintainers
Depends on:
Blocks:
 
 
Reported: 2008-08-18 17:53 UTC by Víctor Manuel Jáquez Leal
Modified: 2013-06-25 11:11 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch to enable goear plugin in rhythmbox. (61.32 KB, patch)
2008-08-18 17:54 UTC, Víctor Manuel Jáquez Leal
reviewed Details | Review
goear icon (1.14 KB, image/png)
2008-08-18 17:55 UTC, Víctor Manuel Jáquez Leal
  Details

Description Víctor Manuel Jáquez Leal 2008-08-18 17:53:20 UTC
goear.com is a free mp3 song streaming. This plugin enable the search and play of songs from there.
Comment 1 Víctor Manuel Jáquez Leal 2008-08-18 17:54:43 UTC
Created attachment 116893 [details] [review]
patch to enable goear plugin in rhythmbox.
Comment 2 Víctor Manuel Jáquez Leal 2008-08-18 17:55:17 UTC
Created attachment 116894 [details]
goear icon
Comment 3 Jonathan Matthew 2008-08-31 12:21:02 UTC
A couple of style things:
- Return statements at the end of void functions look really weird
- The functions in rb-goear-utils.h need much more specific names

and more seriously:
- We really need to avoid synchronous http requests on the main thread
- The source should not construct its own search box.  It should use the RBSource search method instead.
- I'm not sure why get_playback_uri ends up using the metadata reader, but it doesn't seem like a good idea to me.  The basic metadata will be read from the stream during playback, and the other stuff probably isn't very useful here.
- I'd suggest using gio rather than libsoup, maybe

Can you write up an explanation of how this works, preferably as a comment in the code?  It's not particularly clear.

Is this sort of usage acceptable under the site's terms and conditions?
Comment 4 Jonathan Matthew 2008-08-31 12:22:01 UTC
One more thing: the icon should be themeable.  See the visualizer plugin for an example of how to do that.
Comment 5 Víctor Manuel Jáquez Leal 2008-09-01 09:11:06 UTC
(In reply to comment #3)
> A couple of style things:
> - Return statements at the end of void functions look really weird

:) they're a personal taste

> - The functions in rb-goear-utils.h need much more specific names

Yeah, you're right. The rational of that file was to move out the helper functions which didn't have hard dependencies with th RBGoearSource object.

> and more seriously:
> - We really need to avoid synchronous http requests on the main thread

I'm agree, but I need to fetch the XML stream descriptor synchronously in order to provide the stream URI when the get_playback_uri callback is called. In that way bandwidth is save because the XML descriptor is called only when the user ask to play a specific song, and not all the available play list. So, I didn't figure out how to fetch the XML descriptor asynchronously in the get_playback_uri callback.

> - The source should not construct its own search box.  It should use the
> RBSource search method instead.

I played with the search box but it didn't work for me: it does incremental searches on a local database. But in this case the database is remote, so incremental search (a search each time a user type a key) is just crazy. And again, I couldn't figure out how to disable the incremental search in the search box, so I wrote my own.

> - I'm not sure why get_playback_uri ends up using the metadata reader, but it
> doesn't seem like a good idea to me.  The basic metadata will be read from the
> stream during playback, and the other stuff probably isn't very useful here.

I'm agree, but I didn't find how to add the metadata info in the RhythmDB during the playback. I don't like the burden added by the metadata reader, but I found quite useful to see the metadata in the browser.

> - I'd suggest using gio rather than libsoup, maybe

What are the advantages to use gio instead of libsoup? Avoid a dependency?

> Can you write up an explanation of how this works, preferably as a comment in
> the code?  It's not particularly clear.

goear.com is a youtube like site for mp3 streams. This plugin searches on that site, parses the html ouput and fills the browser. When the user plays a song, the plugin fetches the xml descriptor of the song to get the stream URI and then playback the stream.
 
> Is this sort of usage acceptable under the site's terms and conditions?

The terms and conditions are extremely ambiguous (http://www.goear.com/pages/terms_and_conditions.html), but the last paragraph (in Spanish) seems to imply that this type of usage is not acceptable :S 

Thank you very much for your input.
Comment 6 Jonathan Matthew 2008-09-02 11:04:31 UTC
(In reply to comment #5)
 
> > - The source should not construct its own search box.  It should use the
> > RBSource search method instead.
> 
> I played with the search box but it didn't work for me: it does incremental
> searches on a local database. But in this case the database is remote, so
> incremental search (a search each time a user type a key) is just crazy. And
> again, I couldn't figure out how to disable the incremental search in the
> search box, so I wrote my own.

That's the wrong way to go about it.  If the search box doesn't work for you, then we need to find a way to extend it to make it usable for more types of source.  It's a pretty safe bet that someone else is going to make a similar plugin for some other site.
 
> > - I'm not sure why get_playback_uri ends up using the metadata reader, but it
> > doesn't seem like a good idea to me.  The basic metadata will be read from the
> > stream during playback, and the other stuff probably isn't very useful here.
> 
> I'm agree, but I didn't find how to add the metadata info in the RhythmDB
> during the playback. I don't like the burden added by the metadata reader, but
> I found quite useful to see the metadata in the browser.

You should be able to use the RBPlayer 'info' signal instead.
 
> > - I'd suggest using gio rather than libsoup, maybe
> 
> What are the advantages to use gio instead of libsoup? Avoid a dependency?

The interface you've defined in rb-goear-http isn't much different to GFile, and gio handles the proxy configuration stuff by itself.

> > Can you write up an explanation of how this works, preferably as a comment in
> > the code?  It's not particularly clear.
> 
> goear.com is a youtube like site for mp3 streams. This plugin searches on that
> site, parses the html ouput and fills the browser. When the user plays a song,
> the plugin fetches the xml descriptor of the song to get the stream URI and
> then playback the stream.

I was after more of a technical description - which documents does it fetch, what do they look like, and how are the URIs constructed?  If anyone else is going to touch this code, it's better that they have some information to work with, rather than having to piece it together by reading the code and looking at
the various html and xml files.  This sort of detail belongs in the code.

> > Is this sort of usage acceptable under the site's terms and conditions?
> 
> The terms and conditions are extremely ambiguous
> (http://www.goear.com/pages/terms_and_conditions.html), but the last paragraph
> (in Spanish) seems to imply that this type of usage is not acceptable :S 

If the site doesn't allow this sort of use, then I don't think we should distribute the plugin.
Comment 7 Víctor Manuel Jáquez Leal 2008-09-19 10:01:43 UTC
Hi Jonathan,

Sorry for this so delayed reply :(

> > > - The source should not construct its own search box.  It should use the
> > > RBSource search method instead.
> > 
> > I played with the search box but it didn't work for me: it does incremental
> > searches on a local database. But in this case the database is remote, so
> > incremental search (a search each time a user type a key) is just crazy. And
> > again, I couldn't figure out how to disable the incremental search in the
> > search box, so I wrote my own.
> 
> That's the wrong way to go about it.  If the search box doesn't work for you,
> then we need to find a way to extend it to make it usable for more types of
> source.  It's a pretty safe bet that someone else is going to make a similar
> plugin for some other site.

Well, it means basically I'll need to patch it in order to add a property to disable the time-out signal and just attend the activate signal. Is it that ok for you?
 
> > > - I'm not sure why get_playback_uri ends up using the metadata reader, but it
> > > doesn't seem like a good idea to me.  The basic metadata will be read from the
> > > stream during playback, and the other stuff probably isn't very useful here.
> > 
> > I'm agree, but I didn't find how to add the metadata info in the RhythmDB
> > during the playback. I don't like the burden added by the metadata reader, but
> > I found quite useful to see the metadata in the browser.
> 
> You should be able to use the RBPlayer 'info' signal instead.

I did some test, and the 'info' signal is not enough in this case, because I need to know the duration of the stream before goes to PAUSE state, otherwise the seeking will be disabled. Furthermore, the info signal doesn't provide all the data provided by the stream tags.
 
> > > - I'd suggest using gio rather than libsoup, maybe
> > 
> > What are the advantages to use gio instead of libsoup? Avoid a dependency?
> 
> The interface you've defined in rb-goear-http isn't much different to GFile,
> and gio handles the proxy configuration stuff by itself.

Also checked on this, and gio use libsoup beneath of it in http operations. So I don't feel necessary add a new layer for this.
 
> > > Is this sort of usage acceptable under the site's terms and conditions?
> > 
> > The terms and conditions are extremely ambiguous
> > (http://www.goear.com/pages/terms_and_conditions.html), but the last paragraph
> > (in Spanish) seems to imply that this type of usage is not acceptable :S 
> 
> If the site doesn't allow this sort of use, then I don't think we should
> distribute the plugin.

Yeah :( anyway, has been a nice exercise :)

Thank you very much.

Comment 8 Jonathan Matthew 2008-09-20 07:47:56 UTC
(In reply to comment #7)
> > That's the wrong way to go about it.  If the search box doesn't work for you,
> > then we need to find a way to extend it to make it usable for more types of
> > source.  It's a pretty safe bet that someone else is going to make a similar
> > plugin for some other site.
> 
> Well, it means basically I'll need to patch it in order to add a property to
> disable the time-out signal and just attend the activate signal. Is it that ok
> for you?

Off hand, I'd say that extending the 'can_search' source method would be the way to do it.  Currently, the source indicates whether it supports searching at all.  Perhaps it should return 'no search' - hide the search bar; 'instant search' - current behaviour; or 'explicit search' - only search when the search box is activated.  We'd need some visible indicator of the search type, but I don't have any specific ideas.



> > You should be able to use the RBPlayer 'info' signal instead.
> 
> I did some test, and the 'info' signal is not enough in this case, because I
> need to know the duration of the stream before goes to PAUSE state, otherwise
> the seeking will be disabled. Furthermore, the info signal doesn't provide all
> the data provided by the stream tags.

Again, if something in rhythmbox is insufficient for your needs, then we should fix that, rather than work around it some other way.  I'm not sure what to do about the duration, though.
 
> > > > - I'd suggest using gio rather than libsoup, maybe
> > > 
> > > What are the advantages to use gio instead of libsoup? Avoid a dependency?
> > 
> > The interface you've defined in rb-goear-http isn't much different to GFile,
> > and gio handles the proxy configuration stuff by itself.
> 
> Also checked on this, and gio use libsoup beneath of it in http operations. So
> I don't feel necessary add a new layer for this.

You're adding another layer either way.  If you use gio, you don't have to write and maintain that layer yourself.

> > > > Is this sort of usage acceptable under the site's terms and conditions?
> > > 
> > > The terms and conditions are extremely ambiguous
> > > (http://www.goear.com/pages/terms_and_conditions.html), but the last paragraph
> > > (in Spanish) seems to imply that this type of usage is not acceptable :S 
> > 
> > If the site doesn't allow this sort of use, then I don't think we should
> > distribute the plugin.
> 
> Yeah :( anyway, has been a nice exercise :)

It's not really possible to distribute plugins written in C separately from rhythmbox, but that could change.
Comment 9 Víctor Manuel Jáquez Leal 2013-06-25 11:11:55 UTC
Let's close this bug, since it should belong to grilo now