Lua: Removing checks on lens_focus()

[wow0000]

Hello,

Issue

The lua function lens.focus(), when called with continuous autofocus enabled will trigger a lua error that cannot be properly handled. A lua error draw the console and print a large error. I provided an example of what a lua error looks like. It can be annoying in LiveView.

Workaround

It is possible to use the function pcall that act as a try {} catch() {}. However this is considered bad practice and is slow. An example is provided in the Reproducing section.

Reproducing

Run the following script in LiveView with AutoFocus and ContinuousAF enabled then press half shutter + up arrow to trigger the error.

ML/scripts/focus.lua

local is_halfed = false

event.keypress = function(key)
    if key == 1 or key == 2 then -- half shutter
        is_halfed = key == 1
        return true
    end
    if not is_halfed or not lv.enabled or not lens.af or lens.autofocusing then return true end

    if key == 12 then
        lens.focus(1)
        return false
    elseif key == 19 then
        pcall(lens.focus, -1) -- Workaround here
        return false
    elseif key == 10 then
        if console.visible then
            console.hide()
        else
            console.show()
        end
        return false
    end
    -- 1 = half shutter
    -- 2 = release half
    -- 12 = up arrow
    -- 19 = down arrow
    -- 10 = OK
    return true
end

Potential patch

diff --git a/modules/lua/lua_lens.c b/modules/lua/lua_lens.c
index d2ae5dc54..5f54fb462 100644
--- a/modules/lua/lua_lens.c
+++ b/modules/lua/lua_lens.c
@@ -122,7 +122,7 @@ static int luaCB_lens_focus(lua_State * L)

     if (!lv) return luaL_error(L, "lens.focus() only works in LiveView.");
     if (is_manual_focus()) return luaL_error(L, "lens.focus() requires autofocus enabled.");
-    if (is_continuous_af()) return luaL_error(L, "lens.focus() requires %s AF disabled.", is_movie_mode() ? "movie servo" : "continuous");
+    //if (is_continuous_af()) return luaL_error(L, "lens.focus() requires %s AF disabled.", is_movie_mode() ? "movie servo" : "continuous");

     lua_pushboolean(L, lens_focus(num_steps, step_size, wait, delay));

diff --git a/src/lens.c b/src/lens.c
index e56b552ec..6733a2b9f 100644
--- a/src/lens.c
+++ b/src/lens.c
@@ -677,7 +677,7 @@ lens_focus(

     if (!lv) return 0;
     if (is_manual_focus()) return 0;
-    if (is_continuous_af()) return 0;
+    //if (is_continuous_af()) return 0;

     if (num_steps < 0)
     {

Removing these line does work on my device. lens_focus can work even with continuous_af enabled I'm daily driving a Canon 100D on commit 8c1a3278aceb051c1314c8a39e6beff5c4e84025 If this patch cannot be applied on every device, then this Pull Request #122 is still relevant

[reticulatedpines]

I don't know Lua well at all. I've done a little reading and some benchmarking.

When you say "will trigger a lua error that cannot be properly handled" - can this in fact be handled by pcall()? Are you referring to the potential speed cost when you say it can't be done properly?

I benchmarked pcall (on desktop, not cam). While pcall is noticeably slower than a normal call (about 10x), I can still do 50 million pcalls in 12s. I was using a minimal function, whereas here we have lens.focus, which will do more work, so the overhead of pcall will be less. Your code is doing key press handling, so cannot be rapidly called repeatedly.

I don't see any concerning cost to pcall, and it seems to me that this is precisely the circumstance where using pcall is appropriate: an external API can error, pcall allows you to handle this without needing to modify code outside of your domain (that is: modify ML code to allow a user script to work).

As I say, I don't know Lua well, so please let me know if I've misunderstood something or have something incorrect.