Bug 180164 - "Guess tags" hangs with high CPU use
Summary: "Guess tags" hangs with high CPU use
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: general (show other bugs)
Version: 2.0
Platform: Compiled Sources Linux
: NOR crash
Target Milestone: ---
Assignee: Amarok Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-01-09 18:44 UTC by Nicos Gollan
Modified: 2009-12-09 11:28 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Backtrace generated by sending a SIGSEGV to the hanging application (13.43 KB, text/plain)
2009-01-09 18:51 UTC, Nicos Gollan
Details
Fix for case conversion code in the tag guesser (3.98 KB, patch)
2009-01-09 23:45 UTC, Nicos Gollan
Details
New patch (6.39 KB, patch)
2009-01-10 19:09 UTC, Nicos Gollan
Details
Even newer patch, should (almost) comply with coding guidelines (7.88 KB, patch)
2009-01-10 19:33 UTC, Nicos Gollan
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Nicos Gollan 2009-01-09 18:44:38 UTC
Version:           2.0 (using Devel)
OS:                Linux
Installed from:    Compiled sources

From my observations, the required items are:
 * A music file (tested with MP3, on read-only filesystem)
   - without tags
   - filename formatted like "artist - title.mp3"

Then, from the playlist, I open the tag editor, and go to the "Guess tags" dialog, advanced tab, with the following options set:
 * "Edit case" with "Title case"
 * Remove trailing spaces
 * Replace underscores

Then:
 (1) use the format "[%artist] - %title" and press OK (which does not yield results).
 (2) open the guessing dialog again, just change the format to "%artist - %title" (which would yield a result), press OK

After that, Amarok, along with the KDE desktop, becomes completely unresponsive with 100% CPU use, and needs to be killed (TERM signal works).
Comment 1 Nicos Gollan 2009-01-09 18:51:10 UTC
Created attachment 30065 [details]
Backtrace generated by sending a SIGSEGV to the hanging application

Thread 1 looks promising
Comment 2 Nicos Gollan 2009-01-09 23:45:24 UTC
Created attachment 30077 [details]
Fix for case conversion code in the tag guesser

Case conversion is buggy. The patch (against r908562) provides a rewrite and slight refactoring of the relevant code.
Comment 3 Teo Mrnjavac 2009-01-10 00:46:02 UTC
SVN commit 908597 by mrnjavac:

Rewrite of the case conversion code in TagGuesser and fix for an infinite loop bug.
Thanks Nicos Gollan for the patch.
BUG: 180164


 M  +51 -29    TagGuesser.cpp  
 M  +2 -0      TagGuesser.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=908597
Comment 4 Nicos Gollan 2009-01-10 19:09:59 UTC
Created attachment 30107 [details]
New patch

This patch is a slight refinement. The case conversion code is now completely outside the TagGuesser. There's mostly a single change: apostrophes are now handled as word characters, so e.g. "that's" will not be converted to "That'S".

The whole thing is a language-specific bag of fleas, but it should now be more maintainable. I also have a set of unit tests using Qt's framework floating around, if anybody should be interested.
Comment 5 Nicos Gollan 2009-01-10 19:33:25 UTC
Created attachment 30108 [details]
Even newer patch, should (almost) comply with coding guidelines

I hope this one is better. Should also make r908979 unnecessary.
Comment 6 Teo Mrnjavac 2009-01-10 20:10:26 UTC
Your patch is being reviewed. We appreciate your fix to adhere to our coding style guidelines.
Comment 7 Lydia Pintscher 2009-01-11 23:58:44 UTC
reopening so we don't forget about the patch
Comment 8 Teo Mrnjavac 2009-01-12 00:10:23 UTC
Patch reviewed and committed, again thanks Nicos Gollan.