Created attachment 314771 [details] [review]
use MCN to resolve multiple album matches

Added read-out of the MCN (Media catalog number) from the CD using
libdiscid. Added functions to check MCN against barcode provided by
musicbrainz and filtering the list of album matches if a barcode
matching the MCN is found.

Review of attachment 314771 [details] [review]:

Thanks for working on this it will be really good to minimize the number of times we have to show the multiple album dialog. I tried it on https://musicbrainz.org/release-group/32688eb1-f196-3f68-b26b-7e478d5add83 and it selects the right disc.

The patch looks good but I've a few comments:

Formatting wise the indent size should be 2 spaces, there should be a space before '(' when it's preceded by a keyword of function name but not otherwise. There should be spaces around all operators.

I think once we find a match we should just return that disc as there shouldn't be any other matches. There's no point getting more details from MusicBrainz if we're only going to free them in remove_album_if_mcn_barcode_mismatch()

Thanks again for you contributions.

::: libjuicer/sj-metadata-musicbrainz5.c
@@ +934,3 @@
+  int len = strlen(barcode);
+  if(len == 12) /* UPC barcode - skip leading '0' */
+      return strncmp ((mcn+1), barcode, SJ_DISCID_MCN_STRING_LENGTH-1) == 0;

As SJ_DISCID_MCN_STRING_LENGTH is defined it might be better to use that rather than hard coded lengths in these conditionals.

I think to be on the safe side we should check mcn[0] == '0' if the barcode is shorter than SJ_DISCID_MCN_STRING_LENGTH

I'd use strcmp for the comparison.

The brackets around mcn + 1 are unnecessary.

@@ +1088,3 @@
             album->metadata_source = SOURCE_MUSICBRAINZ;
             albums = g_list_append (albums, album);
+            found_mcn = found_mcn || album_barcode_matches_discid_mcn (album->barcode, disc);

I think maybe we should just bail out of the loops once we find a mcn that matches. Getting the results from MusicBrainz is fairly slow and if we can avoid unnecessary queries it will speed things up a bit. This assumes that there will only be one match on MusicBrainz for our mcn but I think barcodes are meant to be unique so it shouldn't be a problem (shout if you think I'm wrong on this!).

You'll need to clear albums and then set it with

g_list_free_full (albums, (GDestroyNotify) album_details_free);
albums = g_list_prepend (NULL, album);

and make sure all the libmusicbrainz resources get freed properly in the outer loops.

@@ +1101,3 @@
+
+  if (found_mcn == TRUE)
+      remove_album_if_mcn_barcode_mismatch(&albums, disc);

If we bail out of the loops we don't need this or remove_album_if_mcn_barcode_mismatch()

Hi Phillip, sorry for the late reply. I'm a little busy lately.
Thanks for the comments and for applying the other patch.

This patch considered multiple releases with same barcode on purpose. I never encountered it, but I just wasn't sure if it really never happens.
But I guess if the barcode/mcn is not a unique identifier, what is?
So yes, I'll modify the patch to bail out of the loops as soon as we find a matching barcode.

Created attachment 320493 [details] [review]
I've modified the patch slightly to bail out early if there is a

match. I'll merge it in time for the next (3.20) release

use MCN to resolve multiple album matches

Added read-out of the MCN (Media catalog number) from the CD using
libdiscid. Added functions to check MCN against barcode provided by
musicbrainz and filtering the list of album matches if a barcode
matching the MCN is found.

Created attachment 321234 [details] [review]
Use MCN to resolve multiple album matches

Updated commit message.

Added read-out of the MCN (Media catalog number) from the CD using
libdiscid. If the MCN matches the barcode from MusicBrainz then return
only the matching release which avoids showing the multiple album
dialog.

g# Bug 757247 - Use MCN and barcode to automatically resolve multiple album matches - NEW

Attachment 321234 [details] pushed as 8a17c08 - Use MCN to resolve multiple album matches

Thanks for committing the patch. Fixing the patch was still on my todo list, but I didn't get around to it... Thanks for fixing it!

(In reply to Andreas H from comment #7)
> Thanks for committing the patch. Fixing the patch was still on my todo list,
> but I didn't get around to it... Thanks for fixing it!

Thanks I hope you don't mind but it didn't need much work and I wanted to get it into the next release. Thanks for the MCN patches they're a really useful addition to sound-juicer.