Bug 95344 - Mass tagging slow, eventually crash
Summary: Mass tagging slow, eventually crash
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: general (show other bugs)
Version: 1.2-CVS
Platform: Debian testing Linux
: NOR crash
Target Milestone: ---
Assignee: Amarok Developers
URL:
Keywords:
: 94045 97502 (view as bug list)
Depends on:
Blocks:
 
Reported: 2004-12-17 17:20 UTC by skrald
Modified: 2006-06-11 12:32 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
gdb output (39.16 KB, text/plain)
2005-01-08 22:31 UTC, Thomas Lanquetin
Details

Note You need to log in before you can comment on or make changes to this bug.
Description skrald 2004-12-17 17:20:51 UTC
Version:           CVS from 2004-12-15 (1.2-beta2?) (using KDE KDE 3.3.1)
Installed from:    Debian testing/unstable Packages
Compiler:          gcc (GCC) 3.3.5 (Debian 1:3.3.5-3) 
OS:                Linux

The following behavior must be unintended and therefore a bug:

1) In the playlist, I select a bunch of songs by the same artist (135 songs).
2) I rightclick on a song and edit the artist name
3) After 90 seconds only 35 tags was written.

That's 2.6 seconds per song, which is quite much.

Furthermore: When many of the tags are written, amarok crashes with a segmentation fault.
Comment 1 Max Howell 2004-12-24 03:04:40 UTC
tag-writing speed is taglib-dependent. Taglib can take some time to write tags, depending on the file, but I should add taglib is the best and fastest tagging library there is.

The crash is almost certainly taglib's fault. A backtrace would reveal whether this is the case. Whatever I can't fix anything without a bt. Thanks.
Comment 2 Krzysztof Wozniak 2004-12-25 21:33:42 UTC
> 
> 
> ------- Additional Comments From max.howell methylblue com  2004-12-24 03:04 -------
> tag-writing speed is taglib-dependent. Taglib can take some time to write tags, depending on the file, but I should add taglib is the best and fastest tagging library there is.
> 
> The crash is almost certainly taglib's fault. A backtrace would reveal whether this is the case. Whatever I can't fix anything without a bt. Thanks.
> 
> 

May be you shold consider adding progress bar during tag writing. 
Updating 40 or more tags is quite time consuming, and during this 
process amaroK UI freezes and it may be annoying for some users.

I observed that this extreme long tag writing is connected to one type 
of mp3 file :   mp3 without any tags !
If tag is present in mp3 file, tags are updated very quicly !

Comment 3 Thomas Lanquetin 2005-01-08 22:30:16 UTC
I witnessed the same kind of behaviour on amarok 1.2-beta3 (and the crash already happened in previous versions). I noticed also that the crash can happen when only one tag is being updated. Yet, I didn't notice the bug in Juk so I wonder if taglib can still be suspected. I have tried to make a backtrace, but it's my first time so I don't know if it's a valid one. Anyway, I join the gdb output, and hope it can be of any help. If it can't could you redirect me to some documentation where I could learn how to make it, because that bugs is really annoying meand I wish I could help you get rid of it.
Comment 4 Thomas Lanquetin 2005-01-08 22:31:41 UTC
Created attachment 8994 [details]
gdb output
Comment 5 Mark Kretschmann 2005-01-20 09:28:45 UTC

*** This bug has been marked as a duplicate of 94045 ***
Comment 6 Mark Kretschmann 2005-01-20 09:30:09 UTC
*** Bug 97502 has been marked as a duplicate of this bug. ***
Comment 7 Mark Kretschmann 2005-01-20 09:32:22 UTC
closed wrong report, sry
Comment 8 Mark Kretschmann 2005-01-20 09:33:52 UTC
*** Bug 94045 has been marked as a duplicate of this bug. ***
Comment 9 Mark Kretschmann 2005-01-22 13:36:22 UTC
CVS commit by markey: 

FIX: Frequent crashes when writing tags.

Problem was, the TagWriter thread was calling CollectionDB::updateTags(), which was directly calling CollectionView::renderView(), still in the thread context. I changed it so the GUI operation is decoupled from the thread.

Please test.

BUG: 95344
CCMAIL: patrol@sinus.cz


  M +1 -0      ChangeLog   1.465
  M +33 -28    src/collectionbrowser.cpp   1.292
  M +3 -2      src/collectionbrowser.h   1.121
  M +5 -2      src/collectiondb.cpp   1.273


