Bug 218152

(In reply to comment #0)
> For the performance and other implicit benefits, KIO Slaves should run in
> parallel.
Correct version: For the performance and other implicit benefits, KIO Slaves should not run in parallel.

*** This bug has been marked as a duplicate of bug 206550 ***

IMHO, this bug is not a duplicate of bug 206550 because 206550 discusses the problem of two sequential copy-to operations but this bug discusses the problem inside of one copy-to operation.

In other terms, 206550 says more than one copy-to operation fails,
This bug says that copy to tries to parallelize the file copy within single copy-to action so performance degrades extremely.

I can confirm that - copying 100 files to USB player just stalled with a lot of
kio_file processes in background.
A good solution would be to implement queueing in old amarok - every single file processed individually and in case of a failure while transferring a report would be generated.

Created attachment 39338 [details]
Output of "ps aux |grep amarok" on my computer during a big copy to collection operation of several albums

Changing element

Hey folks,

I need some more information before I can begin tracking down this bug. Could you all please post the exact devices you are transferring tracks to when you experience this bug?

Was able to test with a generic USB key I have. Amarok was running 150 jobs at once, but I changed this to 1.

After doing so I no longer experience extremely slow write speeds.

However, this bug still needs to be tested against Ipods and MTP devices.

The commit should make it into master soon.

Changing to bugs list then.

I'm experiencing this bug with an iPod and several MTPs (Hard disks and USB keys).
IMHO, Amarok copy files sequentially to every device for optimum performance.

I also ran into this bug when using an iPod Nano 2G with Amarok 2.2.2.

I selected about 12 albums (~150 tracks) and used the "Copy to collection >" feature, which then spawned ~150 kio_file threads, locking up the USB access and creatinga HUGE load average on the computer. I guess this is a pretty clear indication of the parallel access problem.

I hope the fix for this reaches a release pretty soon.. :)

Setting version correctly.

commit 5d5e42c57b483c08c457a8b3570944a4906323de
Author: Casey Link <unnamedrambler@gmail.com>
Date:   Fri Jan 8 13:27:03 2010 -0700

    Change maximum concurrent UMS jobs to 1 (instead of 150), to prevent all the KIO jobs from stalling with dismal write speeds.
    Also, changed the initiation order of UMS devices slightly so the read capability is read at the beginning, this is allows
    the user to copy tracks to a UMS devices before scanning it (aka Read Device).
    CCBUG: 218152

diff --git a/src/collection/mediadevicecollection/handler/MediaDeviceHandler.cpp b/src/collection/mediadevicecollection/handler/MediaDeviceHandler.cpp
index 1d9b5d0..b246613 100644
--- a/src/collection/mediadevicecollection/handler/MediaDeviceHandler.cpp
+++ b/src/collection/mediadevicecollection/handler/MediaDeviceHandler.cpp
@@ -309,6 +309,7 @@ MediaDeviceHandler::copyTrackListToDevice(const Meta::TrackList tracklist)
     DEBUG_BLOCK
 
     setupWriteCapability();
+    setupReadCapability();
 
     if( !m_wcb )
         return;
