Lua vs ML data

Started by garry23, May 23, 2017, 07:43:14 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

garry23

@a1ex, @dmilligan

As I finalize my focus bar work, I'm noticed some 'strangeness' that points to Lua and core ML potentially being different.

I see the differences when calculating the near and far dofs and comparing these with the ML calculated ones.

The equations are the same and I'm concluding that, other than rounding errors, the ML reported dofs may use different (?) source data for certain key variables.

For example, ML calculated dofs use lens_info.x, where x could be, say, aperture, ie lens_info.aperture. Or focal length or focus distance.

Lua gives me access to the aperture data via camera.aperture.value

The question in my mind is, at any given time, are these variables created from the same source? That is Lua camera.aperture.value = ML C lens_info.aperture.

Cheers

Garry

a1ex

Any test case we can use for cross-checking the math?


    else if(!strcmp(key,"value")) lua_pushnumber(L, lens_info.aperture / 10.0);
    else if(!strcmp(key, "focus_distance")) lua_pushinteger(L, lens_info.focus_dist * 10);
    else if(!strcmp(key, "focal_length")) lua_pushinteger(L, lens_info.focal_len);

garry23

@a1ex

Thanks for the confirmation.

Looking in focus.c and the Lua pushes you provide below, tells me that they are both using the same source.

I'll carry on looking into this, which seems to manifest itself at 'large' focus distances, ie where we are approaching 'infinity'.

BTW I need to track two infinity limits at the moment. The lens distance one, reported by Canon is different to the infinity trap used in ML for the dofs.

Cheers

Garry

garry23

@a1ex

I'm mystified.

I took the exact equation from focus.c for the far dof and put in in my Lua focus bar script and compared it with my calculation of dof f.

The two match down to 100th mm: and I used slightly different forms of the dof.

I switched off diffraction in both cases, ie used an ML CoC of 30.

The ML reported dof is different.

I looks like the ML C is doing the calculation differently to Lua.

In focus.c the dof is calculated as:

uint64_t        coc = dof_info_coc;
const uint64_t  fd = lens_info.focus_dist * 10; // into mm
const uint64_t  fl = lens_info.focal_len; // already in mm
const uint64_t  imag = (fd-fl)/fl;
lens_info.dof_far = (fd*fl*10000)/(10000*fl - imag*lens_info.aperture*coc); // in mm


In Lua I used this:


local focus_distance = lens.focus_distance -- lock the next five varables in together in time
local dof_near = lens.dof_near
local dof_far = lens.dof_far
local focal_length = lens.focal_length
local aperture_value = camera.aperture.value
local fb_blur = menu.get("DOF Settings","Circle of Confusion")
local doff = focus_distance*focal_length/(focal_length - (fb_blur/1000)*aperture_value*((focus_distance-focal_length)/focal_length))


As I say, I've checked the equations and it looks like ML C (focus.c) is calculating a different result to Lua, with the same equation.

dmilligan

In C, division of two integers results in an integer (always rounded down to the nearest integer).
7 / 4 => 1

In Lua, unless you use "//" (double slash), integer division promotes to floating point.
7 / 4 => 1.75

Throw in some more operations to compound the difference and you can get wildly different values:
For the computation 5 / 3 - 1 / 4 * 1000:
C => 1
Lua => -248.333

It looks to me like there are some issues with the ML calculation, and I'm guessing the main source of the difference is the line:
const uint64_t  imag = (fd-fl)/fl;

The roundoff error that is created by this line gets compounded in the following line. Whenever doing integer division in C, always save the division operation for last as to avoid propagating and compounding roundoff error. Changing the last line to this algebraic equivalent should improve things a little:
lens_info.dof_far = (fd*fl*fl*10000)/(10000*fl*fl - (fd-fl)*lens_info.aperture*coc)

garry23

@dmilligan

You won't believe this, however, I just woke up at 1.48 in the morning, having just worked out that was the problem.

As I'm correcting another 'issue' in the dof part of focus.c, I'll look into making sure the integer division thing is addressed.

Once I know how to push a change, I'll upload my suggested changes to focus.c.

Cheers

Garry

garry23

@a1ex @dmilligan

I'm off to work now and as I don't know how to make a pull request (tutorial broken), I'm publishing the focus.c fix here:

