Bug 104264 - amarok displays misleading error message (musicbrainz support)
Summary: amarok displays misleading error message (musicbrainz support)
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: general (show other bugs)
Version: 1.2.2
Platform: openSUSE Linux
: NOR normal
Target Milestone: ---
Assignee: Amarok Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-04-20 11:55 UTC by Stephan Kulow
Modified: 2006-06-11 12:32 UTC (History)
0 users

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 Stephan Kulow 2005-04-20 11:55:35 UTC
Reported to SUSE by Ulrich Hecht uli suse.de: 
When trying to use the "Fill-In Tags Using MusicBrainz" button on file types   
that are not supported by libtunepimp or for which the necessary libtunepimp   
plugin is not installed, amarok pops up a window saying "The track was not   
found in the MusicBrainz database." instead of passing libtunepimp's error   
message.
Comment 1 Marshalleq 2005-06-18 01:21:24 UTC
Confrimed this was happening for me in Suse 9.3 because I did not have mad-libtunepimp installed.  I had no idea what was causing it until I saw this message.
Comment 2 Matthieu Faure 2005-11-07 11:13:41 UTC
I confirm this behaviour on Mandriva 2006 (x86_64)
Comment 3 François Obada 2005-12-11 18:48:55 UTC
I confirm this behavior too, this time in KDE 3.5 shipped in Kubuntu Breezy (5.10), for all tracks including those whose tags are nearly complete and accurate.
Comment 4 Alexandre Oliveira 2006-03-22 01:13:51 UTC
SVN commit 521240 by aoliveira:

Show errors returned by tunepimp (musicbrainz).
Unfortunately, it will only work for our most important case (tunepimp has no mp3 support) with a still to be released tunepimp version (probably 0.4.3), due to a bug in other tunepimp versions.
BUG: 104264


 M  +6 -3      ktrm.cpp  
 M  +1 -1      ktrm.h  
 M  +18 -14    tagdialog.cpp  
 M  +3 -3      tagdialog.h  


--- trunk/extragear/multimedia/amarok/src/ktrm.cpp #521239:521240
@@ -454,6 +454,7 @@
     KTRMLookupPrivate() :
         fileId(-1) {}
     QString file;
+    QString errorString;
     KTRMResultList results;
     int fileId;
     bool autoDelete;
