Re: [Easytag-mailing] [PATCH] generate cddb query from song lengths

[Daniel Drake <dan reactivated net>]

justus schwartz wrote:
> > I'm sure Jerome will correct me if I'm wrong, but I don't think the GTK1 
> > version is being worked on any more. You may wish to try writing it against 
> > the GTK2 (development) version. I don't think there is any public CVS, but if 
> > its any use, I upload all the patches that I send in to here:
> > http://www.reactivated.net/patches/easytag/0.31/extras/
>
> hm it seems a lot of them are applied in the current unstable version.

Yes, see the note at the bottom of the page for which ones Jerome hasn't 
merged yet. The main one there at the moment is 19 which is a crashfix. In the 
next few days I'll be posting a patch for better handling of filenames.

> i'll attach the cddb patch for the gtk2 version of easytag. it's in not really
> better than typing some search string into the search box, but its good if you
> are lazy typing and/or wan't to check, if the length (or number) of the files
> are ok. 

If we can get the code simple/clean enough then it should be up for consideration.

> has anybody implemented (or is implementing) the parsing of cddb records of
> albums with various artists. quote from freedb.org faq:
>
>
> >   6. When submitting a sampler or compilation, you should include the
> >   track-artist in the track-name, using the syntax "artist / track-title" and
> >   set the CD-artist to "Various"
>
>
>
> it would be nice to tag mp3s from such records with easytag.

This is also a good idea, fancy writing a patch? :)



See comments below.

