Skip to content

Bug in calculate_covariance? #29

@sgrabli

Description

@sgrabli

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!

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions