Genre becomes a Numeric value

[Manfred Schwind]

Hi,

I recently tested taglib. Great piece of software! But I discovered  
something strange: I have an MP3 file whose Genre shows up e.g. as  
"Rock". But after rewriting the Tag with taglib, it becomes e.g. "17".
Is this a known bug?

I searched the Internet, and the only thing I found was a bug from  
2006 that seems to be solved:
http://bugs.gentoo.org/120968
This bug describes exactly the same problem.

Thank you very much,
Mani

_______________________________________________
taglib-devel mailing list
taglib-devel@...
https://mail.kde.org/mailman/listinfo/taglib-devel

[Michael Smith-59]

On Tue, Sep 15, 2009 at 12:47 PM, Manfred Schwind <lists@...> wrote:
> Hi,
>
> I recently tested taglib. Great piece of software! But I discovered
> something strange: I have an MP3 file whose Genre shows up e.g. as
> "Rock". But after rewriting the Tag with taglib, it becomes e.g. "17".
> Is this a known bug?

That's normal - the ID3 format allows you to do that.

It should still display in application as the correct thing - "Rock"
in this case.

Unfortunately, iTunes implements this incorrectly (I reported the bug
to apple some months ago, but haven't heard back). I'm attaching a
patch I wrote to work around this.

I can't remember if this patch went to upstream taglib bugzilla or not
- I can't find it there, so I guess not, oops.

Mike

[cd-encode-prefs.patch]

---

Index: components/devices/device/src/sbDeviceUtils.cpp
===================================================================
--- components/devices/device/src/sbDeviceUtils.cpp (revision 14757)
+++ components/devices/device/src/sbDeviceUtils.cpp (working copy)
@@ -1174,7 +1174,8 @@
 nsresult
 sbDeviceUtils::ApplyPropertyPreferencesToProfile(sbIDevice* aDevice,
                                                  nsIArray*  aPropertyArray,
-                                                 nsString   aPrefNameBase)
+                                                 nsIPrefBranch *aPrefBranch,
+                                                 nsCString   aPrefNameBase)
 {
   nsresult rv;
 
@@ -1196,23 +1197,22 @@
     rv = property->GetPropertyName(propName);
     NS_ENSURE_SUCCESS(rv, rv);
 
-    nsString prefName = aPrefNameBase;
+    nsCString prefName = aPrefNameBase;
     prefName.AppendLiteral(".");
-    prefName.Append(propName);
+    prefName.Append(NS_ConvertUTF16toUTF8(propName));
 
-    nsCOMPtr<nsIVariant> prefVariant;
-    rv = aDevice->GetPreference(prefName, getter_AddRefs(prefVariant));
+    PRInt32 prefType;
+    rv = aPrefBranch->GetPrefType(prefName.get(), &prefType);
     NS_ENSURE_SUCCESS(rv, rv);
 
-    // Only use the property if we have a real value (not a void/empty variant)
-    PRUint16 dataType;
-    rv = prefVariant->GetDataType(&dataType);
-    NS_ENSURE_SUCCESS(rv, rv);
-    if (dataType != nsIDataType::VTYPE_VOID &&
-        dataType != nsIDataType::VTYPE_EMPTY)
+    if (prefType == nsIPrefBranch::PREF_STRING)
     {
-      rv = property->SetValue(prefVariant);
+      char *prefVal;
+      rv = aPrefBranch->GetCharPref(prefName.get(), &prefVal);
       NS_ENSURE_SUCCESS(rv, rv);
+      rv = property->SetValue(sbNewVariant(prefVal));
+      NS_Free(prefVal);
+      NS_ENSURE_SUCCESS(rv, rv);
     }
   }
 
Index: components/devices/device/src/sbDeviceUtils.h
===================================================================
--- components/devices/device/src/sbDeviceUtils.h (revision 14757)
+++ components/devices/device/src/sbDeviceUtils.h (working copy)
@@ -38,6 +38,7 @@
 #include "sbBaseDevice.h"
 #include "sbIDeviceStatus.h"
 
+class nsIPrefBranch;
 class nsIFile;
 class nsIMutableArray;
 
@@ -247,7 +248,8 @@
    */
   static nsresult ApplyPropertyPreferencesToProfile(sbIDevice *aDevice,
                                                     nsIArray *aPropertyArray,
-                                                    nsString aPrefNameBase);
+                                                    nsIPrefBranch *aPrefBranch,
+                                                    nsCString aPrefNameBase);
 
   /** Get the appropriate file extension for files ended with this profile.
    */