> --- easytag-0.31_gtk2.4_pre1/src/cddb.c	2004-07-16 00:07:21 +0200
> +++ easytag-0.31_gtk2.4_pre1-mod/src/cddb.c	2004-08-19 18:48:49 +0200
> @@ -91,10 +91,11 @@ GtkWidget *CddbAlbumListView = NULL;
>  GtkListStore *CddbAlbumListModel = NULL;
>  GtkWidget *CddbTrackListView = NULL;
>  GtkListStore *CddbTrackListModel = NULL;
>  GtkWidget *CddbApplyButton = NULL;
>  GtkWidget *CddbSearchButton = NULL;
> +GtkWidget *CddbSearchSelectedButton = NULL;
>  GtkWidget *CddbStatusBar;
>  guint      CddbStatusBarContext;
>  
>  GtkWidget *CddbStopSearchButton;
>  GtkWidget *CddbSearchStringInResultNextButton;
> @@ -122,10 +123,11 @@ void Cddb_Close_Connection (gint socket_
>  gint Cddb_Read_Line        (gint socket_id, gchar *cddb_out);
>  gint Cddb_Read_Http_Header (gint socket_id, gchar *cddb_out);
>  gint Cddb_Read_Cddb_Header (gint socket_id, gchar *cddb_out);
>  
>  gboolean Cddb_Search_Album_List_From_String (void);
> +gboolean Cddb_Search_Album_From_Selected_Files (void);
>  gboolean Cddb_Get_Album_Tracks_List_CB(GtkTreeSelection *selection, gpointer data);
>  gboolean Cddb_Get_Album_Tracks_List(GtkTreeSelection *selection);
>  
>  void     Cddb_Load_Album_List              (gboolean only_red_lines);
>  void     Cddb_Load_Track_Album_List        (GList *track_list);
> @@ -254,10 +256,17 @@ void Open_Cddb_Window (void)
>      GTK_WIDGET_SET_FLAGS(CddbSearchButton,GTK_CAN_DEFAULT);
>      gtk_widget_grab_default(CddbSearchButton);
>      g_signal_connect(G_OBJECT(CddbSearchButton),"clicked", G_CALLBACK(Cddb_Search_Album_List_From_String),NULL);
>      g_signal_connect(G_OBJECT(GTK_ENTRY(GTK_BIN(CddbSearchStringCombo)->child)),"changed", G_CALLBACK(Cddb_Set_Search_Button_Sensivity),NULL);
>  
> +    // Button to generate/run the search directly from the file lengths
> +    CddbSearchSelectedButton = gtk_button_new_with_label("Search currently marked");

We should probably use gettext here.

> +    gtk_box_pack_start(GTK_BOX(hbox),CddbSearchSelectedButton,FALSE,FALSE,0);
> +    GTK_WIDGET_SET_FLAGS(CddbSearchSelectedButton,GTK_CAN_DEFAULT);
> +    gtk_widget_grab_default(CddbSearchSelectedButton);
> +    g_signal_connect(GTK_OBJECT(CddbSearchSelectedButton),"clicked",G_CALLBACK (Cddb_Search_Album_From_Selected_Files),NULL);
> +
>      // Button to quit
>      Button = Create_Button_With_Pixmap(BUTTON_CLOSE);
>      gtk_box_pack_end(GTK_BOX(hbox),Button,FALSE,FALSE,0);
>      GTK_WIDGET_SET_FLAGS(Button,GTK_CAN_DEFAULT);
>      g_signal_connect(G_OBJECT(Button),"clicked", G_CALLBACK(Cddb_Destroy_Window),NULL);
> @@ -1785,10 +1794,260 @@ gboolean Cddb_Search_Album_List_From_Str
>      Cddb_Load_Album_List(FALSE);
>  
>      return TRUE;
>  }
>  
> +gboolean Cddb_Search_Album_From_Selected_Files (void)
> +{
> +
> +  gint   socket_id;
> +  gint   bytes_written;
> +  gint   bytes_read;
> +  gulong bytes_read_total = 0;
> +
> +  gchar *cddb_in;               /* For the request to send */
> +  gchar *cddb_out;              /* Answer received */
> +  gchar *cddb_out_tmp;
> +  gchar *msg;
> +  gchar* tmp;
> +  gchar* query_string;
> +  gchar* cddb_discid;
> +
> +  guint total_frames = 150;     /* first offset is (almost) always 150 */
> +  guint disc_length = 0;
> +
> +  GtkTreeSelection *file_selection = NULL;
> +  guint file_selectedcount;
> +  guint counter;
> +  GtkTreeIter  currentIter;
> +  guint total_id;
> +  guint num_tracks;
> +
> +  gpointer iterptr;
> +
> +  GtkListStore *fileListModel;
> +  GtkTreeIter *fileIter;
> +  GList *file_iterlist = NULL;
> +
> +  // Connection to the server
> +  if ( ( socket_id=
> +        Cddb_Open_Connection(CDDB_USE_PROXY?CDDB_PROXY_NAME:"www.freedb.org",
> +                             CDDB_USE_PROXY?CDDB_PROXY_PORT:80)) <= 0 ) {
> +    return FALSE;
> +  }
> +
> +  fileListModel =
> +    GTK_LIST_STORE(gtk_tree_view_get_model(GTK_TREE_VIEW(BrowserList)));
> +  file_selection = gtk_tree_view_get_selection(GTK_TREE_VIEW(BrowserList));
> +  file_selectedcount = gtk_tree_selection_count_selected_rows(file_selection);
> +
> +  if (file_selectedcount > 0)
> +  {
> +    GList* file_selectedrows =
> +      gtk_tree_selection_get_selected_rows(file_selection, NULL);

You need to free the treepaths and the GList somewhere...

> +
> +    while (file_selectedrows)
> +    {
> +      counter++;
> +      iterptr = g_malloc0(sizeof(GtkTreeIter));
> +      gtk_tree_model_get_iter(GTK_TREE_MODEL(fileListModel),
> +                              (GtkTreeIter*) iterptr,
> +                              (GtkTreePath*) file_selectedrows->data);
> +      file_iterlist = g_list_append(file_iterlist, iterptr);
> +
> +      file_selectedrows = file_selectedrows->next;

...and freeing them in the current state won't be possible because the above 
statement sets file_selectedrows to NULL on the last iteration (losing the 
pointer altogether).

> +    }
> +
> +  } else /* No rows selected, use the whole list */
> +  {
> +    gtk_tree_model_get_iter_first(GTK_TREE_MODEL(fileListModel), &currentIter);
> +
> +    do
> +    {
> +      counter++;
> +      iterptr = g_memdup(&currentIter, sizeof(GtkTreeIter));
> +      file_iterlist = g_list_append(file_iterlist, iterptr);
> +    } while (gtk_tree_model_iter_next(
> +                 GTK_TREE_MODEL(fileListModel), &currentIter));
> +
> +    file_selectedcount =
> +      gtk_tree_model_iter_n_children(GTK_TREE_MODEL(fileListModel), NULL);
> +  }
> +
> +
> +  if (file_selectedcount == 0)
> +    return TRUE;
> +
> +
> +  // generate query string and compute discid
> +  total_id = 0;
> +  num_tracks = file_selectedcount;
> +  query_string = g_strdup ("");
> +  while (file_iterlist)
> +  {
> +    gulong secs = 0;
> +    fileIter = (GtkTreeIter*) file_iterlist->data;
> +    ET_File   *etfile   = Browser_List_Get_ETFile_From_Iter(fileIter);
> +    tmp = query_string;
> +    query_string = g_strdup_printf ("%s+%d", query_string, total_frames);
> +    g_free (tmp);
> +    secs = etfile->ETFileInfo->duration;
> +    total_frames +=  secs * 75;
> +    disc_length += secs;
> +    while (secs > 0) {
> +      total_id = total_id + (secs % 10);
> +      secs = secs / 10;
> +    }
> +
> +    file_iterlist = file_iterlist->next;
> +  }

You also need to free file_iterlist somewhere, and you'll run into the same 
pointer-loss problem with this loop..

> +
> +  cddb_discid =
> +    g_strdup_printf("%08x",(guint)(((total_id % 0xFF) << 24) |
> +                                   (disc_length << 8) | num_tracks));
> +
> +  cddb_in = g_strdup_printf("GET http://%s%s?cmd=cddb+query+%s+%d+%s";,
> +                            CDDB_SERVER_NAME,CDDB_SERVER_CGI_PATH,
> +                            cddb_discid,
> +                            num_tracks, query_string);
> +  g_free (cddb_discid);
> +  g_free (query_string);
> +
> +  tmp = cddb_in;
> +  cddb_in = g_strdup_printf ("%s+%d"
> +                             "&hello=noname+localhost+%s+%s"
> +                             "&proto=1 HTTP/1.1\r\n"
> +                             "Host: %s:%d\r\nConnection: close\r\n\r\n",
> +                             tmp, disc_length,
> +                             APPNAME,VERSION,
> +                             CDDB_SERVER_NAME,CDDB_SERVER_PORT
> +                             );
> +  g_free (tmp);
> +
> +  printf ("%s\n", cddb_in);
> +
> +  gtk_statusbar_push(GTK_STATUSBAR(CddbStatusBar),
> +                     CddbStatusBarContext,_("Sending request ..."));
> +
> +  while (gtk_events_pending()) gtk_main_iteration();
> +
> +  if ( (bytes_written=send(socket_id,cddb_in,strlen(cddb_in)+1,0)) < 0)
> +  {
> +    g_print(_("Can't send the request (%s)!\n"),g_strerror(errno));
> +    Cddb_Close_Connection(socket_id);
> +    g_free(cddb_in);
> +    return FALSE;
> +  }
> +
> +  g_free(cddb_in);
> +
> +  // Delete previous album list
> +  gtk_list_store_clear(CddbAlbumListModel);
> +  gtk_list_store_clear(CddbTrackListModel);
> +  Cddb_Free_Album_List();
> +  gtk_widget_set_sensitive(GTK_WIDGET(CddbStopSearchButton),TRUE);
> +
> +
> +  /*
> +   * Read the answer
> +   */
> +  gtk_statusbar_push(GTK_STATUSBAR(CddbStatusBar),
> +                     CddbStatusBarContext,_("Receiving data ..."));
> +
> +  while (gtk_events_pending()) gtk_main_iteration();
> +
> +  cddb_out = g_malloc0(CDDB_ANSWER_LINE_SIZE+1);
> +  if ( !cddb_out || (bytes_read=Cddb_Read_Http_Header(socket_id,cddb_out)) <= 0 )
> +  {
> +    msg = g_strdup_printf(_("The server returned a wrong answer! (%s)"),cddb_out);
> +    gtk_statusbar_push(GTK_STATUSBAR(CddbStatusBar),CddbStatusBarContext,msg);
> +    g_print("%s\n",msg);
> +    g_free(msg);
> +    return FALSE;
> +  }
> +  bytes_read_total = bytes_read;
> +
> +  while ( !CddbStopSearch && (bytes_read=Cddb_Read_Line(socket_id,cddb_out)) > 0 )
> +  {
> +    bytes_read_total += bytes_read;
> +    msg = g_strdup_printf(_("Receiving data (%s) ..."),Convert_Size_1(bytes_read_total));
> +    gtk_statusbar_push(GTK_STATUSBAR(CddbStatusBar),CddbStatusBarContext,msg);
> +    while (gtk_events_pending())
> +      gtk_main_iteration();
> +    g_free(msg);
> +
> +    cddb_out_tmp = cddb_out;
> +
> +    if ( cddb_out != NULL && strstr(cddb_out_tmp,"/") != NULL)
> +    {
> +      gchar* ptr;
> +      gchar* copy;
> +      gchar* valid;
> +
> +      CddbAlbum *cddbalbum;
> +
> +      cddbalbum = g_malloc0(sizeof(CddbAlbum));
> +
> +      // Get album category
> +      ptr = strstr (cddb_out_tmp, " ");
> +      *ptr = 0;
> +      copy = g_strdup(cddb_out_tmp);
> +
> +      if(g_utf8_validate(copy, -1, NULL)) {
> +        valid = copy;
> +      } else {
> +        valid = convert_string(copy, "iso-8859-1", "utf-8");
> +        g_free(copy);
> +      }
> +
> +      cddbalbum->category = valid;
> +      *ptr = ' ';
> +      cddb_out_tmp = ptr + 1;
> +
> +
> +      // Get album ID
> +      ptr = strstr (cddb_out_tmp, " ");
> +      *ptr = 0;
> +      cddbalbum->id = g_strdup(cddb_out_tmp);
> +      *ptr = ' ';
> +      cddb_out_tmp = ptr + 1;
> +
> +      // Get album and artist names.
> +      copy = g_strdup(cddb_out_tmp);
> +
> +      if(g_utf8_validate(copy, -1, NULL)) {
> +        valid = copy;
> +      } else {
> +        valid = convert_string(copy, "iso-8859-1", "utf-8");
> +        g_free(copy);
> +      }
> +
> +      cddbalbum->artist_album = valid;
> +
> +      CddbAlbumList = g_list_append(CddbAlbumList,cddbalbum);
> +    }
> +  }
> +
> +  // Close connection
> +  Cddb_Close_Connection(socket_id);
> +
> +  msg = g_strdup_printf(_("Found %d matching album(s)"),
> +                        g_list_length(CddbAlbumList));
> +
> +  gtk_statusbar_push(GTK_STATUSBAR(CddbStatusBar),CddbStatusBarContext,msg);
> +  g_free(msg);
> +
> +  // Initialize the button
> +  gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(CddbDisplayRedLinesButton),
> +                               FALSE);
> +
> +  // Load the albums found in the list
> +  Cddb_Load_Album_List(FALSE);
> +
> +  return TRUE;
> +}
> +

Apart from those points, and the possibility of more cddb-code-consolidation, 
I don't see much wrong after an initial glance :)

Daniel