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 732444 - Rhythmbox cannot retrieve correctly disk information from MusicBrainz
Rhythmbox cannot retrieve correctly disk information from MusicBrainz
Status: RESOLVED DUPLICATE of bug 708991
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
unspecified
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-06-29 19:33 UTC by Laurent Vivier
Modified: 2014-06-30 08:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Compute musicbrain-discid like diskid from flactag (1.65 KB, patch)
2014-06-29 23:20 UTC, Laurent Vivier
none Details | Review
Same patch for gstreamer-1.0 (1.70 KB, patch)
2014-06-30 00:15 UTC, Laurent Vivier
none Details | Review

Description Laurent Vivier 2014-06-29 19:33:59 UTC
The DiscID generated doesn't seems to be the good one.

For instance, when I press the "submit album" for my unrecognized CD Megadeth/Risk, it generates the URL:

http://musicbrainz.org/cdtoc/attach?id=COJ4Fg1Ijox_yXQFxyQFUQtC0XM-&tracks=13&toc=1+13+286016+150+20720+49650+53585+75915+95735+121272+140785+164935+185040+207587+221432+232385

It seems the discID is in this case : COJ4Fg1Ijox_yXQFxyQFUQtC0XM , but it doesn't exist in MusicBrainz database.

If I use "discid" utilities from "flactag" package, the discid found is : meKBdK4T0Cd7qHCItQhHlkME8UA- .

And this discid is the good one for MusicBrainz.

I have the same problem with some other CDs, like "Emilie Simon/Végétal", "Ayo/Joyfull" ...
Comment 1 Laurent Vivier 2014-06-29 20:25:04 UTC
The wrong discid is provided by gstreamer :

$ gst-launch-0.10 --tags cdparanoiasrc device=/dev/sr0 ! fakesink
Setting pipeline to PAUSED ...
Pipeline is PREROLLING ...
FOUND TAG      : found by element "cdparanoiasrc0".
          discid: c80ee30d
     discid full: c80ee30d 13 150 20720 49650 53585 75915 95735 121272 140785 164935 185040 207587 221432 232385 3811
musicbrainz-discid: COJ4Fg1Ijox_yXQFxyQFUQtC0XM-
musicbrainz-discid-full: 01 0D 00045D40 00000096 000050F0 0000C1F2 0000D151 0001288B 000175F7 0001D9B8 000225F1 00028447 0002D2D0 00032AE3 000360F8 00038BC1
     track count: 12
    track number: 1
        duration: 274266666666
Pipeline is PREROLLED ...
Setting pipeline to PLAYING ...
New clock: GstSystemClock
...
Comment 2 Tim-Philipp Müller 2014-06-29 22:14:54 UTC
Could you please re-check with GStreamer 1.x ?
Comment 3 Laurent Vivier 2014-06-29 22:42:36 UTC
$ gst-launch-1.0 --tags cdparanoiasrc device=/dev/sr0 ! fakesink
Setting pipeline to PAUSED ...
Pipeline is PREROLLING ...
FOUND TAG      : found by element "fakesink0".
          discid: c80ee30d
     discid full: c80ee30d 13 150 20720 49650 53585 75915 95735 121272 140785 164935 185040 207587 221432 232385 3811
musicbrainz-discid: COJ4Fg1Ijox_yXQFxyQFUQtC0XM-
musicbrainz-discid-full: 01 0D 00045D40 00000096 000050F0 0000C1F2 0000D151 0001288B 000175F7 0001D9B8 000225F1 00028447 0002D2D0 00032AE3 000360F8 00038BC1
     track count: 12
    track number: 1
        duration: 274266666666
Pipeline is PREROLLED ...
Setting pipeline to PLAYING ...
New clock: GstSystemClock
Comment 4 Laurent Vivier 2014-06-29 22:50:22 UTC
The bug seems to be in libgstcdda, function gst_cddabasesrc_calculate_musicbrainz_discid().

I've added some traces in this function to compare with some traces I've added too in discid from flactag :

