Bug 195704

Bart, is this still valid?

It's not in 2.2-GIT yet and I think it's unlikely *real* podcast syncing can be added before the feature freeze next Friday.

I will however talk to Alejando to see if we can make basic transfer working.

*** This bug has been confirmed by popular vote. ***

(In reply to comment #2)
> It's not in 2.2-GIT yet and I think it's unlikely *real* podcast syncing can be
> added before the feature freeze next Friday.
> 
> I will however talk to Alejando to see if we can make basic transfer working.

Is there progress on Podcast + iPod support in 2.2.1 or current git?

AFAIK is Alejandro very busy atm and I think the IPOD support needs more time. I'm trying to understand the whole IPOD Code but its hard with low programming skills. If I get through the code maybe I can do something for better IPOD support (not only Podcasts but submitting played tracks to last.fm and syncing the ratings and so on). But I have little time too and cant promise that I can even do anything what will really help.

I tried a little on my own and pushed that into http://gitorious.org/~mab/amarok/mabs-amarok/commits/podcast-ipod-support commit b95d125.

The podcast is copied as a normal music file not as a podcast, but it works at least.

So it's been six months since this bug was filed.  If I can ask without irritating anyone...  Does anyone have any idea when this might be fixed?  I only use amarok for syncing podcasts.

(In reply to comment #7)
> So it's been six months since this bug was filed.  If I can ask without
> irritating anyone...  Does anyone have any idea when this might be fixed?  I
> only use amarok for syncing podcasts.

A patch has been committed, you just need to wait for Amarok 2.2.2 to be released.

(In reply to comment #8)
> (In reply to comment #7)
> > So it's been six months since this bug was filed.  If I can ask without
> > irritating anyone...  Does anyone have any idea when this might be fixed?  I
> > only use amarok for syncing podcasts.
> 
> A patch has been committed, you just need to wait for Amarok 2.2.2 to be
> released.

If the attached patch is indeed committed it should be reverted. It's not the proper way to transfer podcasts.

A real implementation is being developed in a branch on my gitorious clone http://gitorious.org/~Stecchino/amarok/stecchinos-amarok/commits/playlist_sync .

I didn't try that patch yet but as far as I undestood it is a workaround for transferring podcasts as "music". Until your solution is finished, we should stay with the workaround. Just my 2 cants

(In reply to comment #10)
> I didn't try that patch yet but as far as I undestood it is a workaround for
> transferring podcasts as "music". Until your solution is finished, we should
> stay with the workaround. Just my 2 cants

Thank you for that useless comment.

If you compile from source you can apply it yourself. The patch came to late to include it in 2.2.2 because of string and feature freeze. Even though it's not ideal I would have included it otherwise.
The next version will have a real implementation.

(In reply to comment #11)
> Thank you for that useless comment.
> 

Dude, if you found it useless, please dont thank me for it!

Maybe you misunderstood my comment. I just wanted to give you an answer to your question. Myriam told you that there was a patch. Which you wanted to be reverted such a short time before 2.2.2 will be released. Plus: You said that your solution is "being developed", means not finished, right? And I just wanted to say that we should wait for 2.2.3 to implement your solution instead of taking the current patch away and put it in a day before release. I really dont know why you called that useless.

> The patch came to late to
> include it in 2.2.2 because of string and feature freeze.

What are you saying?

1. The patch was posted here on december 13th
2. Miryam said that it will be in 2.2.2 (comment #8)

So it seems not to came too late.

Jochen, this is my bad, I thought it was before the freeze, but since we are in string and feature freeze since December 1st, the patch came indeed too late to be in 2.2.2.

Ah ok, then it is clear and bart and me misunderstood each other. I only wanted to make that up cause I felt a bit mistreaten by Bart. Thank you for the enlightenment.

Jochen

Hi all,

on that branch I made also changes which make it possible to copy them into the Podcast collection. It would be nice, if you could have a look into it, Bart.

I would like to see this in 2.2.3. Either with my patch or yours.

Markus

Will this patch include in amarok 2.3 beta because I do not see this feature in feature list of amarok 2.3 beta 1 (http://amarok.kde.org/en/releases/2.3/beta/1). Is it an oversight or other one?

(In reply to comment #16)
> Will this patch include in amarok 2.3 beta because I do not see this feature in
> feature list of amarok 2.3 beta 1
> (http://amarok.kde.org/en/releases/2.3/beta/1). Is it an oversight or other
> one?

Since this report is still open it didn't end up in the beta version. Let's hope the developers get around to the merge request. Bart, any news on that?

I've got it almost implemented for UMS devices. The iPod implementation proved to be beyond my reach for this version.

commit b9baab10ba3a3a4b0fca09cad4fdad9583c6ec9e
Author: Bart Cerneels <bart.cerneels@kde.org>
Date:   Thu Feb 25 17:52:22 2010 +0100

    Enable dropping episodes on provider.
    
    This completes the implementation of one of the most missed features: copying podcasts to mediadevices.
    
    Only works for UMS at the moment.
    CCBUG:195704

diff --git a/ChangeLog b/ChangeLog
index 6ae0001..aee5ce7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -5,6 +5,8 @@ Amarok ChangeLog
 
 VERSION 2.3
   FEATURES:
+    * Podcast channels and episodes can be dragged to add them to other
+      providers. (BR 195704)
     * Trackaction buttons are now available in the label for the current track
     * Bookmark button is now available in the Current Track applet.
 
diff --git a/src/browsers/playlistbrowser/PlaylistsByProviderProxy.cpp b/src/browsers/playlistbrowser/PlaylistsByProviderProxy.cpp
index edca217..cb567be 100644
--- a/src/browsers/playlistbrowser/PlaylistsByProviderProxy.cpp
+++ b/src/browsers/playlistbrowser/PlaylistsByProviderProxy.cpp
@@ -56,7 +56,6 @@ PlaylistsByProviderProxy::mimeData( const QModelIndexList &indexes ) const
 {
     DEBUG_BLOCK
     QModelIndexList sourceIndexes;
-    QList<int> originalRows;
     foreach( const QModelIndex &idx, indexes )
     {
         debug() << idx;
@@ -70,10 +69,7 @@ PlaylistsByProviderProxy::mimeData( const QModelIndexList &indexes ) const
             debug() << "is original item, add mimeData from source model";
             QModelIndex originalIdx = mapToSource( idx );
             if( originalIdx.isValid() )
-            {
                 sourceIndexes << originalIdx;
-                originalRows << originalIdx.row();
-            }
         }
     }
 
@@ -84,40 +80,67 @@ PlaylistsByProviderProxy::mimeData( const QModelIndexList &indexes ) const
     if( !mime )
         mime = new QMimeData();
 
-    if( !originalRows.isEmpty() )
+    if( !sourceIndexes.isEmpty() )
     {
-        QByteArray encodedRows = encodeMimeRows( originalRows );
-        mime->setData( AMAROK_PROVIDERPROXY_INDEXES, encodedRows );
+        QByteArray encodedIndexes = encodeMimeRows( sourceIndexes );
+        mime->setData( AMAROK_PROVIDERPROXY_INDEXES, encodedIndexes );
     }
 
     return mime;
 }
 
 QByteArray
-PlaylistsByProviderProxy::encodeMimeRows( const QList<int> rows )
+PlaylistsByProviderProxy::encodeMimeRows( const QList<QModelIndex> indexes ) const
 {
-    QByteArray encodedRows;
-    QDataStream stream( &encodedRows, QIODevice::WriteOnly );
-    foreach( int row, rows )
-        stream << row;
+    QByteArray encodedIndexes;
+    QDataStream stream( &encodedIndexes, QIODevice::WriteOnly );
+    foreach( const QModelIndex &idx, indexes )
+    {
+        QStack<QModelIndex> indexStack;
+        //save the index and it's parents until we reach the rootnode so we have the complete tree.
+        QModelIndex i = idx;
+        while( i != m_rootNode )
+        {
+            indexStack.push( i );
+            i = i.parent();
+        }
+        //save the length of the stack first.
+        stream << indexStack.count();
+        while( !indexStack.isEmpty() )
+        {
+            QModelIndex i = indexStack.pop();
+            stream << i.row() << i.column();
+        }
+    }
 
-    return encodedRows;
+    return encodedIndexes;
 }
 
-QList<int>
-PlaylistsByProviderProxy::decodeMimeRows( QByteArray mimeData )
+QList<QModelIndex>
+PlaylistsByProviderProxy::decodeMimeRows( QByteArray mimeData, QAbstractItemModel *model ) const
 {
     DEBUG_BLOCK
     debug() << mimeData;
-    QList<int> rows;
+    QList<QModelIndex> idxs;
     QDataStream stream( &mimeData, QIODevice::ReadOnly );
     while( !stream.atEnd() )
     {
-        int row;
-        stream >> row;
-        rows << row;
+        QStack<QModelIndex> indexStack;
+        int count;
+        stream >> count;
+        //start from the rootNode and build "down" the tree
+        QModelIndex idx = m_rootNode;
+        while( count-- > 0 )
+        {
+            int row;
+            int column;
+            stream >> row >> column;
+            idx = model->index( row, column, idx );
+        }
+        //the last one should be the index we saved in encodeMimeRows
+        idxs << idx;
     }
-    return rows;
+    return idxs;
 }
 
 bool
@@ -136,26 +159,26 @@ PlaylistsByProviderProxy::dropMimeData( const QMimeData *data, Qt::DropAction ac
 
     if( isGroup( parent ) )
     {
-        if( data->hasFormat( AmarokMimeData::PODCASTCHANNEL_MIME ) )
+        if( data->hasFormat( AMAROK_PROVIDERPROXY_INDEXES ) )
         {
             const AmarokMimeData* amarokMime = dynamic_cast<const AmarokMimeData*>( data );
-            QList<int> originalRows = decodeMimeRows( data->data( AMAROK_PROVIDERPROXY_INDEXES ) );
-            foreach( Meta::PodcastChannelPtr channel, amarokMime->podcastChannels() )
+            QList<QModelIndex> originalIndexes =
+                    decodeMimeRows( data->data( AMAROK_PROVIDERPROXY_INDEXES ), m_model );
+            //set the groupedColumn data of all playlist indexes to the data of this group
+            //the model will understand this as a copy to the provider it's dropped on
+            RoleVariantMap groupData =
+                    m_groupMaps.value( parent.row() ).value( parent.column() );
+            bool result = !originalIndexes.isEmpty();
+            foreach( QModelIndex originalIndex, originalIndexes )
             {
-                //set the groupedColumn data of all playlist indexes to the data of this group
-                //the model will understand this as a copy to the provider it's dropped on
-                RoleVariantMap groupData =
-                        m_groupMaps.value( parent.row() ).value( parent.column() );
-
-                int originalRow = originalRows.takeFirst();
-                QModelIndex originalIndex =
-                        m_model->index( originalRow, m_groupedColumn, m_rootNode );
-                if( !originalIndex.isValid() )
+                QModelIndex groupedColumnIndex =
+                        originalIndex.sibling( originalIndex.row(), m_groupedColumn );
+                if( !groupedColumnIndex.isValid() )
                     continue;
 
-                m_model->setItemData( originalIndex, groupData );
+                result = m_model->setItemData( groupedColumnIndex, groupData ) ? result : false;
             }
-            return true;
+            return result;
         }
         return false;
     }
diff --git a/src/browsers/playlistbrowser/PlaylistsByProviderProxy.h b/src/browsers/playlistbrowser/PlaylistsByProviderProxy.h
index f78d25b..d24c9ac 100644
--- a/src/browsers/playlistbrowser/PlaylistsByProviderProxy.h
+++ b/src/browsers/playlistbrowser/PlaylistsByProviderProxy.h
@@ -26,8 +26,14 @@ class PlaylistsByProviderProxy : public QtGroupingProxy
 {
     Q_OBJECT
     public:
-        static QByteArray encodeMimeRows( QList<int> rowList );
-        static QList<int> decodeMimeRows( QByteArray data );
+        /** serializes the indexes into a bytearray
+          */
+        QByteArray encodeMimeRows( const QList<QModelIndex> indexes ) const;
+
+        /** \arg data serialized data
+          * \model this model is used to get the indexes.
+          */
+        QList<QModelIndex> decodeMimeRows( QByteArray data, QAbstractItemModel *model ) const;
         static const QString AMAROK_PROVIDERPROXY_INDEXES;
 
         PlaylistsByProviderProxy( QAbstractItemModel *model, int column );
diff --git a/src/browsers/playlistbrowser/PodcastModel.cpp b/src/browsers/playlistbrowser/PodcastModel.cpp
index 0252322..8ca8df5 100644
--- a/src/browsers/playlistbrowser/PodcastModel.cpp
+++ b/src/browsers/playlistbrowser/PodcastModel.cpp
@@ -375,29 +375,46 @@ PlaylistBrowserNS::PodcastModel::data(const QModelIndex & index, int role) const
 bool
 PlaylistBrowserNS::PodcastModel::setData( const QModelIndex &idx, const QVariant &value, int role )
 {
-    if( idx.isValid() )
+    if( !idx.isValid() )
+        return false;
+
+    if( idx.column() == ProviderColumn )
     {
-        if( idx.column() == ProviderColumn )
+        if( role == Qt::DisplayRole )
         {
-            if( role == Qt::DisplayRole )
-            {
-                Meta::PodcastChannelPtr channel = channelForIndex( idx );
-                if( !channel )
-                    return false;
-                PlaylistProvider *provider = getProviderByName( value.toString() );
-                if( !provider )
-                    return false;
-                debug() << QString( "Copy \"%1\" to \"%2\"." ).arg( channel->prettyName() )
-                        .arg( provider->prettyName() );
-                PodcastProvider *podcastProvider = dynamic_cast<PodcastProvider *>( provider );
-                if( !podcastProvider )
+            PlaylistProvider *provider = getProviderByName( value.toString() );
+            if( !provider )
+                return false;
+            PodcastProvider *podcastProvider = dynamic_cast<PodcastProvider *>( provider );
+            if( !podcastProvider )
                     return false;
-                podcastProvider->addChannel( channel );
+
+            switch( podcastItemType( idx ) )
+            {
+                case Meta::ChannelType:
+                {
+                    Meta::PodcastChannelPtr channel = channelForIndex( idx );
+                    if( !channel )
+                        return false;
+                    debug() << QString( "Copy podcast channel \"%1\" to \"%2\"." )
+                            .arg( channel->prettyName() ).arg( provider->prettyName() );
+                    return !podcastProvider->addChannel( channel ).isNull();
+                }
+                case Meta::EpisodeType:
+                {
+                    Meta::PodcastEpisodePtr episode = episodeForIndex( idx );
+                    if( !episode )
+                        return false;
+                    debug() << QString( "Copy podcast episode \"%1\" to \"%2\"." )
+                            .arg( episode->prettyName() ).arg( provider->prettyName() );
+                    return !podcastProvider->addEpisode( episode ).isNull();
+                }
+                default: return false;
             }
-            //return true even for the data we didn't handle to get QAbstractItemModel::setItemData to work
-            //TODO: implement setItemData()
-            return true;
         }
+        //return true even for the data we didn't handle to get QAbstractItemModel::setItemData to work
+        //TODO: implement setItemData()
+        return true;
     }
 
     return false;
@@ -1216,6 +1233,23 @@ PlaylistBrowserNS::PodcastModel::channelForIndex( const QModelIndex &index )
     return Meta::PodcastChannelPtr();
 }
 
+Meta::PodcastEpisodePtr
+PlaylistBrowserNS::PodcastModel::episodeForIndex( const QModelIndex &index )
+{
+    if( !index.isValid() )
+        return Meta::PodcastEpisodePtr();
+
+    switch( podcastItemType( index ) )
+    {
+        case Meta::EpisodeType:
+            return Meta::PodcastEpisodePtr(
+                static_cast<Meta::PodcastEpisode *>( index.internalPointer() ) );
+        case Meta::ChannelType:
+        default:
+            return Meta::PodcastEpisodePtr();
+    }
+
+}
 
 Meta::PodcastChannelList
 PlaylistBrowserNS::PodcastModel::selectedChannels( const QModelIndexList &indices )
diff --git a/src/browsers/playlistbrowser/PodcastModel.h b/src/browsers/playlistbrowser/PodcastModel.h
index dcdf3e6..b4e9c2c 100644
--- a/src/browsers/playlistbrowser/PodcastModel.h
+++ b/src/browsers/playlistbrowser/PodcastModel.h
@@ -126,6 +126,7 @@ class PodcastModel : public QAbstractItemModel, public MetaPlaylistModel,
         ~PodcastModel();
         static int podcastItemType( const QModelIndex &index );
         static Meta::PodcastChannelPtr channelForIndex( const QModelIndex &index );
+        static Meta::PodcastEpisodePtr episodeForIndex( const QModelIndex &index );
 
         Q_DISABLE_COPY( PodcastModel )

*** Bug 229837 has been marked as a duplicate of this bug. ***

*** Bug 237151 has been marked as a duplicate of this bug. ***

Just tried Amarok 2.3.90, and it would offer in theory the possibility to
transfer a podcast like a normal audiofile to the iPod (not as elegant like
in 1.4 where they automatically were queued for transfer but better than nothing).
But it won´t transfer anything. Am I missing something or is there still no way
to have podcasts transferred to the iPod ?

Amarok 2.4-GIT, kubuntu 10.10, iPod 2g nano. I was able to copy a podcast into home directory from both the playlist and the collection pane. Unable to copy it to the iPod either as music or a podcast. The drag icon changes to a slashy circle when I try to copy the podcast from the playlist to the iPod collection. No popup menu option given to transfer it from the podcast collection to the iPod collection.

Thanks for testing, but this is a wish that has been reopened, hence it is not implemented yet.

*** Bug 269469 has been marked as a duplicate of this bug. ***

Does this patch still apply? If yes, please submit it to http://reviewboard.git.org. You will need an identity on http://identity.kde.org. Please submit this ASAP as we enter feature freeze on July 3rd.

Are there still any plans to fix this, or should I (after more than two years) give up to wait for it ?

We are currently rewriting the whole Media Devices stack to fix this. Feel free to give a hand and get in touch with us in #amarok on irc.freenode.net

Okay, the patch posted here is for USB Mass Storage collection. I plan to work on this for the iPod collection, but it won't happen before Amarok 2.7.

I just tested on Amarok 2.7 on KDE SC 4.10.2 and this bug report is still valid.