void focus_calc_dof()
{
    // Total (defocus + diffraction) blur dia in microns
    uint64_t        coc = dof_info_coc*10; // from ML setting, converted to tenths here, as the base unit for blurs, to increase 'division accuracy'
    uint64_t        coc_hfd = 0; // variable used to calculate HFD

    const uint64_t  fd = lens_info.focus_dist * 10; // into mm
    const uint64_t  fl = lens_info.focal_len; // already in mm

    // If we have no aperture value then we can't compute any of this
    // Also not all lenses report the focus length or distance
    if (fl == 0 || lens_info.aperture == 0 || fd == 0)
    {
        lens_info.dof_near      = 0;
        lens_info.dof_far       = 0;
        lens_info.hyperfocal    = 0;
        return;
    }

    // Set up some dof info. Note lens_info.aperture = 10*N, eg at F/16 lens_info.aperture = 160
    // Diffraction blur at any fd = 2.44*freq*N*(1+mag). Where mag = fl/(fd-fl)
    // ie at fl = 10 and fd = 200, mag = 10/(200-10) = 0.05. But note for 100mm macro lens at min fd = 300, mag = 100/(300-100) = 0.5
    const uint64_t  freq = 550;         // mid vis diffraction freq in nm (use 850 if IR)
    const uint64_t  diff_hfd = (244*freq*lens_info.aperture)/100000; // Estimation of diffraction blur in tenths of microns, without (1+mag) factor as mag assumed at infinity, ie zero
    const uint64_t  diff = diff_hfd*fd/(fd-fl); // Diffraction blur (in tenths of units) in microns at fd, using  (1+mag), ie being technically correct :-)

    int dof_flags = 0;

    if (dof_info_formula == DOF_FORMULA_DIFFRACTION_AWARE)
    {
        // Test if large aperture diffraction limit reached
        if (diff >= coc)
        {
            // note: in this case, DOF near and far will collapse to focus distance
            dof_flags |= DOF_DIFFRACTION_LIMIT_REACHED;
            coc = 0;
        }
        else
        {
            // calculate defocus only blurs for fd and hfd, in tenths of micron
            const uint64_t sq = (coc*coc - diff*diff);
            const uint64_t sq_hfd = (coc*coc - diff_hfd*diff_hfd);
            coc_hfd = (int) sqrtf(sq_hfd); // Estimate of defocus blur at HFD in tenths of units. Doesn't vary when focus changes, as uses a mag (at infinity) of zero.
            coc = (int) sqrtf(sq); // Focus distance aware defocus blur in tenths of units

        }
    }

    const uint64_t        fl2 = fl * fl;

    // Calculate defocus hyperfocal distance H. Note this is diffraction aware, but is indepedent of fd
    const uint64_t H = coc ? fl + ((100000 * fl2) / (lens_info.aperture  * coc_hfd)) : 1000 * 1000; // use coc to test for diffraction limit reached
    lens_info.hyperfocal = H;
 
    // Calculate near and far dofs
    const uint64_t  temp = lens_info.aperture*coc*(fd-fl) // note aperture and coc in tenths of their units, hence the 100000 factor below
    lens_info.dof_near = (fd*fl2*100000)/(100000*fl2 + temp); // in mm
    if( fd >= H )
    {
        lens_info.dof_far = 1000 * 1000; // infinity
    }
    else
    {
        lens_info.dof_far = (fd*fl2*100000)/(100000*fl2 - temp); // in mm
    }

    // update DOF flags
    lens_info.dof_flags = dof_flags;
   
    // make sure we have nonzero DOF values, so they are always displayed
    lens_info.dof_near = MAX(lens_info.dof_near, 1);
    lens_info.dof_far = MAX(lens_info.dof_far, 1);

    lens_info.dof_diffraction_blur = (int) (diff+5)/10; // at point of focus rounded up/down to nearest integer
}


I believe (please check) I've handled (corrected) the integer division issue in C. Plus I've 'corrected' the HFD, ie if doesn't vary with focus but remains 'diffraction aware' based on the diffraction at infinity, ie mag = 0.

May I request this fix be urgently incorporated, as, in all current ML builds, the DoFs are broken.

Also, many of us use the DoF info in our scripts, so the current broken DoFs have a double impact.

Regards

Garry

dmilligan

Quoteurgently incorporated
Your best shot at that is to least submit the pull request yourself. The ML tutorial is not the only tutorial in the world that explains how to make a pull request.

garry23

@dmilligan

Truly I wasn't being difficult or arrogant.

I spent time trying to educate myself and make a pull request.

I just got nowhere.

Cheers

Garry

garry23

OK, I've created a fork :-)

Struggling now how to edit it.

Any pointers from a kind soul?

garry23

Pull submitted...I hope correctly  ;) :)

garry23

@a1ex @dmilligan

I'm worried I've done something wrong.

I think I've created a pull request https://bitbucket.org/garry23/focus.c-dof-fix, in fact I think I've duplicated it as well.

I believe I've flagged you two as reviewers.

But I don't see it in the ML pull request queue.

Can you reassure me: one way or the other, ie I've done it right or I've made a mistake.

Cheers

Garry

garry23

@a1ex @dmilligan

I see it now :-)

I guess it is a time delay?

Cheers

Garry

dmilligan

Nope, not there, you made the pull request against your own fork. You need to make the pull request against the main repository. Go here: https://bitbucket.org/hudson/magic-lantern/pull-requests/ and then click "Create Pull Request"

garry23


garry23

OK, I'm getting really frustrated.

I clicked the link below and I can't see a 'Create Pull Request'.

garry23

Please can someone put me out of my misery.

I clicked the link supplied by @dmilligan which takes me to this page.

I'm logged in.

But I don't see a 'Create Pull Request'


dmilligan

Do you see it on this page: https://bitbucket.org/garry23/dof-fix-for-focus.c/pull-requests/

Click it, then select for the "source" (left side) your branch with changes from your repository and select for "destination" (right side) the "unified" branch of the hudson repository.

The first google result for "bitbucket create pull request":
https://confluence.atlassian.com/bitbucket/work-with-pull-requests-223220593.html

garry23

@dmilligan

Thank you and my apologies for 'being stupid', ie forgetting how to do a pull request.