local cddb...

[ronchon65]

You will find me painful :) I am really sorry, I didn't see that before... Now Aqualung uses the right discid when looking for a CD in the remote data base. And also uses it to write an entry in the local data base. But when searching back in the local data base, it still uses the old (bogus) discid, and don't find the entry it previously wrote (modified in the meantime), and accesses again the remote data base. However it correctly accesses a local entry copied from the right one and updated with the bogus discid... Is it possible to also use the right discid (calculated in cdda.c ?) for accessing the local data base ? Thanks in advance and again all my apologies !

[jeremyevans]

I don't believe Aqualung sets the discid when querying, it only sets the frame offset for each track. The changes to remove Aqualung's CDDB submission support should not have modified the CDDB query support.

That being said, it may be possible to copy the discid and store it in the metadata of the store. However, I'm not sure of the best way to do that, and it's not an area of the codebase I'm familiar with. I'll see if I can review the code later, but if you've already reviewed the related code, it may be helpful to point out what parts are related.

[ronchon65]

Le 2020-07-18 23:35, Jeremy Evans a écrit :

I don't believe Aqualung sets the discid when querying, it only sets the frame offset for each track. The changes to remove Aqualung's CDDB submission support should not have modified the CDDB query support.

That being said, it may be possible to copy the discid and store it in the metadata of the store. However, I'm not sure of the best way to do that, and it's not an area of the codebase I'm familiar with. I'll see if I can review the code later, but if you've already reviewed the related code, it may be helpful to point out what parts are related.

-- You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub [1], or unsubscribe [2].

I understand, it is not really the core of Aqualung.

It is not clear to me how to do that, since the access to the local data base is included in the global libcddb query.

An idea could be to rebuild some query, using lower level libcddb routines but avoiding the one calculating the discid. I shall try to have a look to how things are done in libcddb.

Thanks for your help !

-- GD.

Links:

[1] https://github.com/jeremyevans/aqualung/issues/14#issuecomment-660545794 [2] https://github.com/notifications/unsubscribe-auth/AKLVM76RMOB324JWWR5WU5DR4IIR3ANCNFSM4PAJV4IA

[jeremyevans]

I worked on a patch for this that confirms that even if Aqualung sets the correct discid using cddb_disc_set_discid, and there is a matching entry in the local cache (~/.cddbslave/$genre/$discid), the local lookup still fails. I'm not sure how you were able to get the local lookups working.

diff --git a/src/cdda.c b/src/cdda.c
index 85ca76b..dd40ced 100644
--- a/src/cdda.c
+++ b/src/cdda.c
@@ -273,12 +273,14 @@ calc_cdda_hash(cdda_disc_t * disc) {
            result += tmp % 10;
            tmp /= 10;
        } while (tmp != 0);
+printf("track: %i offset: %i, result: %i\n", i, disc->toc[i] + 150, result);
    }

         discid = (result % 0xff) << 24 | 
-       (disc->toc[disc->n_tracks] / 75) << 8 | 
+       disc->discid_length << 8 |
        disc->n_tracks;

+printf("discid: %p\n", (void *)discid);
    return discid;
 }

@@ -317,6 +319,9 @@ cdda_scan_drive(char * device_path, cdda_drive_t * cdda_drive) {
    }

    cdda_drive->disc.n_tracks = 0;
