bug in cddb discid calculation ?

[Christian Esken]

On Sunday 11 May 2003 23:52, Alexander Neundorf wrote:
> The reason seems to be line 67:
>
>     unsigned int l = (list[numTracks + 1] + 1) / 75;
>     l -= list[numTracks] / 75;
>
> I changed it to
>     l -= list[0] / 75;
>
> and now it works.

I have seen that it is not in CVS. You should commit it, because it is (most 
likely)  correct. I verified this with my own algorithm I wrote a year ago. I 
followed the Specs as published by the FreeDB project. You see below in my 
code that I am taking the leadout track and subtracting the first track:


unsigned long cdaccess::calcDiscID() {
    unsigned int i;
    int t = 0;
    int n = 0;

    i = 0;

    while (i < i_trackcount) {
        n = n + cddb_sum((i_tracks[i].i_min * 60) + i_tracks[i].i_sec);
        i++;
    }

    // !!! Here is the crucial part, that interests us today !!!
    t = ((i_leadout.i_min * 60) + i_leadout.i_sec) -
        ((i_tracks[0].i_min * 60) + i_tracks[0].i_sec);

    return ((n % 0xff) << 24 | t << 8 | i_trackcount);
}

// function cddb_sum() has been taken over 100% from FreeDB example code
int cddb_sum(int n)
{
    int     ret;

    /* For backward compatibility this algorithm must not change */

    ret = 0;

    while (n > 0) {
        ret = ret + (n % 10);
        n = n / 10;
    }

    return (ret);
}


Apart from that I see that we only differ in the location of the division by 
75. I am storing it in my list while you do it in the algoritm itself. But 
this should not matter.

> list[numTracks] is this strange "disc start" value, list[0] is the start
> frame of the first track.
> I think the code should be changed to not include this useless (?) "disc
> start" in the list and use the start frame of the first track instead.

Yes. Please commit

Chris