Bug 195704 - Ability to transfer podcasts to iPods
Summary: Ability to transfer podcasts to iPods
Status: ASSIGNED
Alias: None
Product: amarok
Classification: Applications
Component: Collections/iPod iPhone (show other bugs)
Version: 2.5.0
Platform: Mandriva RPMs Linux
: HI wishlist
Target Milestone: 2.7
Assignee: Amarok Developers
URL:
Keywords:
: 229837 237151 269469 (view as bug list)
Depends on:
Blocks:
 
Reported: 2009-06-08 20:33 UTC by Olivier Delaune
Modified: 2013-04-16 13:43 UTC (History)
15 users (show)

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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Olivier Delaune 2009-06-08 20:33:22 UTC
Version:           2.1 (using KDE 4.2.4)
OS:                Linux
Installed from:    Mandriva RPMs

Hello,
actually (amarok 2.1), we can just transfer songs and album whiwh are in a collection into an iPod. We cannot transfer podcasts into iPod. Can you reimplement it to the future version of amarok?
Thanks
Comment 1 Mikko C. 2009-08-22 15:01:28 UTC
Bart, is this still valid?
Comment 2 Bart Cerneels 2009-08-22 15:09:01 UTC
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.
Comment 3 lambda512 2009-09-02 17:25:31 UTC
*** This bug has been confirmed by popular vote. ***
Comment 4 Markus Becker 2009-12-06 13:33:47 UTC
(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?
Comment 5 Jochen Bauer 2009-12-06 15:33:13 UTC
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.
Comment 6 Markus Becker 2009-12-13 15:02:02 UTC
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.
Comment 7 rfc469 2009-12-25 21:17:55 UTC
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.
Comment 8 Myriam Schweingruber 2010-01-07 11:09:02 UTC
(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.
Comment 9 Bart Cerneels 2010-01-07 11:41:04 UTC
(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 .
Comment 10 Jochen Bauer 2010-01-07 13:49:55 UTC
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
Comment 11 Bart Cerneels 2010-01-07 14:02:11 UTC
(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.
Comment 12 Jochen Bauer 2010-01-07 15:38:12 UTC
(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.
Comment 13 Myriam Schweingruber 2010-01-07 16:03:35 UTC
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.
Comment 14 Jochen Bauer 2010-01-07 16:12:20 UTC
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
Comment 15 Markus Becker 2010-01-11 09:11:04 UTC
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
Comment 16 Olivier Delaune 2010-02-15 22:03:29 UTC
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?
Comment 17 Myriam Schweingruber 2010-02-15 22:29:22 UTC
(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?
Comment 18 Bart Cerneels 2010-02-15 22:47:40 UTC
I've got it almost implemented for UMS devices. The iPod implementation proved to be beyond my reach for this version.
Comment 19 Bart Cerneels 2010-02-25 20:54:42 UTC
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 )
Comment 20 Myriam Schweingruber 2010-03-08 00:18:13 UTC
*** Bug 229837 has been marked as a duplicate of this bug. ***
Comment 21 Myriam Schweingruber 2010-12-11 22:52:29 UTC
*** Bug 237151 has been marked as a duplicate of this bug. ***
Comment 22 Georg Kovalcik 2010-12-13 10:15:27 UTC
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 ?
Comment 23 Timothy Lanzi 2011-01-04 02:37:13 UTC
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.
Comment 24 Myriam Schweingruber 2011-01-05 00:45:20 UTC
Thanks for testing, but this is a wish that has been reopened, hence it is not implemented yet.
Comment 25 Bart Cerneels 2011-03-30 12:50:03 UTC
*** Bug 269469 has been marked as a duplicate of this bug. ***
Comment 26 Myriam Schweingruber 2011-06-26 20:30:01 UTC
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.
Comment 27 Georg Kovalcik 2011-10-05 08:41:04 UTC
Are there still any plans to fix this, or should I (after more than two years) give up to wait for it ?
Comment 28 Myriam Schweingruber 2011-10-06 16:32:13 UTC
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
Comment 29 Matěj Laitl 2012-05-08 11:55:16 UTC
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.
Comment 30 Olivier Delaune 2013-04-06 15:01:22 UTC
I just tested on Amarok 2.7 on KDE SC 4.10.2 and this bug report is still valid.