Bug in calculate_covariance?

Hi,

First of all, thanks for putting up online this source code, it's very useful to have a reference implementation of that great paper. I was curious to understand the rationale behind the 0.2 multiplicator in your fern's threshold calculation so I set out to inspect the max_diff values and noticed that some of them were greater than 255 which didn't seem to make sense. Ultimately, I tracked down the problem to what I believe is a bug in the implementation of calculate_covariance. In this function you pass the image intensities as references to const vector and build OpenCV Mats from them, sharing the data (i.e. copyData is left to false in the Mat constructor). However, at some point, you modify the values inside these Mats, e.g.: v1 = v1 - mean_1; This seems to modify the value of the v_1 vector for me (despite the const qualifier on the argument) and as a result after returning from this function, the values in the 'densities' array are changed. Here is what I think the code should be:

double calculate_covariance(const vector<double>& v_1, 
                            const vector<double>& v_2) {
    Mat_<double> v1(v_1);
    Mat_<double> v2(v_2);
    double mean_1 = mean(v1)[0];
    double mean_2 = mean(v2)[0];
    // FIXME: bug - this modifies v_1 and v_2 (supposed to be const)
    //v1 = v1 - mean_1;
    //v2 = v2 - mean_2;
    //return mean((v1).mul((v2)))[0];
    return mean((v1 - mean_1).mul((v2 - mean_2)))[0];
}

Could you please confirm that you are not intentionally modifying the values passed in that code? Once the densities values are back in [0,255], do we still need the 0.2 multiplicator in the threshold computation?

Thanks!

sai-bi / FaceAlignment

Bug in calculate_covariance? #29