gst_cddabasesrc_calculate_musicbrainz_discid (GstCddaBaseSrc * src)
{
  GString *s;
  GChecksum *sha;
  guchar digest[20];
  gchar *ptr;
  gchar tmp[9];
  gulong i;
  guint leadout_sector;
  gsize digest_len;

  s = g_string_new (NULL);

  leadout_sector = src->tracks[src->num_tracks - 1].end + 1 + CD_MSF_OFFSET;

  /* generate SHA digest */
  sha = g_checksum_new (G_CHECKSUM_SHA1);
  g_snprintf (tmp, sizeof (tmp), "%02X", src->tracks[0].num);
printf("1: %s\n", tmp);
  g_string_append_printf (s, "%02X", src->tracks[0].num);
  g_checksum_update (sha, (guchar *) tmp, 2);

  g_snprintf (tmp, sizeof (tmp), "%02X", src->tracks[src->num_tracks - 1].num);
printf("2: %s\n", tmp);
  g_string_append_printf (s, " %02X", src->tracks[src->num_tracks - 1].num);
  g_checksum_update (sha, (guchar *) tmp, 2);

  g_snprintf (tmp, sizeof (tmp), "%08X", leadout_sector);
printf("3: %s\n", tmp);
  g_string_append_printf (s, " %08X", leadout_sector);
  g_checksum_update (sha, (guchar *) tmp, 8);

  for (i = 0; i < 99; i++) {
    if (i < src->num_tracks) {
      guint frame_offset = src->tracks[i].start + CD_MSF_OFFSET;

      g_snprintf (tmp, sizeof (tmp), "%08X", frame_offset);
printf("%ld: %s\n", 4 + i, tmp);
      g_string_append_printf (s, " %08X", frame_offset);
      g_checksum_update (sha, (guchar *) tmp, 8);
    } else {
      g_checksum_update (sha, (guchar *) "00000000", 8);
printf("%ld: %s\n", 4 + i, tmp);
    }
  }
  digest_len = 20;
  g_checksum_get_digest (sha, (guint8 *) & digest, &digest_len);

....


gives us :

1: 01
2: 0D
3: 00045D40
4: 00000096
5: 000050F0
6: 0000C1F2
7: 0000D151
8: 0001288B
9: 000175F7
10: 0001D9B8
11: 000225F1
12: 00028447
13: 0002D2D0
14: 00032AE3
15: 000360F8
16: 00038BC1
17: 00000000
... 00000000
102: 00000000

And for discid :

static void create_disc_id(mb_disc_private *d, char buf[]) {
        SHA_INFO        sha;
        unsigned char   digest[20], *base64;
        unsigned long   size;
        char            tmp[17]; /* for 8 hex digits (16 to avoid trouble) */
        int             i;

        assert(d != NULL);
        assert(d->success);

        sha_init(&sha);

        sprintf(tmp, "%02X", d->first_track_num);
printf("1: %s\n", tmp);
        sha_update(&sha, (unsigned char *) tmp, strlen(tmp));

        sprintf(tmp, "%02X", d->last_track_num);
printf("2: %s\n", tmp);
        sha_update(&sha, (unsigned char *) tmp, strlen(tmp));

        for (i = 0; i < 100; i++) {
                sprintf(tmp, "%08X", d->track_offsets[i]);
printf("%d: %s\n", 3+i, tmp);
                sha_update(&sha, (unsigned char *) tmp, strlen(tmp));
        }

        sha_final(digest, &sha);

        base64 = rfc822_binary(digest, sizeof(digest), &size);

        memcpy(buf, base64, size);
        buf[size] = '\0';

        free(base64);
}

1: 01
2: 0C
3: 00038BC1
4: 00000096
5: 000050F0
6: 0000C1F2
7: 0000D151
8: 0001288B
9: 000175F7
10: 0001D9B8
11: 000225F1
12: 00028447
13: 0002D2D0
14: 00032AE3
15: 000360F8
16: 00000000
... 00000000
102: 00000000
DiscID        : meKBdK4T0Cd7qHCItQhHlkME8UA-
Comment 5 Laurent Vivier 2014-06-29 22:53:31 UTC
This audio CD has a data track. It should be ignored (last track should be 0x0C, not 0x0D) to compute the discid.
Comment 6 Laurent Vivier 2014-06-29 23:20:29 UTC
Created attachment 279564 [details] [review]
Compute musicbrain-discid like diskid from flactag
Comment 7 Laurent Vivier 2014-06-29 23:35:31 UTC
I've tested my patch with the following list of CDs that had a broken ID before and a good one now (against "discid" result):
Megadeth/Risk, blink-182/Enema of State, The Offspring/Conspiracy of One, David Guetta/Guetta Blaster, Air/Pocket Synphony,Aerosmith/Nine Lives, Emilie Simon/Vegetal, Moby/Hotel, Daft Punk/Human after All.

I did the change on gstreamer-0.10 : my rhythmbox is always broken, but I guess it is using gstreamer-1.0 ?
Comment 8 Laurent Vivier 2014-06-30 00:15:45 UTC
Created attachment 279565 [details] [review]
Same patch for gstreamer-1.0
Comment 9 Nicolas Dufresne (ndufresne) 2014-06-30 00:18:44 UTC
Review of attachment 279565 [details] [review]:

I'm no expert here, though one nithpick

::: gst-plugins-base1.0-1.2.4.orig/gst-libs/gst/audio/gstaudiocdsrc.c
@@ +1204,3 @@
+    if (src->priv->tracks[last_track].is_audio) {
+      break;
+    }

You can remove the scope here.
Comment 10 Nicolas Dufresne (ndufresne) 2014-06-30 00:20:12 UTC
Comment on attachment 279564 [details] [review]
Compute musicbrain-discid like diskid from flactag

0.10 is no longer maintained, no need to provide patches.
Comment 11 Sebastian Dröge (slomo) 2014-06-30 06:59:49 UTC
This was fixed quite some time ago already:
commit 019ef0747dabddb873babbfc91ee77b26e574adf
Author: Johannes Dewender <gnome@JonnyJD.net>
Date:   Sat Sep 28 13:19:02 2013 +0200

    audiocdsrc: Don't consider trailing data tracks for MusicBrainz disc id calculation
    
    MusicBrainz removes trailing data tracks from releases on the server
    and also for the calculation of the MusicBrainz Disc ID.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=708991

*** This bug has been marked as a duplicate of bug 708991 ***