After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 442792 - ignores tags with trailing spaces
ignores tags with trailing spaces
Status: RESOLVED FIXED
Product: rhythmbox
Classification: Other
Component: Importing
HEAD
Other Linux
: Normal normal
: ---
Assigned To: RhythmBox Maintainers
RhythmBox Maintainers
Depends on:
Blocks:
 
 
Reported: 2007-06-01 06:15 UTC by Jonathan Matthew
Modified: 2007-06-03 02:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (1.40 KB, patch)
2007-06-03 00:28 UTC, Jonathan Matthew
committed Details | Review

Description Jonathan Matthew 2007-06-01 06:15:36 UTC
The code that tries to detect duplicate (id3v2 vs id3v1) tags causes tags with trailing whitespace to be ignored altogether.  When importing files with such tags, I see this in the debug output:

(15:49:06) [0x805fff8] [rb_metadata_gst_load_tag] rb-metadata-gst.c:595: uri: file:///space/music/The_Orb/Bicycles_%26_Tricycles_/08_L.U.C.A..ogg tag: album 
(15:49:06) [0x805fff8] [rb_metadata_gst_load_tag] rb-metadata-gst.c:645: Got shorter duplicate tag

and the album tag for all files in that album is ignored.  These files only have one album tag.
Comment 1 Bastien Nocera 2007-06-01 10:34:01 UTC
This isn't normal, and I don't see we'd be doing that in the code. Any ideas?

        case G_TYPE_STRING: {
                /* Reject invalid utf-8 strings, shorter duplicated tags
                 * and then remove leading and trailing whitespace.
                 */
                char *str;
                const char *old_str;

                str = g_value_dup_string (newval);

                if (!g_utf8_validate (str, -1, NULL)) {
                        rb_debug ("Got invalid UTF-8 tag data");
                        g_free (str);
                        g_value_unset (newval);
                        g_free (newval);
                        return;
                }
                str = g_strstrip (str);

                /* Check whether we have a shorter duplicate tag,
                 * Doesn't work with non-normalised UTF-8 strings */
                old_str = g_value_get_string (val);
                if (old_str != NULL
                    && g_utf8_strlen (old_str, -1) > g_utf8_strlen (str, -1)) {
                        if (g_str_has_prefix (old_str, str) != FALSE) {
                                rb_debug ("Got shorter duplicate tag");
                                g_free (str);
                                g_value_unset (newval);
                                g_free (newval);
                                return;
                        }
                }
                g_value_take_string (newval, str);
                break;
Comment 2 Jonathan Matthew 2007-06-03 00:15:47 UTC
'val' and 'newval' are both copies of the string from the incoming tag list, so 'str' is the whitespace-stripped version and 'old_str' is the original.  We'd need to find the existing value from the md->priv->metadata hash table to compare the incoming value against any existing value.
Comment 3 Jonathan Matthew 2007-06-03 00:28:12 UTC
Created attachment 89265 [details] [review]
patch
Comment 4 James "Doc" Livingston 2007-06-03 01:10:56 UTC
Looks okay to me
Comment 5 Jonathan Matthew 2007-06-03 02:32:06 UTC
committed to trunk and stable.