Index: components/devices/device/src/sbBaseDevice.cpp
===================================================================
--- components/devices/device/src/sbBaseDevice.cpp (revision 14757)
+++ components/devices/device/src/sbBaseDevice.cpp (working copy)
@@ -4145,29 +4145,40 @@
     return NS_OK;
   }
 
-  rv = SelectTranscodeProfile(formatType.Type, aProfile);
+  nsCOMPtr<nsIPrefBranch> prefBranch;
+  rv = GetPrefBranch(getter_AddRefs(prefBranch));
   NS_ENSURE_SUCCESS(rv, rv);
 
+  rv = SelectTranscodeProfile(formatType.Type, prefBranch, aProfile);
+  NS_ENSURE_SUCCESS(rv, rv);
+
   return NS_OK;
 }
 
 nsresult
 sbBaseDevice::SelectTranscodeProfile(PRUint32 contentType,
+                                     nsIPrefBranch *aPrefBranch,
                                      sbITranscodeProfile **aProfile)
 {
   nsresult rv;
 
   PRBool hasProfilePref = PR_FALSE;
   // See if we have a preference for the transcoding profile.
-  nsCOMPtr<nsIVariant> profileIdVariant;
+  PRInt32 prefType;
+  nsCString prefName = NS_LITERAL_CSTRING("transcode_profile.profile_id");
+  rv = aPrefBranch->GetPrefType(prefName.get(),
+                                &prefType);
+  NS_ENSURE_SUCCESS(rv, rv);
+
   nsString prefProfileId;
-  rv = GetPreference(NS_LITERAL_STRING("transcode_profile.profile_id"),
-                     getter_AddRefs(profileIdVariant));
-  if (NS_SUCCEEDED(rv))
+  if (prefType == nsIPrefBranch::PREF_STRING)
   {
+    char* prefValue = NULL;
+    rv = aPrefBranch->GetCharPref(prefName.get(), &prefValue);
+    NS_ENSURE_SUCCESS(rv, rv);
+    prefProfileId = NS_ConvertUTF8toUTF16(prefValue);
+    NS_Free(prefValue);
     hasProfilePref = PR_TRUE;
-    rv = profileIdVariant->GetAsAString(prefProfileId);
-    NS_ENSURE_SUCCESS(rv, rv);
     TRACE(("%s: found a profile", __FUNCTION__));
   }
 
@@ -4229,7 +4240,8 @@
     rv = sbDeviceUtils::ApplyPropertyPreferencesToProfile(
             this,
             audioProperties,
-            NS_LITERAL_STRING("transcode_profile.audio_properties"));
+            aPrefBranch,
+            NS_LITERAL_CSTRING("transcode_profile.audio_properties"));
     NS_ENSURE_SUCCESS(rv, rv);
 
     nsCOMPtr<nsIArray> videoProperties;
@@ -4238,7 +4250,8 @@
     rv = sbDeviceUtils::ApplyPropertyPreferencesToProfile(
             this,
             videoProperties,
-            NS_LITERAL_STRING("transcode_profile.video_properties"));
+            aPrefBranch,
+            NS_LITERAL_CSTRING("transcode_profile.video_properties"));
     NS_ENSURE_SUCCESS(rv, rv);
 
     nsCOMPtr<nsIArray> containerProperties;
@@ -4248,7 +4261,8 @@
     rv = sbDeviceUtils::ApplyPropertyPreferencesToProfile(
             this,
             containerProperties,
-            NS_LITERAL_STRING("transcode_profile.container_properties"));
+            aPrefBranch,
+            NS_LITERAL_CSTRING("transcode_profile.container_properties"));
     NS_ENSURE_SUCCESS(rv, rv);
 
     prefProfile.forget(aProfile);
