Bug 135147

Summary: Unable to download certain podcasts
Product: [Applications] amarok Reporter: Edwin Lee <edwin11_79>
Component: generalAssignee: Bart Cerneels <shanachie>
Status: RESOLVED FIXED    
Severity: normal CC: amarok-bugs-dist
Priority: NOR    
Version: 1.4.3   
Target Milestone: ---   
Platform: Ubuntu   
OS: Linux   
Latest Commit: Version Fixed In:
Sentry Crash Report:

Description Edwin Lee 2006-10-05 15:54:43 UTC
Version:           1.4.3 (using KDE KDE 3.5.2)
Installed from:    Ubuntu Packages
OS:                Linux

i am facing some issues downloading the episodes of some podcasts using the Right-Click -> "Download Media" method. i will post two examples here.


Example 1:

RSS: http://leoville.tv/podcasts/twit.xml
Episode URL: http://www.podtrac.com/pts/redirect.mp3?http://aolradio.podcast.aol.com/twit/TWiT0071H.mp3
(Episode: TWiT 71: Netcast Expo)

When i try to download an episode, it tells me
"Media download aborted, unable to connect to server."

and a 0 KB "redirect.mp3" file is placed in the directory where the podcast would have been downloaded to.

However, when i use gwget with the episode URL, i could download the mp3.


Example 2:

RSS: http://feeds.feedburner.com/thebeautifulgamemp3
Episode URL: http://feeds.feedburner.com/~r/thebeautifulgamemp3/~5/31237663/Episode7.mp3

(Episode: Episode 7)
When i try to download an episode, it also tells me
"Media download aborted, unable to connect to server."

and a 0 KB "Episode7.mp3" file is placed in the directory where the podcast would have been downloaded to.

However, when i use gwget with the episode URL, i could download the mp3.


i'm not sure if these two cases are related. The first case seems like a case of not handling the redirect correctly (though i do notice a previous redirect bug that was already fixed). The second case doesn't seem to involve redirects at all.
Comment 1 Bart Cerneels 2006-10-27 22:47:26 UTC
DEVINFO: Issue with PodcastFetcher, not shure how.
Comment 2 Bart Cerneels 2006-12-03 18:57:05 UTC
SVN commit 610189 by shanachie:

Use KIO::StoredGet instead of PodcastFetcher. Podcastfetcher was originally developed to work around a KDELibs bug. This seems to be fixed since at least KDE 3.5.0 but possibly a lot earlier. If problems with weird filenames show up, look at the KDELibs version. If lots of users are affected we might have to use PodcastFetcher for those older KDELibs.
Lot's of possible enhancements: resuming downloads, reducing memory consumption (not StoredGet), MIME type checking....

BUG:135147


 M  +32 -186   playlistbrowseritem.cpp  
 M  +5 -34     playlistbrowseritem.h  
 M  +0 -1      statusbar/progressBar.cpp  


--- trunk/extragear/multimedia/amarok/src/playlistbrowseritem.cpp #610188:610189
@@ -2317,165 +2317,7 @@
     }
 }
 
-
 /////////////////////////////////////////////////////////////////////////////
