Bug 98745 - Incorrect encoding in id3v1 tags patch
Summary: Incorrect encoding in id3v1 tags patch
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: general (show other bugs)
Version: 1.1.1
Platform: Gentoo Packages Linux
: NOR wishlist
Target Milestone: ---
Assignee: Amarok Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-02-07 07:15 UTC by Alexander Darovsky
Modified: 2006-06-11 12:32 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments
The same patch as an attachment (1.78 KB, patch)
2005-02-07 07:18 UTC, Alexander Darovsky
Details
A correct patch. Just tested once more (1.78 KB, patch)
2005-02-07 07:44 UTC, Alexander Darovsky
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Darovsky 2005-02-07 07:15:38 UTC
Version:           1.1.1 (using KDE KDE 3.3.2)
Installed from:    Gentoo Packages
Compiler:          gcc 3.3.3 
OS:                Linux

I have found this issue and want to send you a patch.
I hope, it will work correctly and won't break anything.

It is supposed that if user enabled custom recoding 
of ID3v1 tags, then metainfo is stored in non-latin1
encoding. I replaced TStringToQString macro to
detect characters > 0x80, which have zero subset and
recode them as specified in settings

Here is the patch. It's tiny

 
--- amarok-1.1.1.old/amarok/src/metabundle.cpp 2004-10-09 18:52:09.000000000 +0600
+++ amarok-1.1.1/amarok/src/metabundle.cpp 2005-02-06 15:32:14.887009880 +0500
@@ -5,8 +5,10 @@
 #include "collectiondb.h"
 #include "metabundle.h"
 #include "playlistitem.h"
+#include "amarokconfig.h"
 
 #include <qfile.h>
+#include <qtextcodec.h>
 
 #include <kfilemetainfo.h>
 
@@ -16,12 +18,43 @@
 #include <taglib/tag.h>
 #include <taglib/tstring.h>
 
+#include <stdio.h>
 
 /*
  * This class is not very complete, it fits our needs as they stand currently
  * If it doesn't work for you in some way, extend it sensibly :)
  */
 
+#undef TStringToQString
+
+QString TStringToQString( const TagLib::String& s )
+{
+    bool doCustomDecode = false;
+
+    if( AmarokConfig::recodeID3v1Tags() ) {
+        for( TagLib::String::ConstIterator i = s.begin(); i != s.end(); ++i ) {
+            // The following happens when non-latin1 string
+            // is stored in TagLib::String's UTF-16 and is not
+            // converted to Unicode properly. In this case, we should
+            // recode it as specified in Amarok options
+            if( *i > 0x80 && !(0xff00 & *i) )
+                doCustomDecode = true;
+                break;
+        }
+
+    QTextCodec * codec = 0;
+    if( doCustomDecode )
+        codec = QTextCodec::codecForIndex ( AmarokConfig::tagEncoding() );
+        
+    // Maybe codec index is stale after Qt upgrade, or 
+    // AmarokConfig::recodeID3v1Tags() == false. In this case
+    // fall back to TagLib's TStringToQString default behavior
+    if( codec )
+        return codec->toUnicode( s.toCString() );
+    else
+        return QString::fromUtf8(s.toCString(true));
+}
+
 
 //FIXME these aren't i18n'd
 //the point here is to force sharing of these strings returned from prettyBitrate()
Comment 1 Alexander Darovsky 2005-02-07 07:18:51 UTC
Created attachment 9456 [details]
The same patch as an attachment

The safe, I hope, way to detect and recode id3v1 metainfo.

Unfortunately, I haven't found a way to deduce if given TagLib::Tag is
.mp3 metainfo with id3v1 tag. So, I recode all suspectible strings
Comment 2 Alexander Darovsky 2005-02-07 07:27:56 UTC
Oh!
I forgot to remove #include <stdio.h>!
sorry...
Comment 3 Alexander Darovsky 2005-02-07 07:34:04 UTC
Comment on attachment 9456 [details]
The same patch as an attachment

awfull...
forgotten stdio and 
bracket after IF...
I haven't even checked it.. sorry..
Comment 4 Alexander Darovsky 2005-02-07 07:44:36 UTC
Created attachment 9457 [details]
A correct patch. Just tested once more

This patch is correct.

I tested it with different files, partially with id3v1,
and partially with id3v2 headers. It displays everything correctly.
Comment 5 Max Howell 2005-02-07 12:30:04 UTC
Hi, please see if this doesn't already exist in 1.2-CVS. Thanks.
Comment 6 Seb Ruiz 2005-07-20 08:38:16 UTC
Alexander, is the patch still applicable to 1.3 betas?
Comment 7 Mark Kretschmann 2005-11-19 17:17:06 UTC
Recoding features have been completely removed in amaroK SVN. So this report is no longer valid.