@@ -642,9 +643,11 @@
 {
 #if HAVE_TUNEPIMP
     debug() << k_funcinfo << d->file << endl;
+    track_t track = tp_GetTrack(KTRMRequestHandler::instance()->tunePimp(), d->fileId);
     char error[1000];
-    tp_GetError( KTRMRequestHandler::instance()->tunePimp(), error, 1000);
-    debug() << error << endl;
+    tr_GetError( track, error, 1000);
+    debug() << "Error: " << error << endl;
+    d->errorString = error;
     d->results.clear();
     finished();
 #endif
@@ -667,7 +670,7 @@
 
 {
 #if HAVE_TUNEPIMP
-    emit sigResult( results() );
+    emit sigResult( results(), d->errorString );
 
     if(d->autoDelete)
         delete this;
--- trunk/extragear/multimedia/amarok/src/ktrm.h #521239:521240
@@ -122,7 +122,7 @@
 Q_OBJECT
 
 signals:
-    void sigResult( KTRMResultList );
+    void sigResult( KTRMResultList, QString );
 
 public:
     /**
--- trunk/extragear/multimedia/amarok/src/tagdialog.cpp #521239:521240
@@ -93,7 +93,7 @@
 void
 TagDialog::setTab( int id )
 {
-    kTabWidget->setCurrentPage( id );    
+    kTabWidget->setCurrentPage( id );
 }
 
 
@@ -280,7 +280,7 @@
 
     m_mbTrack = m_bundle.url().path();
     KTRMLookup* ktrm = new KTRMLookup( m_mbTrack, true );
-    connect( ktrm, SIGNAL( sigResult( KTRMResultList ) ), SLOT( queryDone( KTRMResultList ) ) );
+    connect( ktrm, SIGNAL( sigResult( KTRMResultList, QString ) ), SLOT( queryDone( KTRMResultList, QString ) ) );
 
     pushButton_musicbrainz->setEnabled( false );
     pushButton_musicbrainz->setText( i18n( "Generating audio fingerprint..." ) );
@@ -289,22 +289,26 @@
 }
 
 void
-TagDialog::queryDone( KTRMResultList results ) //SLOT
+TagDialog::queryDone( KTRMResultList results, QString error ) //SLOT
 {
 #if HAVE_TUNEPIMP
 
-    if ( !results.isEmpty() )
-    {
-        TrackPickerDialog* t = new TrackPickerDialog(m_bundle.url().filename(),results,this);
-        t->show();
+    if ( !error.isEmpty() ) {
+        KMessageBox::sorry( this, i18n( "Tunepimp (MusicBrainz tagging library) returned the following error: \"%1\"." ).arg(error) );
     }
-    else
-        KMessageBox::sorry( this, i18n( "The track was not found in the MusicBrainz database." ) );
+    else {
+        if ( !results.isEmpty() )
+        {
+            TrackPickerDialog* t = new TrackPickerDialog(m_bundle.url().filename(),results,this);
+            t->show();
+        }
+        else
+            KMessageBox::sorry( this, i18n( "The track was not found in the MusicBrainz database." ) );
+        QApplication::restoreOverrideCursor();
+        pushButton_musicbrainz->setEnabled( true );
+        pushButton_musicbrainz->setText( m_buttonMbText );
+    }
 
-    QApplication::restoreOverrideCursor();
-    pushButton_musicbrainz->setEnabled( true );
-    pushButton_musicbrainz->setText( m_buttonMbText );
-
 #endif
 }
 
@@ -369,7 +373,7 @@
     kComboBox_album->completionObject()->insertItems( albums );
     kComboBox_album->completionObject()->setIgnoreCase( true );
     kComboBox_album->setCompletionMode( KGlobalSettings::CompletionPopup );
-    
+
     const QStringList composers = CollectionDB::instance()->composerList();
     kComboBox_composer->insertStringList( composers );
     kComboBox_composer->completionObject()->insertItems( composers );
--- trunk/extragear/multimedia/amarok/src/tagdialog.h #521239:521240
@@ -38,9 +38,9 @@
         TagDialog( const KURL::List list, QWidget* parent = 0 );
         TagDialog( const MetaBundle& mb, PlaylistItem* item, QWidget* parent = 0 );
         ~TagDialog();
-        
+
         void setTab( int id );
-        
+
         friend class TagSelect;
 
     signals:
@@ -61,7 +61,7 @@
         void musicbrainzQuery();
         void guessFromFilename();
         void setFileNameSchemes();
-        void queryDone( KTRMResultList results );
+        void queryDone( KTRMResultList results, QString error );
         void fillSelected( KTRMResult selected );
 
     private:
Comment 5 Alexandre Oliveira 2006-03-22 01:43:29 UTC
SVN commit 521242 by aoliveira:

backporting tunepimp error handling
CCBUG: 104264


 M  +2 -0      ChangeLog  
 M  +6 -3      src/ktrm.cpp  
 M  +1 -1      src/ktrm.h  
 M  +13 -8     src/tagdialog.cpp  
 M  +1 -1      src/tagdialog.h  


--- branches/stable/extragear/multimedia/amarok/ChangeLog #521241:521242
@@ -8,6 +8,8 @@
     * Support for libtunepimp 0.4. (BR 94988)
 
   BUGFIXES:
+    * On libtunepimp returned errors, show the proper error message,
+      instead of a misleading one. (BR 104264)
     * Fix preamp and frequency band scaling in the xine equalizer. Patch by
       Tobias Knieper <webmaster@micekiller.de>. (BR 116633)
     * Fixed a huge memory leak when using xine-engine with crossfading.
--- branches/stable/extragear/multimedia/amarok/src/ktrm.cpp #521241:521242
@@ -454,6 +454,7 @@
     KTRMLookupPrivate() :
         fileId(-1) {}
     QString file;
+    QString errorString;
     KTRMResultList results;
     int fileId;
     bool autoDelete;
@@ -642,9 +643,11 @@
 {
 #if HAVE_TUNEPIMP
     debug() << k_funcinfo << d->file << endl;
+    track_t track = tp_GetTrack(KTRMRequestHandler::instance()->tunePimp(), d->fileId);
     char error[1000];
-    tp_GetError( KTRMRequestHandler::instance()->tunePimp(), error, 1000);
-    debug() << error << endl;
+    tr_GetError( track, error, 1000);
+    debug() << "Error: " << error << endl;
+    d->errorString = error;
     d->results.clear();
     finished();
 #endif
@@ -667,7 +670,7 @@
 
 {
 #if HAVE_TUNEPIMP
-    emit sigResult( results() );
+    emit sigResult( results(), d->errorString );
 
     if(d->autoDelete)
         delete this;
--- branches/stable/extragear/multimedia/amarok/src/ktrm.h #521241:521242
@@ -122,7 +122,7 @@
 Q_OBJECT
 
 signals:
-    void sigResult( KTRMResultList );
+    void sigResult( KTRMResultList, QString );
 
 public:
     /**
--- branches/stable/extragear/multimedia/amarok/src/tagdialog.cpp #521241:521242
@@ -168,7 +168,7 @@
 
     m_mbTrack = m_bundle.url().path();
     KTRMLookup* ktrm = new KTRMLookup( m_mbTrack, true );
-    connect( ktrm, SIGNAL( sigResult( KTRMResultList ) ), SLOT( queryDone( KTRMResultList ) ) );
+    connect( ktrm, SIGNAL( sigResult( KTRMResultList, QString ) ), SLOT( queryDone( KTRMResultList, QString ) ) );
 
     pushButton_musicbrainz->setEnabled( false );
     pushButton_musicbrainz->setText( i18n( "Generating audio fingerprint..." ) );
@@ -177,17 +177,22 @@
 }
 
 void
-TagDialog::queryDone( KTRMResultList results ) //SLOT
+TagDialog::queryDone( KTRMResultList results, QString error ) //SLOT
 {
 #if HAVE_TUNEPIMP
 
-    if ( !results.isEmpty() )
-    {
-        TrackPickerDialog* t = new TrackPickerDialog(m_bundle.url().filename(),results,this);
-        t->show();
+    if ( !error.isEmpty() ) {
+        KMessageBox::sorry( this, i18n( "Tunepimp (MusicBrainz tagging library) returned the following error: \"%1\"." ).arg(error) );
     }
-    else
-        KMessageBox::sorry( this, i18n( "The track was not found in the MusicBrainz database." ) );
+    else {
+        if ( !results.isEmpty() )
+        {
+            TrackPickerDialog* t = new TrackPickerDialog(m_bundle.url().filename(),results,this);
+            t->show();
+        }
+        else
+            KMessageBox::sorry( this, i18n( "The track was not found in the MusicBrainz database." ) );
+    }
 
     QApplication::restoreOverrideCursor();
     pushButton_musicbrainz->setEnabled( true );
--- branches/stable/extragear/multimedia/amarok/src/tagdialog.h #521241:521242
@@ -39,7 +39,7 @@
         void checkModified();
 
         void musicbrainzQuery();
-        void queryDone( KTRMResultList results );
+        void queryDone( KTRMResultList results, QString error );
         void fillSelected( KTRMResult selected );
 
     private: