Bug 106474 - adding aac/m4a files to the collection
Summary: adding aac/m4a files to the collection
Status: RESOLVED NOT A BUG
Alias: None
Product: amarok
Classification: Applications
Component: general (show other bugs)
Version: 1.2.4
Platform: Gentoo Packages Linux
: NOR wishlist
Target Milestone: ---
Assignee: Amarok Developers
URL:
Keywords:
: 107143 112842 120081 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-05-30 01:08 UTC by Gilles Gagniard
Modified: 2006-12-28 04:14 UTC (History)
4 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 Gilles Gagniard 2005-05-30 01:08:41 UTC
Version:           1.2.4 (using KDE KDE 3.4.0)
Installed from:    Gentoo Packages
Compiler:          gcc (GCC) 3.4.3-20050110 (Gentoo Linux 3.4.3.20050110, ssp-3.4.3.20050110-0, pie-8.7.7) 
OS:                Linux

On this bug database, I found a patch for taglib-1.3.1 here :

https://bugs.kde.org/show_bug.cgi?id=89045

that adds aac/m4a (and also wma) tag reading support to taglib-1.3.1; it seems to work perfectly here, and naturally amarok now can read (but not write ...) tags in those files.

Unfortunately, those files still can't be added to the collection (only mp3/ogg/... are added). It would be great if aac/m4a/wma files could be added in the collection, as this patch will, I think, be added in the next taglib version; in old versions of amarok, there was some simple 'filter' to modify in src/amarok/collectionreader.cpp, but it seems that it changed, and I didn't really read the code for now. If the proposition is likely to be accepted, I can write the patch myself, as I don't think it should be very difficult ...
Comment 1 Thiago Macieira 2005-05-30 01:33:37 UTC
amaroK can't. Not until that patch is accepted into TagLib, and you may have some difficulty convincing the maintainer of that.
Comment 2 Alexandre Oliveira 2005-05-30 01:53:01 UTC
amaroK doesn't check for the type of the file, it just try to read the tags using taglib, and if this fails the file is not added.
So, if adding to the collection doesn't work, then there's something wrong with that taglib patch, it might be incomplete or something.
Comment 3 Ian Monroe 2005-05-30 01:58:27 UTC
I don't see why the m4a files wouldn't be added to the collection if taglib is able to edit them. Perhaps the Taglib patch doesn't work fully? 

Tracing through the code, MetaBundle::readTags in metabundle.cpp is the relevant function. It starts out with m_isValidMedia being false I believe and then makes it true if the object created by Taglib::FileRef returns an object that isn't isNull. Whether this boolean is true or determines whether it is inserted into the database. The API for taglib explictly says not to use FileRef though. :)
Comment 4 Thiago Macieira 2005-06-10 06:24:42 UTC
*** Bug 107143 has been marked as a duplicate of this bug. ***
Comment 5 Alexandre Oliveira 2005-09-18 20:18:01 UTC
*** Bug 112842 has been marked as a duplicate of this bug. ***
Comment 6 Alexandre Oliveira 2006-01-14 04:17:40 UTC
*** Bug 120081 has been marked as a duplicate of this bug. ***
Comment 7 Ariel 2006-12-24 09:23:02 UTC
I had some .ra (RealAudio) files in my music tree which amarok cannot tag due to the TagLib library not supporting them.  That's OK.  I don't care.

However, amarok aborts in the middle of "scanning collection" saying there are "too many errors".

It would be much nicer if it could simply continue building the collection and just skip those files it cannot tag.  Aborting is way too fussy/harsh, especially when the failures are less than ~5% of the whole collection and have a recognizable unsupported format.

On the high side, it was nice to get a popup detailing files that fail so I could get them out of the tree and finally get the collection db built.

I would vote for being less fussy about these failures.
Comment 8 Martin Aumueller 2006-12-27 23:36:42 UTC
SVN commit 617072 by aumuell:

restart collection scanner as long as not more than 5 % of the scanned files make it crash
CCBUG: 106474


 M  +2 -0      ChangeLog  
 M  +6 -2      src/scancontroller.cpp  
 M  +2 -0      src/scancontroller.h  


--- trunk/extragear/multimedia/amarok/ChangeLog #617071:617072
@@ -41,6 +41,8 @@
       you move and rename them.
 
   CHANGES:
+    * Restart collection scanner as long as not more than 5 % of the files
+      make it crash. (BR 106474)
     * Ensure the first selected item in the Collection Browser stays visible
       when the search field is cleared using the clear button.
     * Duplicate filenames are now allowed on MTP media devices if the files are
--- trunk/extragear/multimedia/amarok/src/scancontroller.cpp #617071:617072
@@ -68,6 +68,7 @@
     , m_lastCommandPaused( false )
     , m_isPaused( false )
     , m_tablesCreated( false )
+    , m_scanCount( 0 )
 {
     DEBUG_BLOCK
 
@@ -107,7 +108,7 @@
                                   "<i>" + m_crashedFiles.join( "<br>" ) + "</i>",
                                   i18n( "Collection Scan Report" ) );
     }
-    else if( m_crashedFiles.size() >= MAX_RESTARTS ) {
+    else if( !isAborted() ) {
         KMessageBox::error( 0, i18n( "<p>Sorry, the Collection Scan was aborted, since too many problems were encountered.</p>" ) +
                             "<p>Advice: A common source for this problem is a broken 'TagLib' package on your computer. Replacing this package may help fixing the issue.</p>"
                             "<p>The following files caused problems:</p>" +
@@ -304,7 +305,8 @@
 
         }
         else {
-            if( m_crashedFiles.size() < MAX_RESTARTS ) {
+            if( m_crashedFiles.size() <= MAX_RESTARTS ||
+                    m_crashedFiles.size() <= (m_scanCount * MAX_FAILURE_PERCENTAGE) / 100 ) {
                 kapp->postEvent( this, new RestartEvent() );
                 sleep( 3 );
             }
@@ -454,6 +456,8 @@
             m_filesAdded[bundle.uniqueId()] = bundle.url().path();
             m_fileMapsMutex.unlock();
         }
+
+        m_scanCount++;
     }
 
     else if( localName == "folder" ) {
--- trunk/extragear/multimedia/amarok/src/scancontroller.h #617071:617072
@@ -103,6 +103,7 @@
 
         // Member variables:
         static const uint MAX_RESTARTS = 20;
+        static const uint MAX_FAILURE_PERCENTAGE = 5;
 
         KProcIO* m_scanner;
         QStringList m_folders;
@@ -132,6 +133,7 @@
         bool m_lastCommandPaused;
         bool m_isPaused;
         bool m_tablesCreated;
+        int m_scanCount;
 };
 
 
Comment 9 Martin Aumueller 2006-12-27 23:38:00 UTC
To #7:
Ariel, could you please open another bug report and make the .ra file which causes the crashes available to us? Thanks!
Comment 10 Ariel 2006-12-28 04:14:11 UTC
Sure. Martin, thanks for the partial fix.

I just filed a separate bug for the .ra format issue here:

        http://bugs.kde.org/show_bug.cgi?id=139299