Bug 180164

Summary:	"Guess tags" hangs with high CPU use
Product:	[Applications] amarok	Reporter:	Nicos Gollan <gtdev>
Component:	general	Assignee:	Amarok Developers <amarok-bugs-dist>
Status:	RESOLVED FIXED
Severity:	crash	CC:	teo
Priority:	NOR
Version:	2.0
Target Milestone:	---
Platform:	Compiled Sources
OS:	Linux
Latest Commit:		Version Fixed In:
Attachments:	Backtrace generated by sending a SIGSEGV to the hanging application Fix for case conversion code in the tag guesser New patch Even newer patch, should (almost) comply with coding guidelines

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.