Index: components/devices/device/src/sbBaseDevice.h
===================================================================
--- components/devices/device/src/sbBaseDevice.h (revision 14757)
+++ components/devices/device/src/sbBaseDevice.h (working copy)
@@ -1093,6 +1093,8 @@
   /**
    * \brief Select a transcode profile to use when transcoding to this device.
    * \param aContentType The type of transcoding profile to look for.ng
+   * \param aPrefBranch  The preferences branch to look for transcoding
+   *                     preferences in.
    * \param aProfile     The profile found or may be null if no transcoding
    *                     is needed.
    * \note This selects the best available transcoding profile for this device
@@ -1101,6 +1103,7 @@
    * \note NS_ERROR_NOT_AVAILABLE is returned if no suitable profile is found
    */
   nsresult SelectTranscodeProfile(PRUint32 aContentType,
+                                  nsIPrefBranch *aPrefBranch,
                                   sbITranscodeProfile **aProfile);
 
   /**
Index: components/devices/cd/src/sbCDDeviceRequest.cpp
===================================================================
--- components/devices/cd/src/sbCDDeviceRequest.cpp (revision 14757)
+++ components/devices/cd/src/sbCDDeviceRequest.cpp (working copy)
@@ -650,9 +650,23 @@
   rv = musicFolderURL->SetFileName(filename);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = musicFolderURL->SetFileExtension(NS_LITERAL_CSTRING("ogg"));
+  // Find the preferred audio transcoding profile.
+  sbPrefBranch prefBranch(PREF_CDDEVICE_RIPBRANCH, &rv);
   NS_ENSURE_SUCCESS(rv, rv);
 
+  nsCOMPtr<sbITranscodeProfile> profile;
+  rv = SelectTranscodeProfile(sbIDeviceCapabilities::CONTENT_AUDIO,
+                              prefBranch.get(),
+                              getter_AddRefs(profile));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  nsCString extension;
+  rv = sbDeviceUtils::GetTranscodedFileExtension(profile, extension);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = musicFolderURL->SetFileExtension(extension);
+  NS_ENSURE_SUCCESS(rv, rv);
+
   // Prevent notification while we set the content source on the item
   sbCDAutoIgnoreItem autoUnignore(this, destination);
 
@@ -660,12 +674,6 @@
   rv = destination->SetContentSrc(musicFolderURL);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  nsCOMPtr<sbITranscodeProfile> profile;
-  rv = GetTranscodeProfile(NS_LITERAL_STRING("vorbis"),
-                           NS_LITERAL_STRING("ogg"),
-                           getter_AddRefs(profile));
-  NS_ENSURE_SUCCESS(rv, rv);
-
   nsCOMPtr<sbITranscodeJob> tcJob;
   rv = mTranscodeManager->GetTranscoderForMediaItem(destination,
                                                     profile,
Index: components/include/sbPrefBranch.h
===================================================================
--- components/include/sbPrefBranch.h (revision 14757)
+++ components/include/sbPrefBranch.h (working copy)
@@ -162,6 +162,11 @@
     return mPrefBranch->SetCharPref(aKey, aValue.get());
   }
 
+  nsIPrefBranch *get() {
+    return mPrefBranch;
+  }
+
+
 private:
   nsCOMPtr<nsIPrefBranch> mPrefBranch;
   PRThread * mCreatingThread;

---

_______________________________________________
taglib-devel mailing list
taglib-devel@...
https://mail.kde.org/mailman/listinfo/taglib-devel

[Michael Smith-59]

On Tue, Sep 15, 2009 at 12:54 PM, Michael Smith <msmith@...> wrote:

> On Tue, Sep 15, 2009 at 12:47 PM, Manfred Schwind <lists@...> wrote:
>> Hi,
>>
>> I recently tested taglib. Great piece of software! But I discovered
>> something strange: I have an MP3 file whose Genre shows up e.g. as
>> "Rock". But after rewriting the Tag with taglib, it becomes e.g. "17".
>> Is this a known bug?
>
> That's normal - the ID3 format allows you to do that.
>
> It should still display in application as the correct thing - "Rock"
> in this case.
>
> Unfortunately, iTunes implements this incorrectly (I reported the bug
> to apple some months ago, but haven't heard back). I'm attaching a
> patch I wrote to work around this.
>
> I can't remember if this patch went to upstream taglib bugzilla or not
> - I can't find it there, so I guess not, oops.

Now filed as:
  https://bugs.kde.org/show_bug.cgi?id=207504

Mike
_______________________________________________
taglib-devel mailing list
taglib-devel@...
https://mail.kde.org/mailman/listinfo/taglib-devel

[Manfred Schwind]

>> I recently tested taglib. Great piece of software! But I discovered
>> something strange: I have an MP3 file whose Genre shows up e.g. as
>> "Rock". But after rewriting the Tag with taglib, it becomes e.g.  
>> "17".
>> Is this a known bug?
>
> That's normal - the ID3 format allows you to do that.
>
> It should still display in application as the correct thing - "Rock"
> in this case.
>
> Unfortunately, iTunes implements this incorrectly (I reported the bug
> to apple some months ago, but haven't heard back). I'm attaching a
> patch I wrote to work around this.

OK, thanks!
I looked into the sources and also found a solution that works for me.
Yes, I'm looking with iTunes at the files. First it was "Rock", after  
rewriting the tag with taglib, it was "17".
After looking into the source of ID3v2::Tag::setGenre the workaround  
was pretty easy:
I just do a tag->setGenre(tag->genre()); before saving now ...
;-)

Bye,
Mani

_______________________________________________
taglib-devel mailing list
taglib-devel@...
https://mail.kde.org/mailman/listinfo/taglib-devel

Manfred Schwind wrote:

>>> I recently tested taglib. Great piece of software! But I discovered
>>> something strange: I have an MP3 file whose Genre shows up e.g. as
>>> "Rock". But after rewriting the Tag with taglib, it becomes e.g.  
>>> "17".
>>> Is this a known bug?
>> That's normal - the ID3 format allows you to do that.
>>
>> It should still display in application as the correct thing - "Rock"
>> in this case.
>>
>> Unfortunately, iTunes implements this incorrectly (I reported the bug
>> to apple some months ago, but haven't heard back). I'm attaching a
>> patch I wrote to work around this.
>
> OK, thanks!
> I looked into the sources and also found a solution that works for me.
> Yes, I'm looking with iTunes at the files. First it was "Rock", after  
> rewriting the tag with taglib, it was "17".
> After looking into the source of ID3v2::Tag::setGenre the workaround  
> was pretty easy:
> I just do a tag->setGenre(tag->genre()); before saving now ...
> ;-)

Note that there are already various workarounds for iTunes bugs in
TagLib, which can be toggled at build time. If this was indeed reported
before, it may actually have a fix in existence but just not activated
by default.

--Jeff



_______________________________________________
taglib-devel mailing list
taglib-devel@...
https://mail.kde.org/mailman/listinfo/taglib-devel

[Michael Smith-59]

On Tue, Sep 15, 2009 at 9:03 PM, Jeff Mitchell <mitchell@...> wrote:

> Manfred Schwind wrote:
>>>> I recently tested taglib. Great piece of software! But I discovered
>>>> something strange: I have an MP3 file whose Genre shows up e.g. as
>>>> "Rock". But after rewriting the Tag with taglib, it becomes e.g.
>>>> "17".
>>>> Is this a known bug?
>>> That's normal - the ID3 format allows you to do that.
>>>
>>> It should still display in application as the correct thing - "Rock"
>>> in this case.
>>>
>>> Unfortunately, iTunes implements this incorrectly (I reported the bug
>>> to apple some months ago, but haven't heard back). I'm attaching a
>>> patch I wrote to work around this.
>>
>> OK, thanks!
>> I looked into the sources and also found a solution that works for me.
>> Yes, I'm looking with iTunes at the files. First it was "Rock", after
>> rewriting the tag with taglib, it was "17".
>> After looking into the source of ID3v2::Tag::setGenre the workaround
>> was pretty easy:
>> I just do a tag->setGenre(tag->genre()); before saving now ...
>> ;-)
>
> Note that there are already various workarounds for iTunes bugs in
> TagLib, which can be toggled at build time. If this was indeed reported
> before, it may actually have a fix in existence but just not activated
> by default.

I don't think there is - my patch adds another itunes-bug-workaround,
protected by the same #define as the others - though my patch is
against a slightly older version of taglib, and it's possible it's
been independently fixed in upstream taglib since.

Mike
_______________________________________________
taglib-devel mailing list
taglib-devel@...
https://mail.kde.org/mailman/listinfo/taglib-devel

[Manfred Schwind]

>> OK, thanks!
>> I looked into the sources and also found a solution that works for  
>> me.
>> Yes, I'm looking with iTunes at the files. First it was "Rock", after
>> rewriting the tag with taglib, it was "17".
>> After looking into the source of ID3v2::Tag::setGenre the workaround
>> was pretty easy:
>> I just do a tag->setGenre(tag->genre()); before saving now ...
>> ;-)
>
> Note that there are already various workarounds for iTunes bugs in
> TagLib, which can be toggled at build time. If this was indeed  
> reported
> before, it may actually have a fix in existence but just not activated
> by default.

Yes, I saw that. NO_ITUNES_HACKS must not be defined when building to  
have the iTunes workarounds.
But the problem is that taglib only sets the Genre as text ("iTunes  
hack") when explicitly calling "setGenre". If the tag is just read by  
taglib and then saved (without changing the genre), the text is  
converted from text to number. So this bug was not fixed for every  
case. As a workaround I call setGenre just before saving the tag as  
written above. Works fine for me.

Regards,
Mani

_______________________________________________
taglib-devel mailing list
taglib-devel@...
https://mail.kde.org/mailman/listinfo/taglib-devel