-///    CLASS PodcastFetcher
-///    @note Allows us to download podcasts from any enclosure, even the weird ones, with decent filenames..
-////////////////////////////////////////////////////////////////////////////
-
-PodcastFetcher::PodcastFetcher( QString url, const KURL &directory, int size ):
-        m_url( QUrl( url )),
-        m_directory ( directory ),
-        m_error( 0 ),
-        m_size( size )
-{
-
-    m_redirected = false;
-
-    QString proxy = Amarok::proxyForUrl( m_url );
-    if ( proxy.isNull() )
-        m_http = new QHttp( m_url.host() );
-    else {
-        QUrl proxyUrl( proxy );
-        m_http = new QHttp( proxyUrl.host(), proxyUrl.port() );
-    }
-
-    connect( (m_http), SIGNAL( responseHeaderReceived ( const QHttpResponseHeader & ) ), this,
-                      SLOT( slotResponseReceived( const QHttpResponseHeader & ) ) );
-    connect( (m_http), SIGNAL( done( bool ) ), this, SLOT( slotDone( bool ) ) );
-   // connect( m_http, SIGNAL( dataTransferProgress ( int, int, QNetworkOperation * ) ), this, SLOT( slotProgress( int, int ) ) );
-    connect( (m_http), SIGNAL(  dataReadProgress ( int, int ) ), this, SLOT( slotProgress( int, int ) ) );
-
-    fetch( );
-}
-
-PodcastFetcher::~PodcastFetcher()
-{
-    delete m_http;
-    m_http = 0;
-}
-
-void PodcastFetcher::fetch()
-{
-    if( !m_http )
-        return;
-    KURL filepath = m_directory;
-
-    filepath.addPath( m_url.fileName() );
-    m_file.setName( filepath.path() );
-    if( m_file.exists() )
-    {
-        QFileInfo file( m_file );
-        const QString baseName = file.fileName();
-        int i = 1;
-        while( file.exists() )
-        {
-            QString newName = baseName;
-            QString ext = file.extension();
-            //Insert number right before the extension: podcast.mp3 > podcast_1.mp3
-            int index = newName.findRev( ext, -1, true );
-            newName.insert( index-1, '_'+QString::number(i) );
-
-            file.setFile( file.dirPath( true ) + '/' + newName );
-            i++;
-        }
-        m_file.setName( file.filePath() );
-    }
-
-    // Qhttp::get() "conviniently" "corrects" the path in a way that wouldn't let it work
-    // work with proxies. So let's create the request manually.
-    QHttpRequestHeader request( "GET", m_url );
-    QString host = m_url.host();
-    if ( m_url.port() > 0 ) {
-        host += ':' + QString::number( m_url.port() );
-    }
-    request.setValue( "Host", host );
-    request.setValue( "Connection", "Keep-Alive" );
-    //debug() << request.toString() << endl;
-    m_http->request( request, 0, (&m_file) );
-
-    if( m_http->error() )
-        debug() <<  m_http->errorString() << endl;
-}
-
-void PodcastFetcher::kill()
-{
-    if ( m_http )
-    {
-        m_http->abort();
-        m_http->clearPendingRequests();
-        m_http->closeConnection();
-        if( m_file.exists() )
-            m_file.remove();
-    }
-}
-
-void PodcastFetcher::slotProgress( int bytesDone, int bytesTotal )
-{
-    int percent = 0;
-    if( bytesTotal != 0 )
-        percent = (bytesDone * 100) / bytesTotal;
-    //debug() << "Progress: " << percent << " bytesTotal: " << bytesTotal << endl;
-    emit progress( this, percent );
-}
-
-void PodcastFetcher::slotResponseReceived( const QHttpResponseHeader & resp )
-{
-    //debug() << m_http->currentId() << " RESPONCE, statuscode = " << resp.statusCode() << endl;
-    //debug() << resp.toString() << endl;
-    if( resp.statusCode() == 302 || resp.statusCode() == 301 )
-    {
-        if (resp.hasKey( "location" ) )
-        {
-            QString oldHost = m_url.host();
-            m_url = QUrl( resp.value( "location" ) );
-            //prevent crashing when redirected to host-only url (like www.michaelandevo.com)
-            if( m_url.fileName().isNull() )
-            {
-                m_error = QHttp::InvalidResponseHeader;
-                return;
-            }
-            //debug() << m_http->currentId() << " m_redirected to " << m_url.toString( ) <<endl;
-            if( m_http && (m_url.host() != oldHost) )
-                m_http->setHost( m_url.host() );
-            m_redirected = true;
-        }
-    } else if (resp.statusCode() == 200 )
-    {
-        //TODO: create file here, rename temp file later
-        //debug() << resp.toString() << endl;
-    }
-}
-
-void PodcastFetcher::slotDone( bool error )
-{
-    if( error )
-    {
-            //debug() << m_http->currentId() << " ERROR: " << " errorstring = " << m_http->errorString() << endl;
-            emit result( m_http->error() );
-            return;
-    }
-    if( m_error )
-    {
-        //debug() << m_http->currentId() << " ERROR: m_error = " << m_error << endl;
-        emit result( m_error );
-        return;
-    }
-    if ( m_redirected )
-    {
-        m_redirected = false;
-        if( m_file.exists() )
-            m_file.remove();
-        fetch();
-    }
-    else if ( !error )
-    {
-        //debug() << m_http->currentId() << " downloaded to " << m_file.name() << endl;
-        emit result( m_http->error() ); //0
-    }
-}
-
-/////////////////////////////////////////////////////////////////////////////
 ///    CLASS PodcastEpisode
 ///    @note we fucking hate itunes for taking over podcasts and inserting
 ///          their own attributes.
