Bug 131137 - no proxy support for last.fm streams
Summary: no proxy support for last.fm streams
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: general (show other bugs)
Version: 1.4.1
Platform: Ubuntu Linux
: NOR wishlist
Target Milestone: ---
Assignee: Amarok Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-07-20 23:18 UTC by Felix Eckhofer
Modified: 2006-09-06 20:12 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
Martin Ellis' patch (19.20 KB, patch)
2006-08-29 16:50 UTC, Ian Monroe
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Felix Eckhofer 2006-07-20 23:18:47 UTC
Version:           1.4.1 (using KDE KDE 3.5.3)
Installed from:    Ubuntu Packages
OS:                Linux

last.fm streams don't work when using a proxy.

it seems as if xine is asking the proxy for http://localhost:... which of course cannot work.

i'd really appreciate this feature. perhaps simply adding a "proxy" commandline-parameter to amarok_proxy.rb while overriding the settings for xine would do the trick...

thanks.
Comment 1 richlv 2006-07-24 15:36:26 UTC
so that we can either confirm this or whatever :

i believe xine connecting to localhost is ok, because amarok implements lastfm proxy on localhost, which in turn streams from the lastfm.
lastfm streams are not working for me through proxy (squid), too - i just get

amarok: BEGIN: void Playlist::advanceDynamicTrack(PlaylistItem*)
amarok: END__: void Playlist::advanceDynamicTrack(PlaylistItem*) - Took 0.0002s
amarok: BEGIN: void EngineController::play(const MetaBundle&, uint)
amarok:         [controller] Loading URL: lastfm://user/richlv/neighbours
amarok:         [controller] Connecting to protocol: lastfm
amarok:         [LastFm] Changing station:lastfm://user/richlv/neighbours
amarok: END__: void EngineController::play(const MetaBundle&, uint) - Took 0.43s

and a popup in amarok, which says "Failed to play this last.fm stream."

i have $http_proxy set correctly, but there are no requests in proxy logfiles.
Comment 2 Felix Eckhofer 2006-07-24 19:56:07 UTC
Jep, xine connecting to localhost _would_ be alright. But instead it asks the proxy to connect to localhost (which, in this case, would be the proxy itself). So this can't work.
Also, this forum post by Mark Kretschmann confirmes that this actually is a bug/missing feature: http://amarok.kde.org/forum/index.php/topic,12572.msg13581.html#msg13581

regards, felix
Comment 3 Martin Ellis 2006-08-28 01:35:23 UTC
FWIW, I have Last.FM streams working through an HTTP proxy here.
There's a bit of a trick to getting it working.

Firstly, I had to *unset* the http_proxy environment variable if you are using the xine engine, because it doesn't seem to know not to use the proxy for contacting localhost.