--- kdeextragear-1/amarok/ChangeLog  #1.464:1.465
@@ -5,4 +5,5 @@
 
 VERSION 1.2-beta4:
+  FIX: Frequent crashes when writing tags. (BR 95344)
   FIX: CoverManager updates its status display correctly.
   FIX: "isPlaying" DCOP function now works correctly. (BR 90894)

--- kdeextragear-1/amarok/src/collectionbrowser.cpp  #1.291:1.292
@@ -233,36 +233,10 @@ CollectionView::~CollectionView() {
 }
 
+
 //////////////////////////////////////////////////////////////////////////////////////////
-// private slots
+// public slots
 //////////////////////////////////////////////////////////////////////////////////////////
 
 void
-CollectionView::setupDirs()  //SLOT
-{
-    KDialogBase dialog( this, 0, false );
-    kapp->setTopWidget( &dialog );
-    dialog.setCaption( kapp->makeStdCaption( i18n("Configure Collection") ) );
-
-    CollectionSetup *setup = new CollectionSetup( &dialog );
-    dialog.setMainWidget( setup );
-    dialog.showButtonApply( false );
-    dialog.adjustSize();
-    // Make the dialog a bit wider, default is too narrow to be useful
-    dialog.resize( dialog.width() + 50, dialog.height() );
-
-    if ( dialog.exec() != QDialog::Rejected )
-    {
-        const bool rescan = ( AmarokConfig::collectionFolders() != setup->dirs() );
-        setup->writeConfig();
-
-        if ( rescan )
-            CollectionDB::instance()->startScan();
-
-        m_parent->m_scanAction->setEnabled( !AmarokConfig::monitorChanges() );
-    }
-}
-
-
-void
 CollectionView::renderView()  //SLOT
 {
@@ -404,4 +378,35 @@ CollectionView::renderView()  //SLOT
 
 
+//////////////////////////////////////////////////////////////////////////////////////////
+// private slots
+//////////////////////////////////////////////////////////////////////////////////////////
+
+void
+CollectionView::setupDirs()  //SLOT
+{
+    KDialogBase dialog( this, 0, false );
+    kapp->setTopWidget( &dialog );
+    dialog.setCaption( kapp->makeStdCaption( i18n("Configure Collection") ) );
+
+    CollectionSetup *setup = new CollectionSetup( &dialog );
+    dialog.setMainWidget( setup );
+    dialog.showButtonApply( false );
+    dialog.adjustSize();
+    // Make the dialog a bit wider, default is too narrow to be useful
+    dialog.resize( dialog.width() + 50, dialog.height() );
+
+    if ( dialog.exec() != QDialog::Rejected )
+    {
+        const bool rescan = ( AmarokConfig::collectionFolders() != setup->dirs() );
+        setup->writeConfig();
+
+        if ( rescan )
+            CollectionDB::instance()->startScan();
+
+        m_parent->m_scanAction->setEnabled( !AmarokConfig::monitorChanges() );
+    }
+}
+
+
 void
 CollectionView::scanStarted() // SLOT

--- kdeextragear-1/amarok/src/collectionbrowser.h  #1.120:1.121
@@ -89,6 +89,4 @@ class CollectionView : public KListView
 
         static CollectionView* instance() { return m_instance; }
-        /** Rebuilds and displays the treeview by querying the database. */
-        void renderView();
         void setFilter( const QString &filter ) { m_filter = filter; }
         QString filter() { return m_filter; }
@@ -96,4 +94,7 @@ class CollectionView : public KListView
 
     public slots:
+        /** Rebuilds and displays the treeview by querying the database. */
+        void renderView();
+
         void setTreeMode() { setViewMode( modeTreeView ); };
         void setFlatMode() { setViewMode( modeFlatView ); };

--- kdeextragear-1/amarok/src/collectiondb.cpp  #1.272:1.273
@@ -24,4 +24,5 @@
 #include <qfile.h>
 #include <qimage.h>
+#include <qtimer.h>
 
 #include <kapplication.h>
@@ -1357,6 +1358,8 @@ CollectionDB::updateTags( const QString 
     }
 
-    if ( updateView )    //update the collection browser
-      CollectionView::instance()->renderView();
+    // Update the Collection-Browser view,
+    // using QTimer to make sure we don't manipulate the GUI from a thread
+    if ( updateView )
+        QTimer::singleShot( 0, CollectionView::instance(), SLOT( renderView() ) );
 }