@@ -2681,19 +2523,30 @@
             m_localDir = PodcastSettings("Podcasts").saveLocation();
     createLocalDir( m_localDir );
 
-    m_podcastFetcher = new PodcastFetcher( url().url() , m_localDir, m_bundle.size() );
+    //filename might get changed by redirects later.
+    m_filename = url().fileName();
+    m_localUrl = m_localDir;
+    m_podcastEpisodeJob = KIO::storedGet( url().url(), false, false);
 
-    //TODO: make this work with PodcastFetcher
-    Amarok::StatusBar::instance()->newProgressOperation( m_podcastFetcher )
+    Amarok::StatusBar::instance()->newProgressOperation( m_podcastEpisodeJob )
             .setDescription( title().isEmpty()
                     ? i18n( "Downloading Podcast Media" )
                     : i18n( "Downloading Podcast \"%1\"" ).arg( title() ) )
             .setAbortSlot( this, SLOT( abortDownload()) )
-            .setProgressSignal( m_podcastFetcher, SIGNAL( progress( const QObject*, int ) ) );
+            .setProgressSignal( m_podcastEpisodeJob, SIGNAL( percent( KIO::Job *, unsigned long ) ) );
 
-    connect( m_podcastFetcher, SIGNAL( result( int ) ), SLOT( downloadResult( int ) ) );
+    connect( m_podcastEpisodeJob, SIGNAL(  result( KIO::Job * ) ), SLOT( downloadResult( KIO::Job * ) ) );
+    connect( m_podcastEpisodeJob, SIGNAL( redirection( KIO::Job *,const KURL& ) ), SLOT( redirected( KIO::Job *,const KURL& ) ) );
 }
 
