Bug 147223 - [PATCH] Messy Chinese ID3 tag in Creative ZenMicro using libnjb
Summary: [PATCH] Messy Chinese ID3 tag in Creative ZenMicro using libnjb
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: general (show other bugs)
Version: 1.4.6
Platform: Gentoo Packages Linux
: NOR normal
Target Milestone: ---
Assignee: Amarok Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-06-26 04:36 UTC by Kun Xi
Modified: 2007-06-29 13:52 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments
Patch to setup unicode of libnjb correctly (991 bytes, patch)
2007-06-26 04:38 UTC, Kun Xi
Details
libnjk utf8 fix (3.25 KB, patch)
2007-06-26 07:22 UTC, Kun Xi
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Kun Xi 2007-06-26 04:36:51 UTC
Version:           1.4.6 (using KDE KDE 3.5.7)
Installed from:    Gentoo Packages
Compiler:          gcc (GCC) 4.1.2 (Gentoo 4.1.2) 
OS:                Linux

When downloading tracks to Creative Zen Micro (firmware 1.11.01) using libnjb 2.2.4, the id3 tag in device is messed up. In host, id3 tag is encoded with utf8.

The reason is:
NJB_Set_Unicode( NJB_UC_UTF8 );
is called after NJB_Discover, according to the document, NJB_Set_Unicode( NJB_UC_UTF8 ) is supposed to be called BEFORE any other operations.
Comment 1 Kun Xi 2007-06-26 04:38:10 UTC
Created attachment 20961 [details]
Patch to setup unicode of libnjb correctly
Comment 2 Kun Xi 2007-06-26 07:22:07 UTC
Created attachment 20963 [details]
libnjk utf8 fix
Comment 3 Mark Kretschmann 2007-06-26 11:17:20 UTC
Thanks for the fix, Kun. You have attached two patches. It looks like the second patch obsoletes the first one, am I right?

Myself I do not have a NJB device for testing. Are you confident that your patch does not cause regressions?
Comment 4 Jeff Mitchell 2007-06-26 17:15:37 UTC
SVN commit 680565 by mitchell:

NJB devices could get corrupted ID3 tags if they contained Unicode characters.  Patch by Kun Xi <quinnxi@gmail.com>.

BUG: 147223


 M  +2 -2      ChangeLog  
 M  +1 -1      src/mediadevice/njb/njbmediadevice.cpp  
 M  +6 -7      src/mediadevice/njb/track.cpp  


--- branches/stable/extragear/multimedia/amarok/ChangeLog #680564:680565
@@ -10,8 +10,8 @@
       Gosta <gostaj@gmail.com>. (BR 142999)
 
   BUGFIXES:
-   * ID3 tags with Unicode data could become corrupted when using the NJB
-     plugin.
+   * NJB devices could have tags corrupted that contained Unicode characters.
+     Patch by Kun Xi <quinnxi@gmail.com>. (BR 147223)
    * Show OSD when changing song rating via shortcut. Patch by Tuomas Nurmi
      <tnurmi@edu.kauhajoki.fi>. (BR 146918)
    * Show the stars indicating rating with the correct size in the OSD. Patch 
--- branches/stable/extragear/multimedia/amarok/src/mediadevice/njb/njbmediadevice.cpp #680564:680565
@@ -201,9 +201,9 @@
         return true;
 
     QString genericError = i18n( "Could not connect to Nomad device" );
+    NJB_Set_Unicode( NJB_UC_UTF8 ); // I assume that UTF-8 is fine with everyone...
 
     int n;
-    NJB_Set_Unicode( NJB_UC_UTF8 ); // I assume that UTF-8 is fine with everyone...
     if( NJB_Discover( njbs, 0, &n) == -1 || n == 0 )
     {
         Amarok::StatusBar::instance()->shortLongMessage( genericError, i18n("A suitable Nomad device could not be found"), KDE::StatusBar::Error );
--- branches/stable/extragear/multimedia/amarok/src/mediadevice/njb/track.cpp #680564:680565
@@ -32,7 +32,6 @@
 /* ------------------------------------------------------------------------ */
 NjbTrack::NjbTrack( njb_songid_t* song)
 {
-
     njb_songid_frame_t* frame;
 
     m_id = song->trid;
@@ -65,7 +64,7 @@
     frame = NJB_Songid_Findframe( song, FR_ARTIST );
     if( frame )
     {
-        QString artist = frame->data.strval;
+        QString artist = QString::fromUtf8( frame->data.strval );
         artist.replace( QRegExp( "/" ), "-" );
         bundle->setArtist( artist );
     }
@@ -75,7 +74,7 @@
     frame = NJB_Songid_Findframe( song, FR_ALBUM );
     if( frame)
     {
-        QString album = frame->data.strval;
+        QString album = QString::fromUtf8( frame->data.strval );
         album.replace( QRegExp( "/" ), "-" );
         bundle->setAlbum( album );
     }
@@ -85,7 +84,7 @@
     frame = NJB_Songid_Findframe( song, FR_TITLE);
     if( frame )
     {
-        QString title = frame->data.strval;
+        QString title = QString::fromUtf8( frame->data.strval );
         title.replace( QRegExp( "/"), "-");
         bundle->setTitle( title );
     }
@@ -104,7 +103,7 @@
             bundle->setTrack( frame->data.u_int32_val );
             break;
         case NJB_TYPE_STRING:
-            bundle->setTrack( QString(frame->data.strval).toUInt() );
+            bundle->setTrack( QString::fromUtf8(frame->data.strval).toUInt() );
             break;
         default:
             bundle->setTrack( 0 );
@@ -134,7 +133,7 @@
     if( frame )
     {
         //bundle->setUrl( KURL( frame->data.strval ) );
-        filename = frame->data.strval;
+        filename = QString::fromUtf8( frame->data.strval );
 
     }
     if( filename.isEmpty() )
@@ -157,7 +156,7 @@
             bundle->setYear( frame->data.u_int32_val );
             break;
         case NJB_TYPE_STRING:
-            bundle->setYear( QString( frame->data.strval ).toInt() );
+            bundle->setYear( QString::fromUtf8( frame->data.strval ).toInt() );
             break;
         default:
             bundle->setYear( 0 );
Comment 5 Kun Xi 2007-06-29 13:52:45 UTC
Hello Mark,

The first patch fix the problem downloading mp3 track which has
unicode character in the tag.

The second patch fix that problem as well, and also fix the problem
fetching meta data from device which has unicode character in the tag.

I am quite confident with this patch, and I would like to run the
regression tests if you tell me how to do so.

Best regards,
Kun Xi

On 26 Jun 2007 09:17:21 -0000, Mark Kretschmann <kretschmann@kde.org> wrote:
[bugs.kde.org quoted mail]