#887 Merged at e7ca98e
Repository
aprofiti
Branch
manual_lens_info
Repository
hudson
Branch
manual_lens_info
Author
  1. alessandro profiti
Reviewers
Description

Changes:

  • Allow to write Lens Info metadata to MLV file

  • Added support for AF confirm Chip

  • Implemented set manual_aperture for lens with af chip

  • Added support for saving focal_min and focal_max values

  • Fixed values for Focal Len in Lens Info

  • Added ability to set and get serial number in lua

  • Added submenu for changing aperture and focal length in camera

  • Auto lower boundary and custom values for Aperture

  • Added ability to autoload previous lens on power on

Todo:

  • Find a way to keep lens selected when changing shoot mode (Added. Need to Test)

Comments (37)

  1. Alex

    Quite a few interesting things added here.

    TODO: play with it and find out what's the issue with changing shooting mode (best guess: aperture gets overwritten).

    Nitpicks:

    • lens.lens_id, lens.lens_exists, vs lens.name, lens.serial... (well, it makes sense to say lens_info.lens_exists rather than lens_info.exists, for ML core, but for the scripting API, lens.exists sounds a lot better to me, so it can be renamed in the wrappers)
    • lens.focal_length == "1-65535mm" - this one is integer iirc, so this comparison would be always false.
    1. alessandro profiti author
      1. I can rename it after a bit of cleanup and testing
      2. As now I have more understanding of how it works, It should be changed in:
      (lens.focal_min == 1 and lens.focal_max == 65535)
      

      but I am not sure if it is needed anymore, because checking on name when using AF Chip should be sufficient; focal length would not be overwritten if "focal_length" attribute is not present (when is present only the focal_length -> fixed focal is stored in Lens Info, with focal_min and focal_max -> Show Range in Lens Info).

      Regarding shoot mode: actually it rewrite the correct values because menu is prompted again; I was thinking of storing the name of the selected Lens in a variable and checking at the start of the Lens Handler if the current lens.name value equals the variable, but it fails because it returns a null o empty string... Need some help here

  2. alessandro profiti author

    Added submenu for changing Aperture and Focal Length in camera from Lens Info Prefs Menu. Aperture value in 1/2 Stop from f1.0 to f32, it can be improved by showing a subset of value starting from the max Aperture of selected lens.

    I had to update min and max value of Focal Length in update_menu(), because when placing in select() the submenu get broken..

  3. Lars Steenhoff

    Would love to see this merged into experimental 4k at some point as this would give the most exposure. The lens info experimental builds are very old by now/

  4. Alex

    Hm, merging lua_fix didn't go very well, but the PR merges cleanly from 84858ed91281 (right before f45896fb6b08).

    I've retried your merge (84858ed91281+aa9c7f1b64b2) and got slightly different results - guess it's because of that.

    Since Bitbucket no longer allows setting the PR to an older changeset, do you mind stripping your merge (f45896fb6b08) from your repo? I believe this will clean the diffs on this page.

    (I can still merge it from an older changeset locally, but if I do that way, the PR will no longer be marked as "merged")

  5. alessandro profiti author

    Yes, I don't mind it. I held back pushing more commits because of that, but I'll probably need to strip from bitbucket because sourcetree does't allow me to do that.

    Do I need to sync my repo to latest changes first?

    1. Alex

      No, just strip from current state (from bitbucket UI). After that, syncing to latest changes should be without conflicts.

          1. alessandro profiti author

            I retarted from a clean clone, pulled again manual_lens_info from ML repo to be able to see c84f19d837a8, but when pushing from sourcetree: "abort: push creates new remote head c84f19d837a8 on branch 'manual_lens_info'!"

            EDIT: I can try with committing again changes

  6. Alex

    Seems to work.

    Got some trouble with "Autoload Lens" (it doesn't always get saved); might be a backend issue though.

    60D: in movie mode, when starting without lens, the manual lens dialog pops up, but remains stuck (cannot move the selection, cannot press SET, can press half-shutter to discard the dialog). Also likely a backend issue. Also present before this PR.

    Reproduced the crash after recording a raw video. This one was not present before this PR. It doesn't crash if lens.lua is not running.

    1. alessandro profiti author

      I'm working in a modified version of "config.lua" lib: It allow to have multiple menu in a script and save in a single .cfg all of them, this allow me to move "Autoload Lens" in "Info Prefs" not in "Manual Lens". (in current implementation will not save values of second menu if more than one "config.create_from_menu()" is present)

      Already managed to save all menu's values, i'm figuring how modify load function.

      Does It crashes when you apply the 64Byte lens name patch? Because without it should not be any issues with recording.

      Edit: It should prompt on boot for selection when no lens is attached, can avoid this if autoload is enabled, in this case previous lens will be automatically selected without dialog.

        1. alessandro profiti author

          Mmm.. I was able to record more than one video in a row, but I tested only ut to 4-5 seconds of raw video due to limited card speed... Give me a minute or two to check at the current state

          Edit: no ERR80 with 50D (8 seconds of video)

  7. Lars Steenhoff

    I made some simple scripts before with just one lens each because I also has some error not being able to select multiple from the start up script. They way I use them is just one lens script that autoloads at startup and does not require user input.

    When I want to change a lens I have the other single lens scripts and auto load the other one and disable the one that I changed.

    try to see if this one works, it did not give me any error 80

    -- Nikon 35 f1.4
    -- Prints lens_info.name on the console
    
    menu.close()
    console.show()
    if lens.name == "" then
        print("non-cpu lens")
        lens.name = "Nikon Ais NIKKOR 35mm f/1.4"
        lens.focal_length = 35
        lens.aperture = 14
    end
    print("lens: "..lens.name)
    print("focal_length: "..lens.focal_length)
    print("aperture: "..lens.aperture/10)
    -- print "Press any key to exit."
    -- key.wait()
    console.hide()
    
    1. alessandro profiti author

      Lars, can you try with this PR? Just to know if it is present also on your camera, if it's model specific or something else.

      1. Alex

        Hypothesis: when adding new "choices" to the menu, there is a malloc, but there is no free. Memory leak?

        When creating the menu with predefined choices, it's less of an issue, as it's only created once (and there's no API to delete these menus yet; that's one of the reasons why scripts that create menus are labeled "complex" and forced to stay in background). When you change these choices every time lens properties are updated (in LiveView that may happen many times per second), you'll run out of memory eventually.

        Going to test this.

        edit: nope, it's not this; it's only called a few times.

        1. alessandro profiti author

          I thought the same and was a question to ask to you which got forgotten.

          Some time ago I checked memory usage in menu by switching lenses from one with aperture list and one whitout to trigger newIndex for choices method. Values from memory menu didn't change much, if I remember correctly just overall free memory decreased by 1-2kB.

          Other thing to check is for weird values in lens wich can cause trouble when adding metadata to MLV. Does it crash while using FRSP?

          1. Alex

            Will check after narrowing down this one.

            Commented out the two property handlers => no more ERR80. Focal length displayed as 50mm.

            Added back LV_LENS => no ERR80 (edit: ERR80 on second attempt...). Focal length correct again.

            1. alessandro profiti author

              One thing I noticed: in LV focal length will be set automatically to "50mm"... Adding handler fixes this issue but noticed by adding a console print, that it get called A LOT OF TIMES continually during LV.

              Maybe is this which can cause troubles over time?

              1. Alex

                Still ERR80 with this:

                function property.LV_LENS:handler(value)
                    print(value)
                end
                

                or even with empty property handler.

                However, if I delete everything else in the script, and leave just the prop handler, I no longer get err80 (why?!)

                Card speed is not an issue; return size_used; in write_frames (so it won't save anything) and still crashes. No crash with FRSP MLV.

                1. alessandro profiti author

                  Mmm.. even when eliminate completely LV_LENS Handler and leaving other as is? ERR80 doesn't appear to me, so I can't reproduce it and test..

                  Also check this:

                  function property.LENS_NAME:handler(value)
                      if is_manual_lens() then
                          -- If we are changing shooting mode, restore values and don't prompt again for lens selection
                          if lensSelected then
                            -- Restore lens value as before changing shooting mode
                            restore_lens_values()
                          else
                            task.create(select_lens)
                          end
                      else
                  

                  Let's try to remove code related to copy values when changing shooting mode, as I noticed that camera will not sense when switching lens with AF confirm chip (previously will prompt menu for selection)

                  function property.LENS_NAME:handler(value)
                      if is_manual_lens() then
                            task.create(select_lens)
                      else
                  
                  1. Alex

                    Right, no crash if I just delete property.LENS_NAME:handler and leave everything else untouched.

                    If that property handler is present, even if empty, it crashes. But if the script contains only that property handler (and nothing else), it no longer crashes.

                    Also tried deleting init_mlv_chunk_headers/write_mlv_chunk_headers in mlv_lite; still crashes.

                    1. alessandro profiti author

                      A potentially headache problem... Can it be a backend issue?

                      Can you describe exactly how to reproduce ERR80? I'm limited with testing raw recording, but maybe it show up as a different behavior on my camera.

                      1. Alex

                        Found an easier way to reproduce.

                        1. start camera without lens, run the manual lens script, select a lens
                        2. enter the Free Memory dialog
                        3. it actually prints what's going on!

                        Will look into it later, afk now.

                        1. alessandro profiti author

                          This is what i see: VRAM27.jpeg When entering LV the '0' at bottom moves in other memory regions: VRAM28.jpg

  8. Lars Steenhoff

    I did not yet have the time to check on the camera, I was testing before on the 4k branch with manual lens info merged into that. and got the not responding keys on start-up problem.
    after that I stripped the script to make it only have one lens without user input.

    This has been working for me.
    Nice to see your narrowing down the issue!

  9. alessandro profiti author

    Noticed I was using a build based on previous work, the one where I merged fixes from lua_fix (then stripped). Sorry a1ex.

    So compiled again from current PR status (with latest merge from manual_lens_info branch) and got some issues: I got the first time "card file system error" and unable to record any other videos... No stack overflow from Free Memory Dialog.

    I tried again and was able to record more than one raw video with mlv_rec but after exit LV and entering again I got black screen as already described in previous posts. Now I'm not sure that black screen was present before, even without experimental patch

  10. Alex

    Diagnosed - it's an issue in Lua property backend and/or ML memory backend; using malloc from property handlers fails when some allocator keeps the mem_sem busy for a longer time. The SRM allocator takes a couple of seconds to run; during this time, malloc requests will be delayed, and all those properties triggered in LiveView (many times per second) will get queued and overflow the queues in Canon code.

      1. Alex

        Workaround (will fail with many scripts loaded):

        diff -r 7f04e4036bdd modules/lua/lua_property.c
        --- a/modules/lua/lua_property.c
        +++ b/modules/lua/lua_property.c
        @@ -17,6 +17,7 @@
         #include <propvalues.h>
        
         #include "lua_common.h"
        +#include "umm_malloc/umm_malloc.h"
        
         // !!!DANGER WILL ROBINSON!!!
         //#define LUA_PROP_REQUEST_CHANGE
        @@ -96,18 +97,18 @@
                         printf("[Lua] semaphore timeout: prop handler %d (%dms)\n", lua_prop->prop_id, 1000);
                     }
                 }
        -        free(msg->value);
        -        free(msg);
        +        umm_free(msg->value);
        +        umm_free(msg);
             }
         }
        
         static void lua_prophandler(unsigned property, void * priv, void * addr, unsigned len)
         {
             if (!lua_prop_queue) return;
        -    struct lua_prop_msg * msg = malloc(sizeof(struct lua_prop_msg));
        +    struct lua_prop_msg * msg = umm_malloc(sizeof(struct lua_prop_msg));
             msg->property = property;
             msg->len = MIN(len,255);
        -    msg->value = malloc(msg->len + 1);
        +    msg->value = umm_malloc(msg->len + 1);
             if(msg->value)
             {
                 memcpy(msg->value, addr, MIN(len,255));
        
        1. alessandro profiti author

          No more black screen and also works with 64 Byte lens name! I verified presence of ELNS Block and noticied no more sparse Null block in mlv videos

          This means that after finishing work of config.lua, I can now apply the patch and commit! After that it need just a few reviews and work on this PR can be considered finish

          Thank you a1ex!

          1. Alex

            Yay!

            I'll try to find something that doesn't break when the "umm" memory block gets full. I also have a feeling some Lua property handlers may be dropped in the current implementation; they are executed asynchronously from Canon ones (likely to avoid large delays and stack overflows); that needs some low-level checking.

            You can probably break mlv_rec in the same way (without Lua), as it has a similar issue when the level indicator is enabled. There might be other parts of the code using this.

            1. Alex

              Alright, reworked the property backend to avoid this issue, without requiring memory allocation.

              edit: now that I think about it, this may still get in trouble when running large scripts and the umm buffer runs out. Boo...