@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
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);
@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
@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.
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)
@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
@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
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.
@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
OK, I've created a fork :-)
Struggling now how to edit it.
Any pointers from a kind soul?
Pull submitted...I hope correctly ;) :)
@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 (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
@a1ex @dmilligan
I see it now :-)
I guess it is a time delay?
Cheers
Garry
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"
Bugger!
OK, I'm getting really frustrated.
I clicked the link below and I can't see a 'Create Pull Request'.
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'
(http://thumb.ibb.co/cSnn7a/Capture.jpg) (http://ibb.co/cSnn7a)
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
@dmilligan
Thank you and my apologies for 'being stupid', ie forgetting how to do a pull request.