Simplify CalculateFrameShift()

[mgeier]

The explanation/proof for why that formula is correct is in the referenced paper: https://lac2026.sciencesconf.org/722511

The paper also provides a comparison to the previous implementation, which is described here: https://hal.science/hal-02158912v1

The code was not wrong before, it's now just simpler (and needs fewer computations, but that's probably not really relevant in this case).

[RossBencina]

Thank you. We're going to wait until after the 19.8 release to merge this. If possible could you please update the code comment to give a full bibliographic reference for the citation. Ideally with a succinct (few-line) explanation and/or proof of what properties the shift value needs to have and why this calculation satisfies the properties.

[philburk]

Thanks for the contribution and the links to the papers. I like simpler code.

I wrote a little C program to verify that the new code gives the same result as the old code for M,N between 1 to 10000.
It passed.

//
// Compare the old and new implementations of CalculateFrameShift()
//

#include <stdlib.h>
#include <stdio.h>

#define PA_MIN_( a, b ) ( ((a)<(b)) ? (a) : (b) )

/* greatest common divisor */
static unsigned long GCD( unsigned long a, unsigned long b )
{
return (b==0) ? a : GCD( b, a%b);
}

/* least common multiple - PPCM in French /
static unsigned long LCM( unsigned long a, unsigned long b )
{
return (ab) / GCD(a,b);
}

#define PA_MAX_( a, b ) (((a) > (b)) ? (a) : (b))

static unsigned long CalculateFrameShiftOld( unsigned long M, unsigned long N )
{
unsigned long result = 0;
unsigned long i;
unsigned long lcm;

lcm = LCM( M, N );
for( i = M; i < lcm; i += M )
    result = PA_MAX_( result, i % N );

return result;

}

static unsigned long CalculateFrameShiftNew( unsigned long M, unsigned long N )
{
/* https://lac2026.sciencesconf.org/722511 */
return N - GCD( M, N );
}

int main(int argc, const char * argv[]) {

unsigned long N = 512;
unsigned long M = 440;
unsigned long limit = 10000;
for (M=1; M < limit; M++) {
    for (N=1; N < limit; N++) {
        unsigned long old = CalculateFrameShiftOld(M,N);
        unsigned long new = CalculateFrameShiftNew(M,N);
        if (old != new) {
            printf("M = %d, N = %d, old = %d, new = %d\n", (int)M, (int)N, (int)old, (int)new);
            goto error;
        }
    }
    if ((M % 20) == 0) printf("M = %d\n", (int)M);
}
printf("DONE!\n");
return EXIT_SUCCESS;

error:
printf("MISMATCH!\n");
return EXIT_FAILURE;
}

[mgeier]

We're going to wait until after the 19.8 release to merge this.

Sure, no problem, this is not urgent. I'm looking forward to the new 19.8 release!

If possible could you please update the code comment to give a full bibliographic reference for the citation.

done: 77def5d

Ideally with a succinct (few-line) explanation and/or proof of what properties the shift value needs to have and why this calculation satisfies the properties.

I don't think it's possible to summarize it meaningfully in a few lines.

The proof in the paper has been criticized for being too long, but even the suggested alternative proof sketch is too long to put it into a comment in the source code (see https://lists.linuxaudio.org/hyperkitty/list/linux-audio-dev@lists.linuxaudio.org/thread/LAW4I4EQS6QTUCFE33PVRLQF6DPEEKOS/).