+   cdda_drive->disc.discid_length =
+       (cdio_get_track_lba(cdda_drive->cdio, CDIO_CDROM_LEADOUT_TRACK) / CDIO_CD_FRAMES_PER_SEC) -
+       (cdio_get_track_lba(cdda_drive->cdio, cdio_get_first_track_num(cdda_drive->cdio)) / CDIO_CD_FRAMES_PER_SEC);
    tracks = cdio_get_num_tracks(cdda_drive->cdio);

    if (tracks < 1 || tracks >= 100) {
diff --git a/src/cdda.h b/src/cdda.h
index 5a56155..704bdf1 100644
--- a/src/cdda.h
+++ b/src/cdda.h
@@ -38,6 +38,7 @@

 typedef struct {
    int n_tracks;
+   unsigned int discid_length;
    lsn_t toc[100];
    unsigned long hash;
    unsigned long hash_prev;
diff --git a/src/cddb_lookup.c b/src/cddb_lookup.c
index 29f918b..5b6e292 100644
--- a/src/cddb_lookup.c
+++ b/src/cddb_lookup.c
@@ -76,6 +76,7 @@ typedef struct {
    int ntracks;
    int * frames;
    int record_length;
+        int discid;

    cddb_disc_t ** records;
    int nrecords;
@@ -251,7 +252,13 @@ cddb_lookup(cddb_lookup_t * data) {
        cddb_disc_add_track(disc, track);
    }

+printf("cddb discid: %p\n", data->discid);
+        if (data->discid) {
+                cddb_disc_set_discid(disc, data->discid);
+        }
+
    data->nrecords = cddb_query(conn, disc);
+printf("query finished\n");

    if (data->nrecords <= 0) {
        cddb_destroy(conn);
@@ -1136,7 +1143,7 @@ cddb_lookup_thread(void * arg) {
 void
 cddb_start_lookup_thread(GtkTreeIter * iter_record,
             int progress,
-            int ntracks, int * frames, int length,
+            int ntracks, int * frames, int length, int discid,
             gboolean (*timeout_cb)(gpointer)) {

    cddb_lookup_t * data;
@@ -1150,6 +1157,7 @@ cddb_start_lookup_thread(GtkTreeIter * iter_record,
    data->ntracks = ntracks;
    data->frames = frames;
    data->record_length = length;
+   data->discid = discid;

    AQUALUNG_THREAD_CREATE(data->thread_id, NULL, cddb_lookup_thread, data);
    if (progress) {
@@ -1159,21 +1167,21 @@ cddb_start_lookup_thread(GtkTreeIter * iter_record,
 }

 void
-cddb_start_query(GtkTreeIter * iter_record, int ntracks, int * frames, int length) {
+cddb_start_query(GtkTreeIter * iter_record, int ntracks, int * frames, int length, int discid) {

    cddb_start_lookup_thread(iter_record,
                 1,
-                ntracks, frames, length,
+                ntracks, frames, length, discid,
                 query_timeout_cb);
 }

 #ifdef HAVE_CDDA
 void
-cddb_auto_query_cdda(GtkTreeIter * drive_iter, int ntracks, int * frames, int length) {
+cddb_auto_query_cdda(GtkTreeIter * drive_iter, int ntracks, int * frames, int length, int discid) {

    cddb_start_lookup_thread(drive_iter,
                 0,
-                ntracks, frames, length,
+                ntracks, frames, length, discid,
                 cdda_auto_query_timeout_cb);
 }
 #endif /* HAVE_CDDA */
diff --git a/src/cddb_lookup.h b/src/cddb_lookup.h
index 7667c02..dd0a858 100644
--- a/src/cddb_lookup.h
+++ b/src/cddb_lookup.h
@@ -24,9 +24,9 @@
 #include <gtk/gtk.h>

-void cddb_start_query(GtkTreeIter * record_iter, int ntracks, int * frames, int length);
+void cddb_start_query(GtkTreeIter * record_iter, int ntracks, int * frames, int length, int discid);
 void cddb_start_submit(GtkTreeIter * iter_record, int ntracks, int * frames, int length);
-void cddb_auto_query_cdda(GtkTreeIter * drive_iter, int ntracks, int * frames, int length);
+void cddb_auto_query_cdda(GtkTreeIter * drive_iter, int ntracks, int * frames, int length, int discid);

 void cddb_query_batch(int ntracks, int * frames, int length,
              char * artist, char * record, int * year, char ** tracks);
diff --git a/src/store_cdda.c b/src/store_cdda.c
index 847996b..b125817 100644
--- a/src/store_cdda.c
+++ b/src/store_cdda.c
@@ -532,7 +532,7 @@ cdda_record__rip_cb(gpointer data) {
 #ifdef HAVE_CDDB

 static int
-cddb_init_query_data(GtkTreeIter * iter_record, int * ntracks, int ** frames, int * length) {
+cddb_init_query_data(GtkTreeIter * iter_record, int * ntracks, int ** frames, int * length, int * discid) {

    int i;
    float len = 0.0f;
@@ -540,6 +540,7 @@ cddb_init_query_data(GtkTreeIter * iter_record, int * ntracks, int ** frames, in

    GtkTreeIter iter_track;
    cdda_track_t * data;
+   cdda_drive_t * drive;

    *ntracks = gtk_tree_model_iter_n_children(GTK_TREE_MODEL(music_store), iter_record);

@@ -564,6 +565,9 @@ cddb_init_query_data(GtkTreeIter * iter_record, int * ntracks, int ** frames, in

    *length = (int)len;

+        gtk_tree_model_get(GTK_TREE_MODEL(music_store), iter_record, MS_COL_DATA, &drive, -1);
+        *discid = drive->disc.hash;
+
    return 0;
 }

@@ -577,9 +581,10 @@ cdda_record__cddb_cb(gpointer data) {
        int ntracks;
        int * frames;
        int length;
+                int discid;

-       if (cddb_init_query_data(&iter, &ntracks, &frames, &length) == 0) {
-           cddb_start_query(&iter, ntracks, frames, length);
+       if (cddb_init_query_data(&iter, &ntracks, &frames, &length, &discid) == 0) {
+           cddb_start_query(&iter, ntracks, frames, length, discid);
        }
    }
 }
@@ -590,9 +595,10 @@ cdda_record_auto_query_cddb(GtkTreeIter * drive_iter) {
    int ntracks;
    int * frames;
    int length;
+        int discid;

-   if (cddb_init_query_data(drive_iter, &ntracks, &frames, &length) == 0) {
-       cddb_auto_query_cdda(drive_iter, ntracks, frames, length);
+   if (cddb_init_query_data(drive_iter, &ntracks, &frames, &length, &discid) == 0) {
+       cddb_auto_query_cdda(drive_iter, ntracks, frames, length, discid);
    }
 }

diff --git a/src/store_file.c b/src/store_file.c
index e499aed..4171166 100644
--- a/src/store_file.c
+++ b/src/store_file.c
@@ -2543,7 +2543,7 @@ record__cddb_cb(gpointer data) {
        int length;

        if (cddb_init_query_data(&iter, &ntracks, &frames, &length) == 0) {
-           cddb_start_query(&iter, ntracks, frames, length);
+           cddb_start_query(&iter, ntracks, frames, length, 0);
        }
    }
 }

@tomszilagyi what are your thoughts on removing local cddb support? It shouldn't change any behavior, just make lookups slightly slower. From my testing, Aqualung appears to take about 120 seconds when it starts with a CD in the drive to when it makes the CDDB query, so the local vs remote CDDB query time is basically inconsequential.

[tomscii]

I can't really test this now, but sure, feel free to go ahead with removing local cddb support.

Tom

On Sat, 25 Jul 2020, 17:09 Jeremy Evans, notifications@github.com wrote:

I worked on a patch for this that confirms that even if Aqualung sets the correct discid using cddb_disc_set_discid, and there is a matching entry in the local cache (~/.cddbslave/$genre/$discid), the local lookup still fails. I'm not sure how you were able to get the local lookups working.

diff --git a/src/cdda.c b/src/cdda.c index 85ca76b..dd40ced 100644--- a/src/cdda.c+++ b/src/cdda.c@@ -273,12 +273,14 @@ calc_cdda_hash(cdda_disc_t * disc) { result += tmp % 10; tmp /= 10; } while (tmp != 0);+printf("track: %i offset: %i, result: %i\n", i, disc->toc[i] + 150, result); }

     discid = (result % 0xff) << 24 | -       (disc->toc[disc->n_tracks] / 75) << 8 | +       disc->discid_length << 8 |
  disc->n_tracks;

+printf("discid: %p\n", (void )discid); return discid; } @@ -317,6 +319,9 @@ cdda_scan_drive(char device_path, cdda_drive_t * cdda_drive) { }

cdda_drive->disc.n_tracks = 0;+ cdda_drive->disc.discid_length =+ (cdio_get_track_lba(cdda_drive->cdio, CDIO_CDROM_LEADOUT_TRACK) / CDIO_CD_FRAMES_PER_SEC) -+ (cdio_get_track_lba(cdda_drive->cdio, cdio_get_first_track_num(cdda_drive->cdio)) / CDIO_CD_FRAMES_PER_SEC); tracks = cdio_get_num_tracks(cdda_drive->cdio);

if (tracks < 1 || tracks >= 100) {diff --git a/src/cdda.h b/src/cdda.h index 5a56155..704bdf1 100644--- a/src/cdda.h+++ b/src/cdda.h@@ -38,6 +38,7 @@

typedef struct { int n_tracks;+ unsigned int discid_length; lsn_t toc[100]; unsigned long hash; unsigned long hash_prev;diff --git a/src/cddb_lookup.c b/src/cddb_lookup.c index 29f918b..5b6e292 100644--- a/src/cddb_lookup.c+++ b/src/cddb_lookup.c@@ -76,6 +76,7 @@ typedef struct { int ntracks; int * frames; int record_length;+ int discid;

cddb_disc_t * records; int nrecords;@@ -251,7 +252,13 @@ cddb_lookup(cddb_lookup_t data) { cddb_disc_add_track(disc, track); } +printf("cddb discid: %p\n", data->discid);+ if (data->discid) {+ cddb_disc_set_discid(disc, data->discid);+ }+ data->nrecords = cddb_query(conn, disc);+printf("query finished\n");

if (data->nrecords <= 0) { cddb_destroy(conn);@@ -1136,7 +1143,7 @@ cddb_lookup_thread(void arg) { void cddb_start_lookup_thread(GtkTreeIter iter_record, int progress,- int ntracks, int frames, int length,+ int ntracks, int frames, int length, int discid, gboolean (*timeout_cb)(gpointer)) {

cddb_lookup_t data;@@ -1150,6 +1157,7 @@ cddb_start_lookup_thread(GtkTreeIter iter_record, data->ntracks = ntracks; data->frames = frames; data->record_length = length;+ data->discid = discid;

AQUALUNG_THREAD_CREATE(data->thread_id, NULL, cddb_lookup_thread, data); if (progress) {@@ -1159,21 +1167,21 @@ cddb_start_lookup_thread(GtkTreeIter * iter_record, }

void-cddb_start_query(GtkTreeIter iter_record, int ntracks, int frames, int length) {+cddb_start_query(GtkTreeIter iter_record, int ntracks, int frames, int length, int discid) {

cddb_start_lookup_thread(iter_record, 1,- ntracks, frames, length,+ ntracks, frames, length, discid, query_timeout_cb); }

ifdef HAVE_CDDA

void-cddb_auto_query_cdda(GtkTreeIter drive_iter, int ntracks, int frames, int length) {+cddb_auto_query_cdda(GtkTreeIter drive_iter, int ntracks, int frames, int length, int discid) {

cddb_start_lookup_thread(drive_iter, 0,- ntracks, frames, length,+ ntracks, frames, length, discid, cdda_auto_query_timeout_cb); }

endif / HAVE_CDDA /diff --git a/src/cddb_lookup.h b/src/cddb_lookup.h

index 7667c02..dd0a858 100644--- a/src/cddb_lookup.h+++ b/src/cddb_lookup.h@@ -24,9 +24,9 @@

include <gtk/gtk.h>

-void cddb_start_query(GtkTreeIter record_iter, int ntracks, int frames, int length);+void cddb_start_query(GtkTreeIter record_iter, int ntracks, int frames, int length, int discid); void cddb_start_submit(GtkTreeIter iter_record, int ntracks, int frames, int length);-void cddb_auto_query_cdda(GtkTreeIter drive_iter, int ntracks, int frames, int length);+void cddb_auto_query_cdda(GtkTreeIter drive_iter, int ntracks, int frames, int length, int discid);

void cddb_query_batch(int ntracks, int frames, int length, char artist, char record, int year, char ** tracks);diff --git a/src/store_cdda.c b/src/store_cdda.c index 847996b..b125817 100644--- a/src/store_cdda.c+++ b/src/store_cdda.c@@ -532,7 +532,7 @@ cdda_record__rip_cb(gpointer data) {

ifdef HAVE_CDDB

static int-cddb_init_query_data(GtkTreeIter iter_record, int ntracks, int * frames, int length) {+cddb_init_query_data(GtkTreeIter iter_record, int ntracks, int * frames, int length, int * discid) {

int i; float len = 0.0f;@@ -540,6 +540,7 @@ cddb_init_query_data(GtkTreeIter iter_record, int ntracks, int ** frames, in

GtkTreeIter iter_track; cdda_track_t data;+ cdda_drive_t drive;

ntracks = gtk_tree_model_iter_n_children(GTK_TREE_MODEL(music_store), iter_record); @@ -564,6 +565,9 @@ cddb_init_query_data(GtkTreeIter iter_record, int * ntracks, int ** frames, in

*length = (int)len;

• gtk_tree_model_get(GTK_TREE_MODEL(music_store), iter_record, MS_COL_DATA, &drive, -1);+ discid = drive->disc.hash;+ return 0; } @@ -577,9 +581,10 @@ cdda_record__cddb_cb(gpointer data) { int ntracks; int frames; int length;+ int discid;
• if (cddb_init_query_data(&iter, &ntracks, &frames, &length) == 0) {- cddb_start_query(&iter, ntracks, frames, length);+ if (cddb_init_query_data(&iter, &ntracks, &frames, &length, &discid) == 0) {+ cddb_start_query(&iter, ntracks, frames, length, discid); } } }@@ -590,9 +595,10 @@ cdda_record_auto_query_cddb(GtkTreeIter drive_iter) { int ntracks; int frames; int length;+ int discid;

• if (cddb_init_query_data(drive_iter, &ntracks, &frames, &length) == 0) {- cddb_auto_query_cdda(drive_iter, ntracks, frames, length);+ if (cddb_init_query_data(drive_iter, &ntracks, &frames, &length, &discid) == 0) {+ cddb_auto_query_cdda(drive_iter, ntracks, frames, length, discid); } } diff --git a/src/store_file.c b/src/store_file.c index e499aed..4171166 100644--- a/src/store_file.c+++ b/src/store_file.c@@ -2543,7 +2543,7 @@ record__cddb_cb(gpointer data) { int length;

  if (cddb_init_query_data(&iter, &ntracks, &frames, &length) == 0) {- cddb_start_query(&iter, ntracks, frames, length);+ cddb_start_query(&iter, ntracks, frames, length, 0); } } }

@tomszilagyi https://github.com/tomszilagyi what are your thoughts on removing local cddb support? It shouldn't change any behavior, just make lookups slightly slower. From my testing, Aqualung appears to take about 120 seconds when it starts with a CD in the drive to when it makes the CDDB query, so the local vs remote CDDB query time is basically inconsequential.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jeremyevans/aqualung/issues/14#issuecomment-663866028, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJTXMEYBBIL3OVKOWDCNI3R5LYTDANCNFSM4PAJV4IA .

[ronchon65]

Hello,

Thanks for your help.

I get local lookup working if I create/update an entry with the (wrong) discid calculated by libcddb. Which is not very satisfactory :)

I am currently investigating the cddb code in order to find a workaround, but I had no such time to do that.

For my part I would regret this capability, although I understand your reason to remove it. Would it possible for me to keep it (for example with a fork) to continue my investigation ?

-- GD.

Le 2020-07-25 17:09, Jeremy Evans a écrit :

I worked on a patch for this that confirms that even if Aqualung sets the correct discid using cddb_disc_set_discid, and there is a matching entry in the local cache (~/.cddbslave/$genre/$discid), the local lookup still fails. I'm not sure how you were able to get the local lookups working.

diff --git a/src/cdda.c b/src/cdda.c index 85ca76b..dd40ced 100644 --- a/src/cdda.c +++ b/src/cdda.c @@ -273,12 +273,14 @@ calc_cdda_hash(cdda_disc_t * disc) { result += tmp % 10; tmp /= 10; } while (tmp != 0); +printf("track: %i offset: %i, result: %i\n", i, disc->toc[i] + 150, result); }

discid = (result % 0xff) << 24 |

• (disc->toc[disc->n_tracks] / 75) << 8 |
• disc->discid_length << 8 | disc->n_tracks;

+printf("discid: %p\n", (void *)discid); return discid; }

@@ -317,6 +319,9 @@ cdda_scan_drive(char device_path, cdda_drive_t cdda_drive) { }

cdda_drive->disc.n_tracks = 0;

• cdda_drive->disc.discid_length =
• (cdio_get_track_lba(cdda_drive->cdio, CDIO_CDROM_LEADOUT_TRACK) / CDIO_CD_FRAMES_PER_SEC) -
• (cdio_get_track_lba(cdda_drive->cdio, cdio_get_first_track_num(cdda_drive->cdio)) / CDIO_CD_FRAMES_PER_SEC); tracks = cdio_get_num_tracks(cdda_drive->cdio);

if (tracks < 1 || tracks >= 100) { diff --git a/src/cdda.h b/src/cdda.h index 5a56155..704bdf1 100644 --- a/src/cdda.h +++ b/src/cdda.h @@ -38,6 +38,7 @@

typedef struct { int n_tracks;

• unsigned int discid_length; lsn_t toc[100]; unsigned long hash; unsigned long hash_prev; diff --git a/src/cddb_lookup.c b/src/cddb_lookup.c index 29f918b..5b6e292 100644 --- a/src/cddb_lookup.c +++ b/src/cddb_lookup.c @@ -76,6 +76,7 @@ typedef struct { int ntracks; int * frames; int record_length;
• int discid;

cddb_disc_t * records; int nrecords; @@ -251,7 +252,13 @@ cddb_lookup(cddb_lookup_t data) { cddb_disc_add_track(disc, track); }

+printf("cddb discid: %p\n", data->discid);

• if (data->discid) {
• cddb_disc_set_discid(disc, data->discid);
• }

• data->nrecords = cddb_query(conn, disc); +printf("query finished\n");

if (data->nrecords <= 0) { cddb_destroy(conn); @@ -1136,7 +1143,7 @@ cddb_lookup_thread(void arg) { void cddb_start_lookup_thread(GtkTreeIter iter_record, int progress,

• int ntracks, int * frames, int length,
• int ntracks, int frames, int length, int discid, gboolean (timeout_cb)(gpointer)) {

cddb_lookup_t data; @@ -1150,6 +1157,7 @@ cddb_start_lookup_thread(GtkTreeIter iter_record, data->ntracks = ntracks; data->frames = frames; data->record_length = length;

• data->discid = discid;

AQUALUNG_THREAD_CREATE(data->thread_id, NULL, cddb_lookup_thread, data); if (progress) { @@ -1159,21 +1167,21 @@ cddb_start_lookup_thread(GtkTreeIter * iter_record, }

void -cddb_start_query(GtkTreeIter iter_record, int ntracks, int frames, int length) { +cddb_start_query(GtkTreeIter iter_record, int ntracks, int frames, int length, int discid) {

cddb_start_lookup_thread(iter_record, 1,

• ntracks, frames, length,
• ntracks, frames, length, discid, query_timeout_cb); }

ifdef HAVE_CDDA

void -cddb_auto_query_cdda(GtkTreeIter drive_iter, int ntracks, int frames, int length) { +cddb_auto_query_cdda(GtkTreeIter drive_iter, int ntracks, int frames, int length, int discid) {

cddb_start_lookup_thread(drive_iter, 0,

• ntracks, frames, length,
• ntracks, frames, length, discid, cdda_auto_query_timeout_cb); }

  endif / HAVE_CDDA /

  diff --git a/src/cddb_lookup.h b/src/cddb_lookup.h index 7667c02..dd0a858 100644 --- a/src/cddb_lookup.h +++ b/src/cddb_lookup.h @@ -24,9 +24,9 @@

  include <gtk/gtk.h>

-void cddb_start_query(GtkTreeIter record_iter, int ntracks, int frames, int length); +void cddb_start_query(GtkTreeIter record_iter, int ntracks, int frames, int length, int discid); void cddb_start_submit(GtkTreeIter iter_record, int ntracks, int frames, int length); -void cddb_auto_query_cdda(GtkTreeIter drive_iter, int ntracks, int frames, int length); +void cddb_auto_query_cdda(GtkTreeIter drive_iter, int ntracks, int frames, int length, int discid);

void cddb_query_batch(int ntracks, int frames, int length, char artist, char record, int year, char ** tracks); diff --git a/src/store_cdda.c b/src/store_cdda.c index 847996b..b125817 100644 --- a/src/store_cdda.c +++ b/src/store_cdda.c @@ -532,7 +532,7 @@ cdda_record__rip_cb(gpointer data) {

ifdef HAVE_CDDB

static int -cddb_init_query_data(GtkTreeIter iter_record, int ntracks, int * frames, int length) { +cddb_init_query_data(GtkTreeIter iter_record, int ntracks, int * frames, int length, int * discid) {

int i; float len = 0.0f; @@ -540,6 +540,7 @@ cddb_init_query_data(GtkTreeIter iter_record, int ntracks, int ** frames, in

GtkTreeIter iter_track; cdda_track_t * data;

• cdda_drive_t * drive;

*ntracks = gtk_tree_model_iter_n_children(GTK_TREE_MODEL(music_store), iter_record);

@@ -564,6 +565,9 @@ cddb_init_query_data(GtkTreeIter iter_record, int ntracks, int ** frames, in

*length = (int)len;

• gtk_tree_model_get(GTK_TREE_MODEL(music_store), iter_record, MS_COL_DATA, &drive, -1);
• *discid = drive->disc.hash;

• return 0; }

@@ -577,9 +581,10 @@ cdda_record__cddb_cb(gpointer data) { int ntracks; int * frames; int length;

• int discid;

• if (cddb_init_query_data(&iter, &ntracks, &frames, &length) == 0) {

• cddb_start_query(&iter, ntracks, frames, length);

• if (cddb_init_query_data(&iter, &ntracks, &frames, &length, &discid) == 0) {

• cddb_start_query(&iter, ntracks, frames, length, discid); } } } @@ -590,9 +595,10 @@ cdda_record_auto_query_cddb(GtkTreeIter drive_iter) { int ntracks; int frames; int length;

• int discid;

• if (cddb_init_query_data(drive_iter, &ntracks, &frames, &length) == 0) {

• cddb_auto_query_cdda(drive_iter, ntracks, frames, length);

• if (cddb_init_query_data(drive_iter, &ntracks, &frames, &length, &discid) == 0) {

• cddb_auto_query_cdda(drive_iter, ntracks, frames, length, discid); } }

diff --git a/src/store_file.c b/src/store_file.c index e499aed..4171166 100644 --- a/src/store_file.c +++ b/src/store_file.c @@ -2543,7 +2543,7 @@ record__cddb_cb(gpointer data) { int length;

if (cddb_init_query_data(&iter, &ntracks, &frames, &length) == 0) {

• cddb_start_query(&iter, ntracks, frames, length);
• cddb_start_query(&iter, ntracks, frames, length, 0); } } }

@tomszilagyi [1] what are your thoughts on removing local cddb support? It shouldn't change any behavior, just make lookups slightly slower. From my testing, Aqualung appears to take about 120 seconds when it starts with a CD in the drive to when it makes the CDDB query, so the local vs remote CDDB query time is basically inconsequential.

-- You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub [2], or unsubscribe [3].

Links:

[1] https://github.com/tomszilagyi [2] https://github.com/jeremyevans/aqualung/issues/14#issuecomment-663866028 [3] https://github.com/notifications/unsubscribe-auth/AKLVM75QIOLG44DOXMPH3KLR5LYTDANCNFSM4PAJV4IA

[ronchon65]

Le 2020-07-25 17:09, Jeremy Evans a écrit :

I worked on a patch for this that confirms that even if Aqualung sets the correct discid using cddb_disc_set_discid, and there is a matching entry in the local cache (~/.cddbslave/$genre/$discid), the local lookup still fails. I'm not sure how you were able to get the local lookups working.

You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub [1], or unsubscribe [2].

Hello,

I tried your patch. Probably I did something wrong because it compiles well, but I get a segmentation fault at run time.

Anyway I think the problem is the following statement in the code of cddb_query (in libcddb) shortly after the beginning :

---

/ recalculate disc ID to make sure it matches the disc data / cddb_disc_calc_discid(disc);

---

I suspect it is the reason why the discid previously set is erased. My idea was to include in the aqualung code an alternative version of the cddb_query routine, removing from it this suspicious statement. My skills on project building are not sufficient, and until now I was not able to obtain an executable.

-- GD.

Links:

[1] https://github.com/jeremyevans/aqualung/issues/14#issuecomment-663866028 [2] https://github.com/notifications/unsubscribe-auth/AKLVM75QIOLG44DOXMPH3KLR5LYTDANCNFSM4PAJV4IA

[ronchon65]

Le 2020-07-26 21:09, Guy Durrieu a écrit :

Le 2020-07-25 17:09, Jeremy Evans a écrit :

I worked on a patch for this that confirms that even if Aqualung sets the correct discid using cddb_disc_set_discid, and there is a matching entry in the local cache (~/.cddbslave/$genre/$discid), the local lookup still fails. I'm not sure how you were able to get the local lookups working.

You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub [1], or unsubscribe [2].

Hello,

I tried your patch. Probably I did something wrong because it compiles well, but I get a segmentation fault at run time.

Anyway I think the problem is the following statement in the code of cddb_query (in libcddb) shortly after the beginning :

---

/ recalculate disc ID to make sure it matches the disc data / cddb_disc_calc_discid(disc);

---

I suspect it is the reason why the discid previously set is erased. My idea was to include in the aqualung code an alternative version of the cddb_query routine, removing from it this suspicious statement. My skills on project building are not sufficient, and until now I was not able to obtain an executable.

-- GD.

Hello,

I confirm that using aqualung patched as indicated and removing the above statement in cddb_query makes everything work.

If it is possible to introduce the patched cddb_query function in the aqualung code, that would solve the issue...

Thanks a lot for your help !

-- GD.

Links:

[1] https://github.com/jeremyevans/aqualung/issues/14#issuecomment-663866028 [2] https://github.com/notifications/unsubscribe-auth/AKLVM75QIOLG44DOXMPH3KLR5LYTDANCNFSM4PAJV4IA

[jeremyevans]

Unfortunately, I don't think it's practical or reasonable to replace a function in a shared library (libcddb). I'm not sure why libcddb calculates the discid if a discid has already been set manually, but considering the libcddb appears to have stopped development, it seems unlikely it will be fixed. Considering the local cddb cache doesn't add much value, I'll work on a patch to remove it this week.

[ronchon65]

I understand your arguments, although I was not at all proposing to replace a function in a shared library.

However one reason for which I use Aqualung is precisely its aptitude to deal with local cddb, and as far as I know it is the only linux audio tool to do that (maybe except abcde, but it is not the same level of user interface). Audacious don't do that. Clementine don't do that.

I personally have CDs which are not repertoried in the few remaining free remote data bases, and for which I used to create local entries. Of course it is not essential, but...

Anyway, thanks for your help.

Le 2020-07-27 16:57, Jeremy Evans a écrit :

Unfortunately, I don't think it's practical or reasonable to replace a function in a shared library (libcddb). I'm not sure why libcddb calculates the discid if a discid has already been set manually, but considering the libcddb appears to have stopped development, it seems unlikely it will be fixed. Considering the local cddb cache doesn't add much value, I'll work on a patch to remove it this week.

-- You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub [1], or unsubscribe [2].

Links:

[1] https://github.com/jeremyevans/aqualung/issues/14#issuecomment-664447519 [2] https://github.com/notifications/unsubscribe-auth/AKLVM72QUV3TCEQVELCFB5DR5WIWFANCNFSM4PAJV4IA

[jeremyevans]

@ronchon65 I agree it would be best if the feature worked. However, it is better to remove a broken feature than keep it. Unless someone can provide a reasonable patch to Aqualung to fix the feature, it should be removed.