+/* change the localurl if redirected, allows us to use the original filename to transfer to mediadevices*/
+void PodcastEpisode::redirected( KIO::Job * job, const KURL & redirectedUrl )
+{
+    DEBUG_BLOCK
+    debug() << "redirecting to " << redirectedUrl << ". filename: " << redirectedUrl.fileName() << endl;
+    m_filename = redirectedUrl.fileName();
+}
+
 void PodcastEpisode::createLocalDir( const KURL &localDir )
 {
     if( localDir.isEmpty() ) return;
@@ -2712,8 +2565,8 @@
 PodcastEpisode::abortDownload() //SLOT
 {
     emit downloadAborted();
-    if ( m_podcastFetcher )
-        m_podcastFetcher->kill();
+    if( m_podcastEpisodeJob )
+        m_podcastEpisodeJob->kill( false );
 
     //don't delete m_podcastFetcher yet, kill() is async
     stopAnimation();
@@ -2722,36 +2575,32 @@
     updatePixmap();
 }
 
-void
-PodcastEpisode::downloadResult( int error ) //SLOT
+void PodcastEpisode::downloadResult( KIO::Job * transferJob )
 {
-    //gets called after PodcastFetcher::kill()
-    if( error == QHttp::Aborted )
-    {
-        m_podcastFetcher->deleteLater();
-        m_podcastFetcher = 0;
-        return;
-    }
+    DEBUG_BLOCK
     emit downloadFinished();
-
     stopAnimation();
     setText( 0, title() );
 
-    if ( error != 0 ) {
+    if( transferJob->error() )
+    {
         Amarok::StatusBar::instance()->shortMessage( i18n( "Media download aborted, unable to connect to server." ) );
-        debug() << "Unable to retrieve podcast media. KIO Error: " << error << endl;
+        debug() << "Unable to retrieve podcast media. KIO Error: " << transferJob->error() << endl;
 
         setPixmap( 0, SmallIcon("cancel") );
-
-        m_podcastFetcher->deleteLater();
-        m_podcastFetcher = 0;
         return;
     }
 
-    m_onDisk = true;
+    m_localUrl.addPath( m_filename );
+    debug() << "filename: " << m_localUrl.path() << endl;
+    QFile *localFile = new QFile( m_localUrl.path() );
+    localFile->open( IO_WriteOnly );
+    localFile->writeBlock( m_podcastEpisodeJob->data() );
+    localFile->close();
 
-    setLocalUrl( m_podcastFetcher->localUrl() );
-
+    m_bundle.setLocalURL( m_localUrl );
+    CollectionDB::instance()->updatePodcastEpisode( dBId(), m_bundle );
+    m_onDisk = true;
     PodcastChannel *channel = dynamic_cast<PodcastChannel *>( m_parent );
     if( channel && channel->autotransfer() && MediaBrowser::isAvailable() )
     {
@@ -2760,10 +2609,7 @@
     }
 
     updatePixmap();
-    m_podcastFetcher->deleteLater();
-    m_podcastFetcher = 0;
 }
-
 void
 PodcastEpisode::setLocalUrl( const KURL &localUrl )
 {
--- trunk/extragear/multimedia/amarok/src/playlistbrowseritem.h #610188:610189
@@ -15,6 +15,7 @@
 #include "podcastsettings.h"
 
 #include <kdialogbase.h> // StreamEditor baseclass
+#include <kio/job.h>
 #include <klineedit.h>
 #include <klistview.h>
 #include <kurl.h>
@@ -249,38 +250,6 @@
         TrackItemInfo *m_trackInfo;
 };
 
-class PodcastFetcher : public QObject
-{
-    Q_OBJECT
-    public:
-        PodcastFetcher( QString url, const KURL &directory, int size );
-        ~PodcastFetcher();
-        QString filename() const { return m_url.fileName(); }
-        KURL    localUrl() const { return KURL::fromPathOrURL( m_file.name() ); }
-        void    kill();
-
-    signals:
-        void result( int error );
-        void progress( const QObject* thisObject, int steps );
-
-    private slots:
-        void slotResponseReceived( const QHttpResponseHeader & resp );
-        void slotDone( bool error );
-        void slotProgress( int bytesDone, int bytesTotal );
-
-    private:
-        void    fetch();
-
-        QFile   m_file;
-        QUrl    m_url;
-        QHttp  *m_http;
-        KURL    m_directory;
-        bool    m_redirected;
-        int     m_error;
-        int     m_size;
-
-};
-
 /// Stored in the database
 class PodcastEpisode : public PlaylistBrowserEntry
 {
@@ -338,8 +307,9 @@
 
     private slots:
         void abortDownload();
-        void downloadResult( int error );
+        void downloadResult( KIO::Job * transferJob );
         void slotAnimation();
+        void redirected( KIO::Job * job,const KURL & redirectedUrl );
 
     private:
         enum FeedType{ RSS=0, ATOM=1 };
@@ -360,7 +330,8 @@
         QTimer      m_animationTimer;
         uint        m_iconCounter;
 
-        PodcastFetcher  *m_podcastFetcher;
+        KIO::StoredTransferJob* m_podcastEpisodeJob;
+        QString m_filename;
 
         bool        m_downloaded;       //marked as downloaded in cached xml
         bool        m_onDisk;
--- trunk/extragear/multimedia/amarok/src/statusbar/progressBar.cpp #610188:610189
@@ -90,7 +90,6 @@
 ProgressBar::setProgressSignal( QObject *sender, const char *signal )
 {
     setTotalSteps( 100 );
-    debug() << "connecting " << signal << " LOOKATME\n";
     connect( sender, signal, Amarok::StatusBar::instance(), SLOT( setProgress ( const QObject*, int ) ) );
     return *this;
 }