@@ -454,6 +455,7 @@ MediaDeviceHandler::copyTrackListToDevice(const Meta::TrackList tracklist)
 void
 MediaDeviceHandler::copyNextTrackToDevice()
 {
+    DEBUG_BLOCK
     Meta::TrackPtr track;
 
     // If there are more tracks to copy, copy the next one
@@ -1004,15 +1006,22 @@ MediaDeviceHandler::slotCopyTrackJobsDone( ThreadWeaver::Job* job )
 float
 MediaDeviceHandler::freeSpace() const
 {
+    DEBUG_BLOCK
     if ( m_rcb )
+    {
+        debug() << "totalCapacity:" << m_rcb->totalCapacity();
+        debug() << "usedCapacity():" << m_rcb->usedCapacity();
         return ( m_rcb->totalCapacity() - m_rcb->usedCapacity() );
-    else
+    } else {
+        debug() << "m_rcb null!";
         return 0.0;
+    }
 }
 
 float
 MediaDeviceHandler::usedcapacity() const
 {
+    DEBUG_BLOCK
     if ( m_rcb )
         return m_rcb->usedCapacity();
     else
@@ -1022,6 +1031,7 @@ MediaDeviceHandler::usedcapacity() const
 float
 MediaDeviceHandler::totalcapacity() const
 {
+    DEBUG_BLOCK
     if ( m_rcb )
         return m_rcb->totalCapacity();
     else
diff --git a/src/collection/umscollection/handler/UmsHandler.cpp b/src/collection/umscollection/handler/UmsHandler.cpp
index 047691c..17e18f8 100644
--- a/src/collection/umscollection/handler/UmsHandler.cpp
+++ b/src/collection/umscollection/handler/UmsHandler.cpp
@@ -65,6 +65,15 @@ using namespace Meta;
 
 /// UmsHandler
 
+// Define the maximum number of concurrent kio jobs allowed.
+// This used to be 150, but flash media doesn't handle parallel
+// writes well, so by forcing this constant to 1 the jobs
+// are run sequentially.
+// BUG: 218152
+#ifndef UMS_MAX_CONCURRENT_JOBS
+#define UMS_MAX_CONCURRENT_JOBS 1
+#endif
+
 UmsHandler::UmsHandler( UmsCollection *mc, const QString& mountPoint )
     : MediaDeviceHandler( mc )
     , m_watcher()
@@ -162,9 +171,35 @@ UmsHandler::init()
     m_parsed = false;
     m_parseAction = 0;
 
+    // Get storage access for getting device space capacity/usage
+    Solid::Device device = Solid::Device(  m_memColl->udi() );
+    if (  device.isValid() )
+    {
+        Solid::StorageAccess *storage = device.as<Solid::StorageAccess>();
+        if ( storage )
+            m_filepath = storage->filePath();
+        else if ( !m_mountPoint.isEmpty() )
+            m_filepath = m_mountPoint;
+
+        if ( !m_filepath.isEmpty() )
+            m_capacity = KDiskFreeSpaceInfo::freeSpaceInfo( m_filepath ).size();
+        else
+        {
+            debug() << "capacity = 0.0 because m_filepath.isEmpty()";
+            m_capacity = 0.0;
+        }
+    }
+    else
+    {
+        debug() << "device is not valid";
+        m_filepath = "";
+        m_capacity = 0.0;
+    }
+
     debug() << "Succeeded: true";
     m_memColl->emitCollectionReady();
     //m_memColl->slotAttemptConnectionDone( true );
+
 }
 
 void
@@ -475,7 +510,7 @@ UmsHandler::kioCopyTrack( const KUrl &src, const KUrl &dst )
     KIO::CopyJob *job = KIO::copy( src, dst, KIO::HideProgressInfo );
     m_jobcounter++;
 
-    if( m_jobcounter < 150 )
+    if( m_jobcounter < UMS_MAX_CONCURRENT_JOBS )
         copyNextTrackToDevice();
 
 
@@ -507,10 +542,10 @@ UmsHandler::fileTransferred( KJob *job )  //SLOT
         return;
     }
 
-    // Limit max number of jobs to 150, make sure more tracks left
+    // Limit max number of jobs to 1, make sure more tracks left
     // to copy
     debug() << "Tracks to copy still remain";
-    if( m_jobcounter < 150 )
+    if( m_jobcounter < 1 )
     {
         debug() << "Jobs: " << m_jobcounter;
         copyNextTrackToDevice();
@@ -549,7 +584,7 @@ UmsHandler::deleteFile( const KUrl &url )
 
     m_jobcounter++;
 
-    if( m_jobcounter < 150 )
+    if( m_jobcounter < UMS_MAX_CONCURRENT_JOBS )
         removeNextTrackFromDevice();
 
     connect( job, SIGNAL( result( KJob * ) ),
@@ -567,10 +602,8 @@ UmsHandler::fileDeleted( KJob *job )  //SLOT
 
     m_jobcounter--;
 
-    // Limit max number of jobs to 150, make sure more tracks left
-    // to delete
     debug() << "Tracks to delete still remain";
-    if( m_jobcounter < 150 )
+    if( m_jobcounter < UMS_MAX_CONCURRENT_JOBS )
     {
         debug() << "Jobs: " << m_jobcounter;
         removeNextTrackFromDevice();
@@ -638,6 +671,7 @@ UmsHandler::usedCapacity() const
 float
 UmsHandler::totalCapacity() const
 {
+    DEBUG_BLOCK
     return m_capacity;
 }
 
@@ -654,28 +688,6 @@ UmsHandler::prepareToParseTracks()
 {
     DEBUG_BLOCK
 
-    // Get storage access for getting device space capacity/usage
-
-    Solid::Device device = Solid::Device(  m_memColl->udi() );
-    if (  device.isValid() )
-    {
-        Solid::StorageAccess *storage = device.as<Solid::StorageAccess>();
-        if ( storage )
-            m_filepath = storage->filePath();
-        else if ( !m_mountPoint.isEmpty() )
-            m_filepath = m_mountPoint;
-
-        if ( !m_filepath.isEmpty() )
-            m_capacity = KDiskFreeSpaceInfo::freeSpaceInfo( m_filepath ).size();
-        else
-            m_capacity = 0.0;
-    }
-    else
-    {
-        m_filepath = "";
-        m_capacity = 0.0;
-    }
-
     m_watcher.addDir( m_mountPoint, KDirWatch::WatchDirOnly | KDirWatch::WatchFiles | KDirWatch::WatchSubDirs );
 
     QDirIterator it( m_mountPoint, QDirIterator::Subdirectories );
diff --git a/src/dialogs/OrganizeCollectionDialog.cpp b/src/dialogs/OrganizeCollectionDialog.cpp
index 39bb9e6..fac12c5 100644
--- a/src/dialogs/OrganizeCollectionDialog.cpp
+++ b/src/dialogs/OrganizeCollectionDialog.cpp
@@ -28,6 +28,8 @@
 #include "QStringx.h"
 #include "ui_OrganizeCollectionDialogBase.h"
 
+#include <kcolorscheme.h>
+
 #include <QDir>
 
 OrganizeCollectionDialog::OrganizeCollectionDialog( const Meta::TrackList &tracks, const QStringList &folders, QWidget *parent,  const char *name, bool modal,
@@ -57,37 +59,41 @@ OrganizeCollectionDialog::OrganizeCollectionDialog( const Meta::TrackList &track
     ui->setupUi( mainContainer );
 
     m_filenameLayoutDialog = new FilenameLayoutDialog( mainContainer, 1 );   //", 1" means isOrganizeCollection ==> doesn't show Options frame
-    m_filenameLayoutDialog->hide();
+//    m_filenameLayoutDialog->hide();
     connect( this, SIGNAL( accepted() ),
              m_filenameLayoutDialog, SLOT( onAccept() ) );
-    ui->verticalLayout->insertWidget( 3, m_filenameLayoutDialog );
+    ui->verticalLayout->insertWidget( 1, m_filenameLayoutDialog );
     ui->ignoreTheCheck->show();
 
     ui->folderCombo->insertItems( 0, folders );
     ui->folderCombo->setCurrentIndex( AmarokConfig::organizeDirectory() );
     ui->overwriteCheck->setChecked( AmarokConfig::overwriteFiles() );
-    ui->filetypeCheck->setChecked( AmarokConfig::groupByFiletype() );
-    ui->initialCheck->setChecked( AmarokConfig::groupArtists() );
     ui->spaceCheck->setChecked( AmarokConfig::replaceSpace() );
     ui->ignoreTheCheck->setChecked( AmarokConfig::ignoreThe() );
     ui->vfatCheck->setChecked( AmarokConfig::vfatCompatible() );
     ui->asciiCheck->setChecked( AmarokConfig::asciiOnly() );
-    ui->customschemeCheck->setChecked( AmarokConfig::useCustomScheme() );
     ui->regexpEdit->setText( AmarokConfig::replacementRegexp() );
     ui->replaceEdit->setText( AmarokConfig::replacementString() );
 
+    ui->vfatCheck->hide(); // this is no longer an option, vfat safe is always enabled.
+
+    ui->previewTableWidget->horizontalHeader()->setResizeMode( QHeaderView::ResizeToContents );
+    ui->conflictLabel->hide();
+    QPalette p = ui->conflictLabel->palette();
+    KColorScheme::adjustForeground( p, KColorScheme::NegativeText ); // TODO this isn't working, the color is still normal
+    ui->conflictLabel->setPalette( p );
+
+    // to show the conflict error
+    connect( ui->overwriteCheck, SIGNAL( stateChanged( int ) ), SLOT( slotUpdatePreview() ) );
+
     connect( this, SIGNAL( updatePreview( QString ) ), ui->previewText, SLOT( setText( QString ) ) );
 
-    connect( ui->filetypeCheck , SIGNAL(toggled(bool)), SLOT(slotUpdatePreview()) );
-    connect( ui->initialCheck  , SIGNAL(toggled(bool)), SLOT(slotUpdatePreview()) );
     connect( ui->ignoreTheCheck, SIGNAL(toggled(bool)), SLOT(slotUpdatePreview()) );
     connect( ui->spaceCheck    , SIGNAL(toggled(bool)), SLOT(slotUpdatePreview()) );
     connect( ui->asciiCheck    , SIGNAL(toggled(bool)), SLOT(slotUpdatePreview()) );
-    connect( ui->customschemeCheck, SIGNAL(toggled(bool)), SLOT(slotUpdatePreview()) );
     connect( ui->regexpEdit    , SIGNAL(textChanged(QString)), SLOT(slotUpdatePreview()) );
     connect( ui->replaceEdit    , SIGNAL(textChanged(QString)), SLOT(slotUpdatePreview()) );
     connect( m_filenameLayoutDialog, SIGNAL( schemeChanged() ), this, SLOT( slotUpdatePreview() ) );
-    connect( ui->customschemeCheck, SIGNAL( toggled( bool ) ), this, SLOT( toggleCustomScheme( bool ) ) );
 
     connect( this , SIGNAL( accepted() ), SLOT( slotDialogAccepted() ) );
     connect( ui->folderCombo, SIGNAL( currentIndexChanged( const QString & ) ),
@@ -95,7 +101,6 @@ OrganizeCollectionDialog::OrganizeCollectionDialog( const Meta::TrackList &track
     connect( ui->folderCombo, SIGNAL( currentIndexChanged( const QString & ) ),
              this, SLOT( slotEnableOk( const QString & ) ) );
 
-    toggleCustomScheme( ui->customschemeCheck->isChecked() );
     slotEnableOk( ui->folderCombo->currentText() );
 
     init();
@@ -229,29 +234,32 @@ OrganizeCollectionDialog::buildFormatTip() const
 QString
 OrganizeCollectionDialog::buildFormatString() const
 {
-    if( ui->customschemeCheck->isChecked() )
-        return "%folder/" + m_filenameLayoutDialog->getParsableScheme() + ".%filetype";
-    QString format = "%folder/";
-    if( ui->filetypeCheck->isChecked() )
-        format += "%filetype/";
-    if( ui->initialCheck->isChecked() )
-        format += "%initial/";
-
-    format += "%albumartist/";
-    if( ui->spaceCheck->isChecked() )   //replace spaces with underscores
-    {
-        format += "%album{_(Disc_%discnumber)}/";
-        format += "{%track_-_}%title.%filetype";
-    }
-    else
+    return "%folder/" + m_filenameLayoutDialog->getParsableScheme() + ".%filetype";
+}
+
+QString
+OrganizeCollectionDialog::commonPrefix( const QStringList &list ) const
+{
+    QString option = list.first().toLower();
+    int length = option.length();
+    while( length > 0 )
     {
-        format += "%album{ (Disc %discnumber)}/";
-        format += "{%track - }%title.%filetype";
+        bool found = true;
+        foreach( QString string, list )
+        {
+            if( string.left(length).toLower() != option )
+            {
+                found = false;
+                break;
+            }
+        }
+        if( found )
+            break;
+        --length;
+        option = option.left( length );
     }
+    return option;
 
-
-    format = QDir::fromNativeSeparators( format );      //fromNativeSeparators handles \\ under windows
-    return format;
 }
 
 
@@ -259,8 +267,56 @@ OrganizeCollectionDialog::buildFormatString() const
 void
 OrganizeCollectionDialog::preview( const QString &format )
 {
-    if( m_previewTrack )
-        emit updatePreview( buildDestination( format, m_previewTrack ) );
+    DEBUG_BLOCK
+    /*if( m_previewTrack )
+        emit updatePreview( buildDestination( format, m_previewTrack ) );*/
+
+    ui->previewTableWidget->clearContents();
+    ui->previewTableWidget->setRowCount( m_allTracks.size() );
+    bool conflict = false;
+    for (int i = 0; i < m_allTracks.size(); ++i)
+    {
+        Meta::TrackPtr track = m_allTracks.at(i);
+
+        QString originalPath = track->prettyUrl();
+        QString newPath = buildDestination( format, track );
+
+//         QStringList list;
+//         list << originalPath << newPath;
+//
+//         QString common_prefix = commonPrefix( list );
+//         debug() << "common prefix: " << common_prefix;
+//         originalPath = originalPath.mid( common_prefix.length() );
+//         newPath = newPath.mid( common_prefix.length() );
+
+        QFileInfo info( newPath );
+        if( !conflict  && info.exists() )
+            conflict = true;
+
+        QTableWidgetItem* item = new QTableWidgetItem( originalPath );
+        ui->previewTableWidget->setItem(i, 0, item);
+        QPalette p = ui->previewTableWidget->palette();
+        KColorScheme::adjustBackground(p, KColorScheme::NegativeBackground);
+        if( info.exists() )
+        {
+
+            item->setBackgroundColor( p.color( QPalette::Base ) );
+        }
+
+        item = new QTableWidgetItem( newPath );
+        if( info.exists() )
+            item->setBackgroundColor( p.color( QPalette::Base ) );
+        ui->previewTableWidget->setItem(i, 1, item);
+
+    }
+    if( conflict )
+    {
+        ui->conflictLabel->show();
+        if( ui->overwriteCheck->isChecked() )
+            ui->conflictLabel->setText( i18n( "There is a filename conflict, existing files will be overwritten." ) );
+        else
+            ui->conflictLabel->setText( i18n( "There is a filename conflict, existing files will not be changed." ) );
+    }
 }
 
 
@@ -281,9 +337,9 @@ OrganizeCollectionDialog::cleanPath( const QString &component ) const
     result.simplified();
     if( ui->spaceCheck->isChecked() )
         result.replace( QRegExp( "\\s" ), "_" );
-    debug()<<"I'm about to do Amarok::vfatPath( result ), this is result: "<<result;
-    if( ui->vfatCheck->isChecked() )
-        result = Amarok::vfatPath( result );
+//     debug()<<"I'm about to do Amarok::vfatPath( result ), this is result: "<<result;
+//    if( ui->vfatCheck->isChecked() )
+    result = Amarok::vfatPath( result );
 
     result.replace( '/', '-' );
 
@@ -298,10 +354,7 @@ OrganizeCollectionDialog::update( int dummy )   //why the dummy?
 
     if( m_previewTrack )
     {
-        if( ui->customschemeCheck->isChecked() )
-            emit updatePreview( buildDestination( "%folder/" + m_filenameLayoutDialog->getParsableScheme(), m_previewTrack ) );
-        else
-            emit updatePreview( buildDestination( buildFormatString(), m_previewTrack ) );
+        emit updatePreview( buildDestination( "%folder/" + m_filenameLayoutDialog->getParsableScheme(), m_previewTrack ) );
     }
 }
 
@@ -330,35 +383,14 @@ void
 OrganizeCollectionDialog::slotDialogAccepted()
 {
     AmarokConfig::setOrganizeDirectory( ui->folderCombo->currentIndex() );
-    AmarokConfig::setGroupByFiletype( ui->filetypeCheck->isChecked() );
-    AmarokConfig::setGroupArtists( ui->initialCheck->isChecked() );
     AmarokConfig::setIgnoreThe( ui->ignoreTheCheck->isChecked() );
     AmarokConfig::setReplaceSpace( ui->spaceCheck->isChecked() );
     AmarokConfig::setVfatCompatible( ui->vfatCheck->isChecked() );
     AmarokConfig::setAsciiOnly( ui->asciiCheck->isChecked() );
-    AmarokConfig::setUseCustomScheme( ui->customschemeCheck->isChecked() );
     AmarokConfig::setReplacementRegexp( ui->regexpEdit->text() );
     AmarokConfig::setReplacementString( ui->replaceEdit->text() );
 }
 
-//Hides and shows the right elements in the interface in the right order to keep the layout sane
-void
-OrganizeCollectionDialog::toggleCustomScheme( bool state )  //SLOT
-{
-    if( state )
-    {
-        ui->initialCheck->hide();
-        ui->filetypeCheck->hide();
-        m_filenameLayoutDialog->setVisible( true );
-    }
-    else
-    {
-        m_filenameLayoutDialog->hide();
-        ui->initialCheck->show();
-        ui->filetypeCheck->show();
-    }
-}
-
 //The Ok button should be disabled when there's no collection root selected, and when there is no .%filetype in format string
 void
 OrganizeCollectionDialog::slotEnableOk( const QString & currentCollectionRoot )
diff --git a/src/dialogs/OrganizeCollectionDialog.h b/src/dialogs/OrganizeCollectionDialog.h
index a5a8fab..cadeb7b 100644
--- a/src/dialogs/OrganizeCollectionDialog.h
+++ b/src/dialogs/OrganizeCollectionDialog.h
@@ -60,6 +60,7 @@ class AMAROK_EXPORT OrganizeCollectionDialog : public KDialog
         QString cleanPath( const QString &component ) const;
         QString buildFormatTip() const;
         QString buildFormatString() const;
+        QString commonPrefix( const QStringList &list ) const;
         void toggleDetails();
         void preview( const QString &format );
         void update( int dummy );
@@ -73,7 +74,6 @@ class AMAROK_EXPORT OrganizeCollectionDialog : public KDialog
         Meta::TrackList m_allTracks;
 
     private slots:
-        void toggleCustomScheme( bool state );
         void slotEnableOk( const QString & currentCollectionRoot );
 };

As of commit 5d5e42c57b483c08c457a8b3570944a4906323de  this bug should be resolved for UMS devices. MTP and iPods still suffer from it. Could someoneverify this please?

commit 5d5e42c57b483c08c457a8b3570944a4906323de
Author: Casey Link <unnamedrambler@gmail.com>
Date:   Fri Jan 8 13:27:03 2010 -0700

    Change maximum concurrent UMS jobs to 1 (instead of 150), to prevent all the KIO jobs from stalling with dismal write speeds.
    Also, changed the initiation order of UMS devices slightly so the read capability is read at the beginning, this is allows
    the user to copy tracks to a UMS devices before scanning it (aka Read Device).
    CCBUG: 218152

diff --git a/src/collection/mediadevicecollection/handler/MediaDeviceHandler.cpp b/src/collection/mediadevicecollection/handler/MediaDeviceHandler.cpp
index 1d9b5d0..b246613 100644
--- a/src/collection/mediadevicecollection/handler/MediaDeviceHandler.cpp
+++ b/src/collection/mediadevicecollection/handler/MediaDeviceHandler.cpp
@@ -309,6 +309,7 @@ MediaDeviceHandler::copyTrackListToDevice(const Meta::TrackList tracklist)
     DEBUG_BLOCK
 
     setupWriteCapability();
+    setupReadCapability();
 
     if( !m_wcb )
         return;
@@ -454,6 +455,7 @@ MediaDeviceHandler::copyTrackListToDevice(const Meta::TrackList tracklist)
 void
 MediaDeviceHandler::copyNextTrackToDevice()
 {
+    DEBUG_BLOCK
     Meta::TrackPtr track;
 
     // If there are more tracks to copy, copy the next one
@@ -1004,15 +1006,22 @@ MediaDeviceHandler::slotCopyTrackJobsDone( ThreadWeaver::Job* job )
 float
 MediaDeviceHandler::freeSpace() const
 {
+    DEBUG_BLOCK
     if ( m_rcb )
+    {
+        debug() << "totalCapacity:" << m_rcb->totalCapacity();
+        debug() << "usedCapacity():" << m_rcb->usedCapacity();
         return ( m_rcb->totalCapacity() - m_rcb->usedCapacity() );
-    else
+    } else {
+        debug() << "m_rcb null!";
         return 0.0;
+    }
 }
 
 float
 MediaDeviceHandler::usedcapacity() const
 {
+    DEBUG_BLOCK
     if ( m_rcb )
         return m_rcb->usedCapacity();
     else
@@ -1022,6 +1031,7 @@ MediaDeviceHandler::usedcapacity() const
 float
 MediaDeviceHandler::totalcapacity() const
 {
+    DEBUG_BLOCK
     if ( m_rcb )
         return m_rcb->totalCapacity();
     else
diff --git a/src/collection/umscollection/handler/UmsHandler.cpp b/src/collection/umscollection/handler/UmsHandler.cpp
index 047691c..17e18f8 100644
--- a/src/collection/umscollection/handler/UmsHandler.cpp
+++ b/src/collection/umscollection/handler/UmsHandler.cpp
@@ -65,6 +65,15 @@ using namespace Meta;
 
 /// UmsHandler
 
+// Define the maximum number of concurrent kio jobs allowed.
+// This used to be 150, but flash media doesn't handle parallel
+// writes well, so by forcing this constant to 1 the jobs
+// are run sequentially.
+// BUG: 218152
+#ifndef UMS_MAX_CONCURRENT_JOBS
+#define UMS_MAX_CONCURRENT_JOBS 1
+#endif
+
 UmsHandler::UmsHandler( UmsCollection *mc, const QString& mountPoint )
     : MediaDeviceHandler( mc )
     , m_watcher()
@@ -162,9 +171,35 @@ UmsHandler::init()
     m_parsed = false;
     m_parseAction = 0;
 
+    // Get storage access for getting device space capacity/usage
+    Solid::Device device = Solid::Device(  m_memColl->udi() );
+    if (  device.isValid() )
+    {
+        Solid::StorageAccess *storage = device.as<Solid::StorageAccess>();
+        if ( storage )
+            m_filepath = storage->filePath();
+        else if ( !m_mountPoint.isEmpty() )
+            m_filepath = m_mountPoint;
+
+        if ( !m_filepath.isEmpty() )
+            m_capacity = KDiskFreeSpaceInfo::freeSpaceInfo( m_filepath ).size();
+        else
+        {
+            debug() << "capacity = 0.0 because m_filepath.isEmpty()";
+            m_capacity = 0.0;
+        }
+    }
+    else
+    {
+        debug() << "device is not valid";
+        m_filepath = "";
+        m_capacity = 0.0;
+    }
+
     debug() << "Succeeded: true";
     m_memColl->emitCollectionReady();
     //m_memColl->slotAttemptConnectionDone( true );
+
 }
 
 void
@@ -475,7 +510,7 @@ UmsHandler::kioCopyTrack( const KUrl &src, const KUrl &dst )
     KIO::CopyJob *job = KIO::copy( src, dst, KIO::HideProgressInfo );
     m_jobcounter++;
 
-    if( m_jobcounter < 150 )
+    if( m_jobcounter < UMS_MAX_CONCURRENT_JOBS )
         copyNextTrackToDevice();
 
 
@@ -507,10 +542,10 @@ UmsHandler::fileTransferred( KJob *job )  //SLOT
         return;
     }
 
-    // Limit max number of jobs to 150, make sure more tracks left
+    // Limit max number of jobs to 1, make sure more tracks left
     // to copy
     debug() << "Tracks to copy still remain";
-    if( m_jobcounter < 150 )
+    if( m_jobcounter < 1 )
     {
         debug() << "Jobs: " << m_jobcounter;
         copyNextTrackToDevice();
@@ -549,7 +584,7 @@ UmsHandler::deleteFile( const KUrl &url )
 
     m_jobcounter++;
 
-    if( m_jobcounter < 150 )
+    if( m_jobcounter < UMS_MAX_CONCURRENT_JOBS )
         removeNextTrackFromDevice();
 
     connect( job, SIGNAL( result( KJob * ) ),
@@ -567,10 +602,8 @@ UmsHandler::fileDeleted( KJob *job )  //SLOT
 
     m_jobcounter--;
 
-    // Limit max number of jobs to 150, make sure more tracks left
-    // to delete
     debug() << "Tracks to delete still remain";
-    if( m_jobcounter < 150 )
+    if( m_jobcounter < UMS_MAX_CONCURRENT_JOBS )
     {
         debug() << "Jobs: " << m_jobcounter;
         removeNextTrackFromDevice();
@@ -638,6 +671,7 @@ UmsHandler::usedCapacity() const
 float
 UmsHandler::totalCapacity() const
 {
+    DEBUG_BLOCK
     return m_capacity;
 }
 
@@ -654,28 +688,6 @@ UmsHandler::prepareToParseTracks()
 {
     DEBUG_BLOCK
 
-    // Get storage access for getting device space capacity/usage
-
-    Solid::Device device = Solid::Device(  m_memColl->udi() );
-    if (  device.isValid() )
-    {
-        Solid::StorageAccess *storage = device.as<Solid::StorageAccess>();
-        if ( storage )
-            m_filepath = storage->filePath();
-        else if ( !m_mountPoint.isEmpty() )
-            m_filepath = m_mountPoint;
-
-        if ( !m_filepath.isEmpty() )
-            m_capacity = KDiskFreeSpaceInfo::freeSpaceInfo( m_filepath ).size();
-        else
-            m_capacity = 0.0;
-    }
-    else
-    {
-        m_filepath = "";
-        m_capacity = 0.0;
-    }
-
     m_watcher.addDir( m_mountPoint, KDirWatch::WatchDirOnly | KDirWatch::WatchFiles | KDirWatch::WatchSubDirs );
 
     QDirIterator it( m_mountPoint, QDirIterator::Subdirectories );
diff --git a/src/dialogs/OrganizeCollectionDialog.cpp b/src/dialogs/OrganizeCollectionDialog.cpp
index 39bb9e6..fac12c5 100644
--- a/src/dialogs/OrganizeCollectionDialog.cpp
+++ b/src/dialogs/OrganizeCollectionDialog.cpp
@@ -28,6 +28,8 @@
 #include "QStringx.h"
 #include "ui_OrganizeCollectionDialogBase.h"
 
+#include <kcolorscheme.h>
+
 #include <QDir>
 
 OrganizeCollectionDialog::OrganizeCollectionDialog( const Meta::TrackList &tracks, const QStringList &folders, QWidget *parent,  const char *name, bool modal,
@@ -57,37 +59,41 @@ OrganizeCollectionDialog::OrganizeCollectionDialog( const Meta::TrackList &track
     ui->setupUi( mainContainer );
 
     m_filenameLayoutDialog = new FilenameLayoutDialog( mainContainer, 1 );   //", 1" means isOrganizeCollection ==> doesn't show Options frame
-    m_filenameLayoutDialog->hide();
+//    m_filenameLayoutDialog->hide();
     connect( this, SIGNAL( accepted() ),
              m_filenameLayoutDialog, SLOT( onAccept() ) );
-    ui->verticalLayout->insertWidget( 3, m_filenameLayoutDialog );
+    ui->verticalLayout->insertWidget( 1, m_filenameLayoutDialog );
     ui->ignoreTheCheck->show();
 
     ui->folderCombo->insertItems( 0, folders );
     ui->folderCombo->setCurrentIndex( AmarokConfig::organizeDirectory() );
     ui->overwriteCheck->setChecked( AmarokConfig::overwriteFiles() );
-    ui->filetypeCheck->setChecked( AmarokConfig::groupByFiletype() );
-    ui->initialCheck->setChecked( AmarokConfig::groupArtists() );
     ui->spaceCheck->setChecked( AmarokConfig::replaceSpace() );
     ui->ignoreTheCheck->setChecked( AmarokConfig::ignoreThe() );
     ui->vfatCheck->setChecked( AmarokConfig::vfatCompatible() );
     ui->asciiCheck->setChecked( AmarokConfig::asciiOnly() );
-    ui->customschemeCheck->setChecked( AmarokConfig::useCustomScheme() );
     ui->regexpEdit->setText( AmarokConfig::replacementRegexp() );
     ui->replaceEdit->setText( AmarokConfig::replacementString() );
 
+    ui->vfatCheck->hide(); // this is no longer an option, vfat safe is always enabled.
+
+    ui->previewTableWidget->horizontalHeader()->setResizeMode( QHeaderView::ResizeToContents );
+    ui->conflictLabel->hide();
+    QPalette p = ui->conflictLabel->palette();
+    KColorScheme::adjustForeground( p, KColorScheme::NegativeText ); // TODO this isn't working, the color is still normal
+    ui->conflictLabel->setPalette( p );
+
+    // to show the conflict error
+    connect( ui->overwriteCheck, SIGNAL( stateChanged( int ) ), SLOT( slotUpdatePreview() ) );
+
     connect( this, SIGNAL( updatePreview( QString ) ), ui->previewText, SLOT( setText( QString ) ) );
 
-    connect( ui->filetypeCheck , SIGNAL(toggled(bool)), SLOT(slotUpdatePreview()) );
-    connect( ui->initialCheck  , SIGNAL(toggled(bool)), SLOT(slotUpdatePreview()) );
     connect( ui->ignoreTheCheck, SIGNAL(toggled(bool)), SLOT(slotUpdatePreview()) );
     connect( ui->spaceCheck    , SIGNAL(toggled(bool)), SLOT(slotUpdatePreview()) );
     connect( ui->asciiCheck    , SIGNAL(toggled(bool)), SLOT(slotUpdatePreview()) );
-    connect( ui->customschemeCheck, SIGNAL(toggled(bool)), SLOT(slotUpdatePreview()) );
     connect( ui->regexpEdit    , SIGNAL(textChanged(QString)), SLOT(slotUpdatePreview()) );
     connect( ui->replaceEdit    , SIGNAL(textChanged(QString)), SLOT(slotUpdatePreview()) );
     connect( m_filenameLayoutDialog, SIGNAL( schemeChanged() ), this, SLOT( slotUpdatePreview() ) );
-    connect( ui->customschemeCheck, SIGNAL( toggled( bool ) ), this, SLOT( toggleCustomScheme( bool ) ) );
 
     connect( this , SIGNAL( accepted() ), SLOT( slotDialogAccepted() ) );
     connect( ui->folderCombo, SIGNAL( currentIndexChanged( const QString & ) ),
@@ -95,7 +101,6 @@ OrganizeCollectionDialog::OrganizeCollectionDialog( const Meta::TrackList &track
     connect( ui->folderCombo, SIGNAL( currentIndexChanged( const QString & ) ),
              this, SLOT( slotEnableOk( const QString & ) ) );
 
-    toggleCustomScheme( ui->customschemeCheck->isChecked() );
     slotEnableOk( ui->folderCombo->currentText() );
 
     init();
@@ -229,29 +234,32 @@ OrganizeCollectionDialog::buildFormatTip() const
 QString
 OrganizeCollectionDialog::buildFormatString() const
 {
-    if( ui->customschemeCheck->isChecked() )
-        return "%folder/" + m_filenameLayoutDialog->getParsableScheme() + ".%filetype";
-    QString format = "%folder/";
-    if( ui->filetypeCheck->isChecked() )
-        format += "%filetype/";
-    if( ui->initialCheck->isChecked() )
-        format += "%initial/";
-
-    format += "%albumartist/";
-    if( ui->spaceCheck->isChecked() )   //replace spaces with underscores
-    {
-        format += "%album{_(Disc_%discnumber)}/";
-        format += "{%track_-_}%title.%filetype";
-    }
-    else
+    return "%folder/" + m_filenameLayoutDialog->getParsableScheme() + ".%filetype";
+}
+
+QString
+OrganizeCollectionDialog::commonPrefix( const QStringList &list ) const
+{
+    QString option = list.first().toLower();
+    int length = option.length();
+    while( length > 0 )
     {
-        format += "%album{ (Disc %discnumber)}/";
-        format += "{%track - }%title.%filetype";
+        bool found = true;
+        foreach( QString string, list )
+        {
+            if( string.left(length).toLower() != option )
+            {
+                found = false;
+                break;
+            }
+        }
+        if( found )
+            break;
+        --length;
+        option = option.left( length );
     }
+    return option;
 
-
-    format = QDir::fromNativeSeparators( format );      //fromNativeSeparators handles \\ under windows
-    return format;
 }
 
 
@@ -259,8 +267,56 @@ OrganizeCollectionDialog::buildFormatString() const
 void
 OrganizeCollectionDialog::preview( const QString &format )
 {
-    if( m_previewTrack )
-        emit updatePreview( buildDestination( format, m_previewTrack ) );
+    DEBUG_BLOCK
+    /*if( m_previewTrack )
+        emit updatePreview( buildDestination( format, m_previewTrack ) );*/
+
+    ui->previewTableWidget->clearContents();
+    ui->previewTableWidget->setRowCount( m_allTracks.size() );
+    bool conflict = false;
+    for (int i = 0; i < m_allTracks.size(); ++i)
+    {
+        Meta::TrackPtr track = m_allTracks.at(i);
+
+        QString originalPath = track->prettyUrl();
+        QString newPath = buildDestination( format, track );
+
+//         QStringList list;
+//         list << originalPath << newPath;
+//
+//         QString common_prefix = commonPrefix( list );
+//         debug() << "common prefix: " << common_prefix;
+//         originalPath = originalPath.mid( common_prefix.length() );
+//         newPath = newPath.mid( common_prefix.length() );
+
+        QFileInfo info( newPath );
+        if( !conflict  && info.exists() )
+            conflict = true;
+
+        QTableWidgetItem* item = new QTableWidgetItem( originalPath );
+        ui->previewTableWidget->setItem(i, 0, item);
+        QPalette p = ui->previewTableWidget->palette();
+        KColorScheme::adjustBackground(p, KColorScheme::NegativeBackground);
+        if( info.exists() )
+        {
+
+            item->setBackgroundColor( p.color( QPalette::Base ) );
+        }
+
+        item = new QTableWidgetItem( newPath );
+        if( info.exists() )
+            item->setBackgroundColor( p.color( QPalette::Base ) );
+        ui->previewTableWidget->setItem(i, 1, item);
+
+    }
+    if( conflict )
+    {
+        ui->conflictLabel->show();
+        if( ui->overwriteCheck->isChecked() )
+            ui->conflictLabel->setText( i18n( "There is a filename conflict, existing files will be overwritten." ) );
+        else
+            ui->conflictLabel->setText( i18n( "There is a filename conflict, existing files will not be changed." ) );
+    }
 }
 
 
@@ -281,9 +337,9 @@ OrganizeCollectionDialog::cleanPath( const QString &component ) const
     result.simplified();
     if( ui->spaceCheck->isChecked() )
         result.replace( QRegExp( "\\s" ), "_" );
-    debug()<<"I'm about to do Amarok::vfatPath( result ), this is result: "<<result;
-    if( ui->vfatCheck->isChecked() )
-        result = Amarok::vfatPath( result );
+//     debug()<<"I'm about to do Amarok::vfatPath( result ), this is result: "<<result;
+//    if( ui->vfatCheck->isChecked() )
+    result = Amarok::vfatPath( result );
 
     result.replace( '/', '-' );
 
@@ -298,10 +354,7 @@ OrganizeCollectionDialog::update( int dummy )   //why the dummy?
 
     if( m_previewTrack )
     {
-        if( ui->customschemeCheck->isChecked() )
-            emit updatePreview( buildDestination( "%folder/" + m_filenameLayoutDialog->getParsableScheme(), m_previewTrack ) );
-        else
-            emit updatePreview( buildDestination( buildFormatString(), m_previewTrack ) );
+        emit updatePreview( buildDestination( "%folder/" + m_filenameLayoutDialog->getParsableScheme(), m_previewTrack ) );
     }
 }
 
@@ -330,35 +383,14 @@ void
 OrganizeCollectionDialog::slotDialogAccepted()
 {
     AmarokConfig::setOrganizeDirectory( ui->folderCombo->currentIndex() );
-    AmarokConfig::setGroupByFiletype( ui->filetypeCheck->isChecked() );
-    AmarokConfig::setGroupArtists( ui->initialCheck->isChecked() );
     AmarokConfig::setIgnoreThe( ui->ignoreTheCheck->isChecked() );
     AmarokConfig::setReplaceSpace( ui->spaceCheck->isChecked() );
     AmarokConfig::setVfatCompatible( ui->vfatCheck->isChecked() );
     AmarokConfig::setAsciiOnly( ui->asciiCheck->isChecked() );
-    AmarokConfig::setUseCustomScheme( ui->customschemeCheck->isChecked() );
     AmarokConfig::setReplacementRegexp( ui->regexpEdit->text() );
     AmarokConfig::setReplacementString( ui->replaceEdit->text() );
 }
 
-//Hides and shows the right elements in the interface in the right order to keep the layout sane
-void
-OrganizeCollectionDialog::toggleCustomScheme( bool state )  //SLOT
-{
-    if( state )
-    {
-        ui->initialCheck->hide();
-        ui->filetypeCheck->hide();
-        m_filenameLayoutDialog->setVisible( true );
-    }
-    else
-    {
-        m_filenameLayoutDialog->hide();
-        ui->initialCheck->show();
-        ui->filetypeCheck->show();
-    }
-}
-
 //The Ok button should be disabled when there's no collection root selected, and when there is no .%filetype in format string
 void
 OrganizeCollectionDialog::slotEnableOk( const QString & currentCollectionRoot )
diff --git a/src/dialogs/OrganizeCollectionDialog.h b/src/dialogs/OrganizeCollectionDialog.h
index a5a8fab..cadeb7b 100644
--- a/src/dialogs/OrganizeCollectionDialog.h
+++ b/src/dialogs/OrganizeCollectionDialog.h
@@ -60,6 +60,7 @@ class AMAROK_EXPORT OrganizeCollectionDialog : public KDialog
         QString cleanPath( const QString &component ) const;
         QString buildFormatTip() const;
         QString buildFormatString() const;
+        QString commonPrefix( const QStringList &list ) const;
         void toggleDetails();
         void preview( const QString &format );
         void update( int dummy );
@@ -73,7 +74,6 @@ class AMAROK_EXPORT OrganizeCollectionDialog : public KDialog
         Meta::TrackList m_allTracks;
 
     private slots:
-        void toggleCustomScheme( bool state );
         void slotEnableOk( const QString & currentCollectionRoot );
 };

Hello,

I updated to amarok 2.3.0 today and while the bug seems to be vanished for universal media storage class (flash disks and similar devices), the problem persists for iPods.

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

commit 32f0246e55029c4703c283519396d0f764f24e29
Author: Casey Link <unnamedrambler@gmail.com>
Date:   Sat Mar 27 18:06:08 2010 -0400

    Limit the max number of concurrent transfer jobs to ipods to 1, that is,
    only one transfer job can be active at once. This should remedy the
    problem of 150 kio jobs being started and slowing disk i/o to a crawl.
    CCBUG: 218152

diff --git a/src/collection/ipodcollection/handler/IpodHandler.cpp b/src/collection/ipodcollection/handler/IpodHandler.cpp
index c86119a..f3563fc 100644
--- a/src/collection/ipodcollection/handler/IpodHandler.cpp
+++ b/src/collection/ipodcollection/handler/IpodHandler.cpp
@@ -68,6 +68,14 @@ extern "C" {
 using namespace Meta;
 
 /// IpodHandler
+// Define the maximum number of concurrent kio jobs allowed.
+// This used to be 150, but ipods don't handle parallel
+// writes well, so by forcing this constant to 1 the jobs
+// are run sequentially.
+// BUG: 218152
+#ifndef IPOD_MAX_CONCURRENT_JOBS
+#define IPOD_MAX_CONCURRENT_JOBS 1
+#endif
 
 IpodHandler::IpodHandler( IpodCollection *mc, const IpodDeviceInfo *deviceInfo )
     : MediaDeviceHandler( mc )
@@ -1276,9 +1284,9 @@ IpodHandler::kioCopyTrack( const KUrl &src, const KUrl &dst )
 
     KIO::CopyJob *job = KIO::copy( src, dst, KIO::HideProgressInfo );
     job->setDefaultPermissions(true);
-    m_jobcounter++;
+    ++m_jobcounter;
 
-    if( m_jobcounter < 150 )
+    if( m_jobcounter < IPOD_MAX_CONCURRENT_JOBS )
         copyNextTrackToDevice();
 
 
@@ -1308,10 +1316,10 @@ IpodHandler::fileTransferred( KJob *job )  //SLOT
         return;
     }
 
-    // Limit max number of jobs to 150, make sure more tracks left
+    // Limit max number of jobs to IPOD_MAX_CONCURRENT_JOBS, make sure more tracks left
     // to copy
     debug() << "Tracks to copy still remain";
-    if( m_jobcounter < 150 )
+    if( m_jobcounter < IPOD_MAX_CONCURRENT_JOBS )
     {
         debug() << "Jobs: " << m_jobcounter;
         copyNextTrackToDevice();
@@ -1344,7 +1352,7 @@ IpodHandler::deleteFile( const KUrl &url )
 
     m_jobcounter++;
 
-    if( m_jobcounter < 150 )
+    if( m_jobcounter < IPOD_MAX_CONCURRENT_JOBS )
         removeNextTrackFromDevice();
 
     connect( job, SIGNAL( result( KJob * ) ),
@@ -1362,10 +1370,10 @@ IpodHandler::fileDeleted( KJob *job )  //SLOT
 
     m_jobcounter--;
 
-    // Limit max number of jobs to 150, make sure more tracks left
+    // Limit max number of jobs to IPOD_MAX_CONCURRENT_JOBS, make sure more tracks left
     // to delete
     debug() << "Tracks to delete still remain";
-    if( m_jobcounter < 150 )
+    if( m_jobcounter < IPOD_MAX_CONCURRENT_JOBS )
     {
         debug() << "Jobs: " << m_jobcounter;
         removeNextTrackFromDevice();

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

Looks like this one is fixed.  We can re-open if there are still problems.

The commit in this thread only fixes the bug for USB Mass Storage Devices. iPods and MTP devices still suffer.

Changing target.

ipods and ums devices should no longer suffer from this bug.. as my above commits indicates.

Yes, my bad. I've overlooked to the report and didn't see the new commit, will be more careful next time. I'll test the patch tonight and update the ticket.

Thanks for the heads up Casey.

As of Amarok 2.3.1 Beta 1 (2.3.0.90?) the file copy speed for my iPod Nano 2G and USB sticks is nominal again. I'm closing this bug ticket. Sorry again for overlooking the last patch and reopening the ticket and the associated noise.