Secondly, I had to replacing most of the references to QHttp to something that uses KIO, and therefore respects the KDE proxy settings. (QHttp doesn't want to use proxies in Qt3)

My change won't go into SVN, because the developers say it already uses KIO:

Aug 27, 23:36 <eean> I thought markey had changed to using KIO for
                     that stuff
Aug 27, 23:36 <mart> huh? that must've happened very recently.
Aug 27, 23:36 [markey went all KIO, it's true]

As far as I can see, it is still largely using QHttp:
http://websvn.kde.org/trunk/extragear/multimedia/amarok/src/lastfm.cpp?rev=574383&view=auto

Either way, if anyone would like a patch for current SVN on a "works for me" basis (and I can't guarantee it's any better than that) you're welcome to mail me privately (m.a.ellis@ncl.ac.uk), and I'll send you the latest diff.
Comment 4 Ian Monroe 2006-08-28 05:03:41 UTC
Please attach the patch to this bug report.
Comment 5 Mark Kretschmann 2006-08-28 10:36:01 UTC
On Monday 28 August 2006 01:35, Martin Ellis wrote:
> Aug 27, 23:36 <eean> I thought markey had changed to using KIO for
>                      that stuff
> Aug 27, 23:36 <mart> huh? that must've happened very recently.
> Aug 27, 23:36 [markey went all KIO, it's true]


Oh, heh. Sorry for the confusion, but I was joking there. The only thing that 
I have converted to KIO is the fetching of the cover image from Amazon.com. 
The rest still uses QHttp.
Comment 6 Martin Ellis 2006-08-29 15:15:47 UTC
The patch I'm using is currently here:
http://martin.ellis.name/amarok/proxy.diff

It's not for SVN yet.  Please don't commit.

Off the top of my head: I'm still a little concerned about the memory safety of the memcpy (there has to be a better way, surely?). I haven't tested that code path yet.  Also nothing is documented.  Um... I think something else, but that escapes me right now.

However, it should also fix 122092 and 130372.

Let me know if you're happy with the approach, and can commit when it's properly ready and tested.  And please CC me next time, I've no idea why bugzilla isn't subscribing me.
Comment 7 Ian Monroe 2006-08-29 16:45:41 UTC
It doesn't make sense to me to make an AmarokHttp that acts like QHttp but uses KIO. Couldn't you just use KIO directly instead?
Comment 8 Ian Monroe 2006-08-29 16:50:53 UTC
Created attachment 17554 [details]
Martin Ellis' patch

Makes me nervous having patches on remote urls. :)
Comment 9 Martin Ellis 2006-08-29 18:13:31 UTC
[Resend.  Perhaps b.k.o didn't like my mail address?]

On Tuesday 29 August 2006 15:50, Ian Monroe wrote:
> It doesn't make sense to me to make an AmarokHttp that acts like QHttp but
> uses KIO. Couldn't you just use KIO directly instead?  


I started to do that, but the changes got rather out of hand.  Certainly too 
large to consider for a point release.

Although QHttp works asynchronously, it's API 'looks' synchronous - there are 
no callback functions.

KIO has an asynchronous API - you need a callback slot for each request.  This 
quickly doubles the number of methods you need (actually not a big problem in 
itself).

The difference is evident from a comparison of the get(...)/readAll(...) 
pattern used in lastfm.cpp, and the signals/slots used by KIO.

The problem then, is that the current design assumes that the call sequence 
that sets up the LastFM proxy (which begins in EngineController::instance, 
and in turn invokes methods on WebService) returns once the proxy is set up.

There are two ways to solve this using KIO, as far as I can see:

The first involves quite a large redesign to allow the methods that 
EngineController::instance calls to return before they can determine whether 
they've been successful or not.   This would allow the WebService methods to 
invoke async. KIO methods, connect slots, and return immediately. 

For this, you'd need extra slots in EngineController to represent a 
continuation for each flow control branch in that method (at least, one for 
each basic block where one of it's predecessors invoked a WebService method 
that returns immediately).    You'd also need the extra slots in the 
WebService class each time you see a get(...)/readAll(...) pair.

The resulting design is more complex.  You end up emitting signals from 
WebService that hit slots in EngineController, which then invoke methods on 
WebService, and set up some more slots.  For example, playbackFailed, would 
have to become a slot in EC.

I think I can show you a patch that represents the start of an abortive 
attempt at this approach.


The second approach is to use KIO each time in lastfm.cpp.
This way would continue to use kapp->processEvents to wait for HTTP responses, 
rather than returning directly, as per method 1.
Rather than returning immediately, functions that use get()/readAll() would 
invoke a KIO operation, and repeatedly hit processEvents, as it does 
currently.

Of course, setting up a KIO job each time gets a bit cumbersome, so you'd end 
up refactoring all the KIO stuff into one place.... but then you end up with 
something not a million miles away from the proposed patch.

Given that QHttp supports proxies in Qt4, you might be able to simply return 
to Qt3 when that arrives.  Else the KIO API may have improved sufficiently - 
I haven't looked at KDE4 yet.
Either way, you haven't wasted much work, and you haven't introduced any 
sweeping changes.

This method is less robust than Method 1 though.  Currently, when LastFM is 
down, some busy loop spins on kapp->processEvents(), eating CPU.  This method 
still has that flaw, but it's no worse than the status quo.

The difference between the two approaches boils down to whether you continue 
to spin in the processEvents() busy loop.

Method 1:
 +  You don't end up doing this kapp->processEvents() within a loop stuff, 
making it more robust when LastFM stops responding.
 - BIG redesign.
 - Resulting design more complex. 
 - Who'd code it?

Method 2:
 + Simple changes - could probably fit in a stable release series.
 + Already coded, patch fixes 3 bugs.
 + If someone does want to code a more robust redesign, it's no harder with 
this approach, than with remaining in QHttp.
 - No improvement in robustness when LastFM is down.
Comment 10 Mark Kretschmann 2006-08-29 18:54:58 UTC
On Tuesday 29 August 2006 18:13, Martin Ellis wrote:
> On Tuesday 29 August 2006 15:50, Ian Monroe wrote:
> > It doesn't make sense to me to make an AmarokHttp that acts like QHttp
> > but uses KIO. Couldn't you just use KIO directly instead?
>
> I started to do that, but the changes got rather out of hand.  Certainly
> too large to consider for a point release.
>
> Although QHttp works asynchronously, it's API 'looks' synchronous - there
> are no callback functions.


I don't see what's synchronous about QHttp's API. It's entirely asynchronous. 
You start it, connect to a signal, and when it's finished the signal gets 
raised.

--
Mark
Comment 11 Martin Ellis 2006-08-29 19:01:15 UTC
On Tuesday 29 August 2006 17:55, Mark Kretschmann wrote:
> I don't see what's synchronous about QHttp's API. It's entirely
> asynchronous. You start it, connect to a signal, and when it's finished the
> signal gets raised.


It *can* be used asynchronously, but that's not how you use it:

bool
WebService::changeStation( QString url )
{
...
    QHttp http( m_baseHost, 80 );
    http.get( ... );

    do
        kapp->processEvents();
    while( http.state() != QHttp::Unconnected );

    if ( http.error() != QHttp::NoError )
    {
...
    }

    const QString result( QDeepCopy<QString>( http.readAll() ) );


I see no signals.
Comment 12 Martin Ellis 2006-08-29 19:20:43 UTC
On Tuesday 29 August 2006 18:01, Martin Ellis wrote:
> It *can* be used asynchronously, but that's not how you use it:


Uh.  Let me clarify that...

If you did use it as an asyncronous API ('scuse the abuse of terminology), 
then the result would look pretty much like the first method I described.

Of course, there'd be no point in doing that with Qt3, since if you were going 
to rewrite to use signals/slots model,  you might as well use KIO rather than 
QHttp anyway.

I'm not saying the design I've implemented in the patch is good - actually, I 
dislike the whole idea of an AmarokHttp - but I do think that this approach 
is the easiest and simplest way to get proxy support in the 1.4 series.

As such, is there any reason not to treat it as a disposable class? That is, 
use it to fix a few bugs now, and then scrap it when someone writes it for an 
asynchronous API.
Comment 13 Martin Ellis 2006-09-03 23:46:47 UTC
On a related note, you may want to fix some outdated text on your wiki...

http://amarok.kde.org/wiki/FAQ#Do_you_accept_patches.3F

  Do you accept patches?
  Happily! Your best course of action is to let us know what you plan
  to do before you do any work so we can discuss it, but don't fret we
  haven't turned down a patch yet! Discussion is mostly to help you patch
  the correct bits of Amarok. Then either submit a patch to the mailing
  list, or commit if you have svn access. However, please let us know
  before you commit new features to svn! If you have a bug fix then
  just go ahead, this is open source after all :)
Comment 14 Ian Monroe 2006-09-04 04:16:04 UTC
Martin, you haven't given us a patch to turn down yet. Last we talked I pointed out a few things with the patch (a code path that would never occur IIRC), and you said it wasn't finished.
Comment 15 Martin Ellis 2006-09-04 04:38:02 UTC
On Monday 04 September 2006 03:16, Ian Monroe wrote:
> Martin, you haven't given us a patch to turn down yet. Last we talked I
> pointed out a few things with the patch (a code path that would never occur
> IIRC), and you said it wasn't finished.


Ian,

The patch I'm talking about is the one you attached to this BR.

What's wrong with covering that code path?  Are you sure that KIO will never 
split that message into two calls to slotData?  Across all supported KDE 
versions?  Can you guarantee that sure Last.FM will not change the size of 
the result for any of the queries?
It could be removed if you feel more comfortable with less robust code!?!

I said it wasn't finished because I wasn't sure about the memcpy.  But you 
didn't suggest anything better, and I've looked at it again, and I can't 
figure out how to break it.

The only thing left as far as I can see is adding documentation.

The point is moot now, I guess - I don't think my patch will work after
rev. 580587 anyway.

Martin
Comment 16 Alexandre Oliveira 2006-09-06 04:36:40 UTC
SVN commit 581321 by aoliveira:

Lastfm streams work with proxies.
Mostly extracted from a patch by Martin Ellis <martin.ellis@kdemail.net>.
Unfortunately we can't use the dcop call for this, since it would cause those timing problems again.
I tried hard not to break DAAP (what might even work with proxies as well, as soon as eean makes the 
necessary changes to his code, and stop using QHttp).
BUG: 131137


 M  +18 -8     amarok_proxy.rb  
 M  +103 -21   lastfm.cpp  
 M  +32 -0     lastfm.h  
 M  +1 -0      mediadevice/daap/proxy.cpp  


--- trunk/extragear/multimedia/amarok/src/amarok_proxy.rb #581320:581321
@@ -7,6 +7,8 @@
 # (c) 2006 Mark Kretschmann <markey@web.de>
 # (c) 2006 Michael Fellinger <manveru@weez-int.com>
 # (c) 2006 Ian Monroe <ian@monroe.nu>
+# (c) 2006 Martin Ellis <martin.ellis@kdemail.net>
+# (c) 2006 Alexandre Oliveira <aleprj@gmail.net>
 #
 # License: GNU General Public License V2
 
@@ -21,7 +23,7 @@
 class Proxy
   ENDL = "\r\n"
 
-  def initialize( port, remote_url, engine )
+  def initialize( port, remote_url, engine, proxy )
     @engine = engine
 
     myputs( "running with port: #{port} and url: #{remote_url} and engine: #{engine}" )
@@ -40,7 +42,15 @@
     uri = URI.parse( remote_url )
     myputs("host " << uri.host << " ")
     myputs( port )
-    serv = TCPSocket.new( uri.host, uri.port )
+
+    #Check for proxy
+    proxy_uri = URI.parse( proxy )
+    if ( proxy_uri.class != URI::Generic )
+      serv = TCPSocket.new( proxy_uri.host, proxy_uri.port )
+    else
+      serv = TCPSocket.new( uri.host, uri.port )
+    end
+
     serv.sync = true
     myputs( "running with port: #{uri.port} and host: #{uri.host}" )
 
@@ -156,10 +166,10 @@
 end
 
 class DaapProxy < Proxy
-  def initialize( port, remote_url, engine, hash, request_id )
+  def initialize( port, remote_url, engine, hash, request_id, proxy )
     @hash = hash
     @requestId = request_id
-    super( port, remote_url, engine )
+    super( port, remote_url, engine, proxy )
   end
 
   def get_request( remote_uri )
@@ -182,11 +192,11 @@
 begin
   myputs( ARGV )
   if( ARGV[0] == "--lastfm" ) then
-    option, port, remote_url, engine = ARGV
-    LastFM.new( port, remote_url, engine )
+    option, port, remote_url, engine, proxy = ARGV
+    LastFM.new( port, remote_url, engine, proxy )
   else
-    option, port, remote_url, engine, hash, request_id = ARGV
-    DaapProxy.new( port, remote_url, engine, hash, request_id )
+    option, port, remote_url, engine, hash, request_id, proxy = ARGV
+    DaapProxy.new( port, remote_url, engine, hash, request_id, proxy )
   end
 rescue
   myputs( $!.to_s )
--- trunk/extragear/multimedia/amarok/src/lastfm.cpp #581320:581321
@@ -46,6 +46,87 @@
 using namespace LastFm;
 
 ///////////////////////////////////////////////////////////////////////////////
+// CLASS AmarokHttp
+// AmarokHttp is a hack written so that lastfm code could easily use something proxy aware.
+// DO NOT use this class for anything else, use KIO directly instead.
+////////////////////////////////////////////////////////////////////////////////
+AmarokHttp::AmarokHttp ( const QString& hostname, Q_UINT16 port,
+                         QObject* parent )
+    : QObject( parent ),
+      m_hostname( hostname ),
+      m_port( port )
+{}
+
+int
+AmarokHttp::get ( const QString & path )
+{
+    QString uri = QString( "http://%1:%2/%3" )
+                  .arg( m_hostname )
+                  .arg( m_port )
+                  .arg( path );
+
+    m_done = false;
+    m_error = QHttp::NoError;
+    m_state = QHttp::Connecting;
+    KIO::TransferJob *job = KIO::get(uri, true, false);
+    connect(job,  SIGNAL(data(KIO::Job*, const QByteArray&)),
+            this, SLOT(slotData(KIO::Job*, const QByteArray&)));
+    connect(job,  SIGNAL(result(KIO::Job*)),
+            this, SLOT(slotResult(KIO::Job*)));
+
+    return 0;
+}
+
+QHttp::State
+AmarokHttp::state() const
+{
+    return m_state;
+}
+
+QByteArray
+AmarokHttp::readAll ()
+{
+    return m_result;
+}
+
+QHttp::Error
+AmarokHttp::error()
+{
+    return m_error;
+}
+
+void
+AmarokHttp::slotData(KIO::Job*, const QByteArray& data)
+{
+    if( data.size() == 0 ) {
+        return;
+    }
+    else if ( m_result.size() == 0 ) {
+        m_result = data;
+    }
+    else if ( m_result.resize( m_result.size() + data.size() ) ) {
+        memcpy( m_result.end(), data.data(),  data.size() );
+    }
+}
+
+void
+AmarokHttp::slotResult(KIO::Job* job)
+{
+    bool err = job->error();
+    if( err || m_error != QHttp::NoError ) {
+        m_error = QHttp::UnknownError;
+    }
+    else {
+        m_error = QHttp::NoError;
+    }
+    m_done = true;
+    m_state = QHttp::Unconnected;
+    emit( requestFinished( 0, err ) );
+}
+
+
+
+///////////////////////////////////////////////////////////////////////////////
 // CLASS Controller
 ////////////////////////////////////////////////////////////////////////////////
 
@@ -312,7 +393,7 @@
     m_username = username;
     m_password = password;
 
-    QHttp http( "ws.audioscrobbler.com", 80 );
+    AmarokHttp http( "ws.audioscrobbler.com", 80 );
 
     const QString path =
             QString( "/radio/handshake.php?version=%1&platform=%2&username=%3&passwordmd5=%4&debug=%5" )
@@ -362,6 +443,7 @@
     *m_server << QString::number( port );
     *m_server << m_streamUrl.toString();
     *m_server << AmarokConfig::soundSystem();
+    *m_server << Amarok::proxyForUrl( m_streamUrl.toString() );
 
     if( !m_server->start( KProcIO::NotifyOnExit, true ) ) {
         error() << "Failed to start amarok_proxy.rb" << endl;
@@ -387,7 +469,7 @@
 {
     debug() << "Changing station:" << url << endl;
 
-    QHttp http( m_baseHost, 80 );
+    AmarokHttp http( m_baseHost, 80 );
 
     http.get( QString( m_basePath + "/adjust.php?session=%1&url=%2&debug=0" )
              .arg( m_session )
@@ -427,7 +509,7 @@
 void
 WebService::requestMetaData() //SLOT
 {
-    QHttp *http = new QHttp( m_baseHost, 80, this );
+    AmarokHttp *http = new AmarokHttp( m_baseHost, 80, this );
     connect( http, SIGNAL( requestFinished( int, bool ) ), this, SLOT( metaDataFinished( int, bool ) ) );
 
     http->get( QString( m_basePath + "/np.php?session=%1&debug=%2" )
@@ -441,7 +523,7 @@
 {
     DEBUG_BLOCK
 
-    QHttp* http = (QHttp*) sender();
+    AmarokHttp* http = (AmarokHttp*) sender();
     http->deleteLater();
     if( error ) return;
 
@@ -514,7 +596,7 @@
     else
         debug() << "Disabling Scrobbling!" << endl;
 
-    QHttp *http = new QHttp( m_baseHost, 80, this );
+    AmarokHttp *http = new AmarokHttp( m_baseHost, 80, this );
     connect( http, SIGNAL( requestFinished( int, bool ) ), this, SLOT( enableScrobblingFinished( int, bool ) ) );
 
     http->get( QString( m_basePath + "/control.php?session=%1&command=%2&debug=%3" )
@@ -527,7 +609,7 @@
 void
 WebService::enableScrobblingFinished( int /*id*/, bool error ) //SLOT
 {
-    QHttp* http = (QHttp*) sender();
+    AmarokHttp* http = (AmarokHttp*) sender();
     http->deleteLater();
     if ( error ) return;
 
@@ -538,7 +620,7 @@
 void
 WebService::love() //SLOT
 {
-    QHttp *http = new QHttp( m_baseHost, 80, this );
+    AmarokHttp *http = new AmarokHttp( m_baseHost, 80, this );
     connect( http, SIGNAL( requestFinished( int, bool ) ), this, SLOT( loveFinished( int, bool ) ) );
 
     http->get( QString( m_basePath + "/control.php?session=%1&command=love&debug=%2" )
@@ -551,7 +633,7 @@
 void
 WebService::skip() //SLOT
 {
-    QHttp *http = new QHttp( m_baseHost, 80, this );
+    AmarokHttp *http = new AmarokHttp( m_baseHost, 80, this );
     connect( http, SIGNAL( requestFinished( int, bool ) ), this, SLOT( skipFinished( int, bool ) ) );
 
     http->get( QString( m_basePath + "/control.php?session=%1&command=skip&debug=%2" )
@@ -564,7 +646,7 @@
 void
 WebService::ban() //SLOT
 {
-    QHttp *http = new QHttp( m_baseHost, 80, this );
+    AmarokHttp *http = new AmarokHttp( m_baseHost, 80, this );
     connect( http, SIGNAL( requestFinished( int, bool ) ), this, SLOT( banFinished( int, bool ) ) );
 
     http->get( QString( m_basePath + "/control.php?session=%1&command=ban&debug=%2" )
@@ -579,7 +661,7 @@
 {
     DEBUG_BLOCK
 
-    QHttp* http = (QHttp*) sender();
+    AmarokHttp* http = (AmarokHttp*) sender();
     http->deleteLater();
     if( error ) return;
 
@@ -592,7 +674,7 @@
 {
     DEBUG_BLOCK
 
-    QHttp* http = (QHttp*) sender();
+    AmarokHttp* http = (AmarokHttp*) sender();
     http->deleteLater();
     if( error ) return;
 
@@ -606,7 +688,7 @@
 {
     DEBUG_BLOCK
 
-    QHttp* http = (QHttp*) sender();
+    AmarokHttp* http = (AmarokHttp*) sender();
     http->deleteLater();
     if( error ) return;
 
@@ -622,7 +704,7 @@
     if ( username.isEmpty() )
         username = m_username;
 
-    QHttp *http = new QHttp( m_baseHost, 80, this );
+    AmarokHttp *http = new AmarokHttp( m_baseHost, 80, this );
     connect( http, SIGNAL( requestFinished( bool ) ), this, SLOT( friendsFinished( bool ) ) );
 
     http->get( QString( "/1.0/user/%1/friends.xml" )
@@ -633,7 +715,7 @@
 void
 WebService::friendsFinished( int /*id*/, bool error ) //SLOT
 {
-    QHttp* http = (QHttp*) sender();
+    AmarokHttp* http = (AmarokHttp*) sender();
     http->deleteLater();
     if( error ) return;
 
@@ -664,7 +746,7 @@
     if ( username.isEmpty() )
         username = m_username;
 
-    QHttp *http = new QHttp( m_baseHost, 80, this );
+    AmarokHttp *http = new AmarokHttp( m_baseHost, 80, this );
     connect( http, SIGNAL( requestFinished( bool ) ), this, SLOT( neighboursFinished( bool ) ) );
 
     http->get( QString( "/1.0/user/%1/neighbours.xml" )
@@ -675,7 +757,7 @@
 void
 WebService::neighboursFinished( int /*id*/, bool error ) //SLOT
 {
-    QHttp* http = (QHttp*) sender();
+    AmarokHttp* http = (AmarokHttp*) sender();
     http->deleteLater();
     if( error )  return;
 
@@ -706,7 +788,7 @@
     if ( username.isEmpty() )
         username = m_username;
 
-    QHttp *http = new QHttp( m_baseHost, 80, this );
+    AmarokHttp *http = new AmarokHttp( m_baseHost, 80, this );
     connect( http, SIGNAL( requestFinished( bool ) ), this, SLOT( userTagsFinished( bool ) ) );
 
     http->get( QString( "/1.0/user/%1/tags.xml?debug=%2" )
@@ -717,7 +799,7 @@
 void
 WebService::userTagsFinished( int /*id*/, bool error ) //SLOT
 {
-    QHttp* http = (QHttp*) sender();
+    AmarokHttp* http = (AmarokHttp*) sender();
     http->deleteLater();
     if( error ) return;
 
@@ -748,7 +830,7 @@
     if ( username.isEmpty() )
         username = m_username;
 
-    QHttp *http = new QHttp( m_baseHost, 80, this );
+    AmarokHttp *http = new AmarokHttp( m_baseHost, 80, this );
     connect( http, SIGNAL( requestFinished( bool ) ), this, SLOT( recentTracksFinished( bool ) ) );
 
     http->get( QString( "/1.0/user/%1/recenttracks.xml" )
@@ -759,7 +841,7 @@
 void
 WebService::recentTracksFinished( int /*id*/, bool error ) //SLOT
 {
-    QHttp* http = (QHttp*) sender();
+    AmarokHttp* http = (AmarokHttp*) sender();
     http->deleteLater();
     if( error ) return;
 
@@ -834,7 +916,7 @@
 void
 WebService::recommendFinished( int /*id*/, bool /*error*/ ) //SLOT
 {
-    QHttp* http = (QHttp*) sender();
+    AmarokHttp* http = (AmarokHttp*) sender();
     http->deleteLater();
 
     debug() << "Recommendation:" << http->readAll() << endl;
--- trunk/extragear/multimedia/amarok/src/lastfm.h #581320:581321
@@ -19,6 +19,7 @@
 
 #include "metabundle.h"
 
+#include <qhttp.h>
 #include <qobject.h>
 #include <qserversocket.h>
 #include <qurl.h>
@@ -35,6 +36,37 @@
 
 namespace KIO { class Job; }
 
+/* AmarokHttp is a hack written so that lastfm code could easily use something proxy aware.
+   DO NOT use this class for anything else, use KIO directly instead. */
+class AmarokHttp : public QObject
+{
+    Q_OBJECT
+
+    public:
+    AmarokHttp ( const QString & hostname, Q_UINT16 port = 80, QObject* parent = 0 );
+    int get ( const QString & path );
+    QHttp::State state() const;
+    QByteArray readAll ();
+    QHttp::Error error();
+
+    signals:
+    void requestFinished ( int id, bool error );
+
+    protected slots:
+    void slotData(KIO::Job*, const QByteArray& );
+    void slotResult(KIO::Job*);
+
+    protected:
+    QString m_hostname;
+    Q_UINT16 m_port;
+    QString  m_path;
+    QHttp::State m_state;
+    QHttp::Error m_error;
+    bool m_done;
+    QByteArray m_result;
+};
+
+
 namespace LastFm
 {
     class WebService;
--- trunk/extragear/multimedia/amarok/src/mediadevice/daap/proxy.cpp #581320:581321
@@ -71,6 +71,7 @@
     *m_proxy << AmarokConfig::soundSystem();
     *m_proxy << hash;
     *m_proxy << QString::number( revisionId );
+    *m_proxy << Amarok::proxyForUrl( realStream.url() );
 
     if( !m_proxy->start( KProcIO::NotifyOnExit, true ) ) {
         error() << "Failed to start amarok_proxy.rb" << endl;
Comment 17 Martin Ellis 2006-09-06 19:05:04 UTC
On Wednesday 06 September 2006 03:36, Alexandre Oliveira wrote:
> I tried hard not to break DAAP (what might
> even work with proxies as well, as soon as eean makes the necessary changes
> to his code, 


Yeah, my patch didn't bother with the DAAP code.

Unless I'm missing something about DAAP, I can't imagine there being much 
demand for using DAAP with a proxy.

> +// AmarokHttp is a hack written so that lastfm code could easily use
> something proxy aware. 
> +// DO NOT use this class for anything else, use KIO directly instead.


I concur.

Putting this in lastfm.{h,cpp} seems like a good idea to prevent further use.   
It was only in a separate file was to make it easier to delete when a decent 
fix is ready.
Comment 18 simon 2006-09-06 19:41:32 UTC
Hi there, i'm having problems to get this stuff working. So far i tried the patch by Martin Ellis 
and today normal svn r581425.

When i try to play anything (neighbor radio or global tags) i get this:

Error Loading Media
No suitable input plugin. This often means that the url's protocol is not supported. Network failures are other possible causes.
http://localhost:47249/lastfm.mp3

I've tried with the proxy entered in xine and without as Martin suggested. But it made no differnce.
If i use "netstat -l" it  shows:
tcp        0      0 *:54179                 *:*                     LISTEN

corresponding to ps -ef:
myuser    17435 21436  0 19:23 ?        00:00:00 ruby /usr/bin/amarok_proxy.rb --lastfm 54179 http://streamer2.last.fm:80/last.mp3?Session=405a9b821ed177977fd8b64f94a1a753 xine-engine http://myproxy:80

which seems ok to me. But i also had no luck to connect to this adress with xine/mplayer/xmms directly when using http://127.0.0.1:54179/lastfm.mp3

The different ports in my examples are from different tests.

Can someone give me any hint how/where to look for the problem ?
The http-proxy worked fine while i tested this.

Thx s|mon
Comment 19 Ian Monroe 2006-09-06 19:47:00 UTC
Can you play mp3s simon?
Comment 20 Martin Ellis 2006-09-06 19:55:35 UTC
On Wednesday 06 September 2006 18:41, simon wrote:
> Hi there, i'm having problems to get this stuff working. So far i tried the
> patch by Martin Ellis and today normal svn r581425.


I'd be very surprised if my patch worked with todays svn.  I'm surprised you 
could even apply it to current svn, to be honest.

Alexandre Oliveira commited some fixes earlier, which will hopefully make 
last.fm work without my separate patch.  I have not tested this.

My patch has only been tested against svn as of about the 29th August.
That's around revision 578312.

Martin
Comment 21 simon 2006-09-06 20:04:12 UTC
No you misunderstood me there. I applied your patch to a version before r580587 (because you mentioned the problem above) and todays svn - in both cases i had no luck.
Comment 22 Martin Ellis 2006-09-06 20:12:25 UTC
On Wednesday 06 September 2006 19:04, simon wrote:
> ------- Additional Comments From s7mon web de  2006-09-06 20:04 -------
> No you misunderstood me there. I applied your patch to a version before
> r580587 (because you mentioned the problem above) and todays svn - in both
> cases i had no luck.


The only way I can reproduce the error you are getting is if I forget to type
"unset http_proxy" at the bash prompt before I run amarok from the same 
prompt.

Now I just have the "unset http_proxy ; amarok" in my bash history.

I assume you can play local mp3s OK.

Martin