| Summary: | Incorrect encoding in id3v1 tags patch | ||
|---|---|---|---|
| Product: | [Applications] amarok | Reporter: | Alexander Darovsky <adarovsky> |
| Component: | general | Assignee: | Amarok Developers <amarok-bugs-dist> |
| Status: | RESOLVED FIXED | ||
| Severity: | wishlist | ||
| Priority: | NOR | ||
| Version: | 1.1.1 | ||
| Target Milestone: | --- | ||
| Platform: | Gentoo Packages | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | ||
| Sentry Crash Report: | |||
| Attachments: |
The same patch as an attachment
A correct patch. Just tested once more |
||
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
Oh! I forgot to remove #include <stdio.h>! sorry... 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..
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.
Hi, please see if this doesn't already exist in 1.2-CVS. Thanks. Alexander, is the patch still applicable to 1.3 betas? Recoding features have been completely removed in amaroK SVN. So this report is no longer valid. |
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()