HDR ISO Shifting finer increments

Started by jkruser, September 13, 2015, 12:20:41 AM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

jkruser

I have an application for a very large dynamic range shot and wanted to use ISO shifting to get the most out of the camera.  Unfortunately when set to half, I hit ISO limits way before shutter speed becomes an issue. There are about 5 orders of magnitude of shutter speed range, but little more than one worth of usable ISO.  It seems reasonable to have a smaller increment than half.  Ideally a process that gives more control over when and how ISO shifting kicks in could be implemented, but that is beyond the scope of this request.

I have some software experience, but am not at the point that I can fully implement, build, test, and upload a project of this complexity.  With that said, I dug into shot.c and found it should be a reasonably simple process to add a Quarter shift option assuming the ISO rounding math works out.  An "Eighth" option could probably be added as well, but I am starting simple.  I have listed snippets below that would be a start to implementing this, I don't know variable naming conventions so the new name is arbitrary.

I apologize for the long post, but am trying to do what I can to help.  Feel free to ignore it if I am being too presumptuous.

Updates probably needed to shoot.c Changes marked in red

Add menu items under menu_entry shoot_menus[]

                .name = "ISO shifting",
                .priv       = &hdr_iso,
                .max = 3,
                .help =  "Also use ISO as bracket variable. Range: 100 - max AutoISO.",
                .help2 = " \n"
                         "Full: try ISO bracket first. If out of range, use main var.\n"
                         "Half: Bracket with both ISO (50%) and main variable (50%).\n",
                         "Quarter: Bracket with both ISO (25%) and main variable (75%).\n",
                .choices = CHOICES("OFF", "Full", "Half", "Quarter"),
                .icon_type = IT_DICE_OFF,

Under hdr_iso_shift, the existing code takes advantage of an inline statement to select between one of 2 factors. While this is efficient code, it needs to be replaced by a look up table  that sets a division factor (I'll call the new variable hdr_iso_shift_factor) based on the menu item chosen.  "1 Full" needs to be set to 1, "2 Half" needs to be set to 2, and "3 Quarter" needs to be set to 4.  For code organization purposes, this could be put in several different areas.  The code is trivial, but I will assume hdr_iso_shift_factor has already been set by the time the code below runs.


static int hdr_iso_shift(int ev_x8)
{
    hdr_iso00 = lens_info.raw_iso;
    int iso0 = hdr_iso00;
    if (!iso0) iso0 = lens_info.raw_iso_auto;

    if (hdr_iso && iso0) // dynamic range optimization
    {
        if (ev_x8 < 0)
        {
            int iso_delta = MIN(iso0 - MIN_ISO, -ev_x8 / hdr_iso_shift_factor); // lower ISO, down to ISO 100

            // if we are going to hit shutter speed limit, use more iso shifting, to get the correct bracket
            int rs = lens_info.raw_shutter;
            int rc = rs - (ev_x8 + iso_delta);
            if (rc >= FASTEST_SHUTTER_SPEED_RAW)
                iso_delta = MIN(iso0 - MIN_ISO, iso_delta + rc - FASTEST_SHUTTER_SPEED_RAW + 1);

            iso_delta = (iso_delta+6)/8*8; // round to full stops; also, prefer lower ISOs
            ev_x8 += iso_delta;
            hdr_set_rawiso(iso0 - iso_delta);
        }
        else if (ev_x8 > 0)
        {
            int max_auto_iso = auto_iso_range & 0xFF;
            int iso_delta = MIN(max_auto_iso - iso0, ev_x8 / hdr_iso_shift_factor); // raise ISO, up to max auto iso
            iso_delta = (iso_delta)/8*8; // round to full stops; also, prefer lower ISOs
            if (iso_delta < 0) iso_delta = 0;
            ev_x8 -= iso_delta;
            hdr_set_rawiso(iso0 + iso_delta);
        }
    }
    return ev_x8;
}

I'm not quite sure whats going on in MENU_UPDATE_FUNC and why ISO needs to be capitalized for "Full" option 1 and lower case for "Half" Option 2.  For a new "Quarter" option 3 setting this to ISO or iso would aid consistency, or all of them could be updated to reflect the actual setting more closely.

        MENU_SET_RINFO("%s%s%s",
            hdr_sequence == 0 ? "0--" : hdr_sequence == 1 ? "0-+" : "0++",
            hdr_delay ? ", 2s" : "",
            hdr_iso == 1 ? ", ISO" : hdr_iso == 2 ? ", iso" : hdr_iso == 3 ? ", ISO" : ""
        );

Now for the obligatory thanks to all of the Devs, yes, this is my first post, but I have been a long time user and love the work being done on the 70D branch.  I have always found what I needed with the search and just now had a real need to create an account.

dmilligan

You don't need a lookup table, you can get what you want with the bit shift operator: 1 << (hdr_iso - 1)

1 << 0 = 1
1 << 1 = 2
1 << 2 = 4
1 << 3 = 8
etc.

jkruser

dmilligan, You are absolutely correct.  I only suggested a look up table as it decoupled the order of the menu from actual values in the chart (I can't see the need for a 3/4 setting, but this would enable it).  My programming experience is typically not on resource limited platforms, but this seems like a good way to economize as long as it is documented that the menu order is critical.

Out of curiosity, do you have any thoughts as to its merits for inclusion?  I am after all just a weekend hack photographer/programmer.

DeafEyeJedi

Love the concept you're producing, @jkruser & enjoyed the read on this thread... Keep it up!
5D3.113 | 5D3.123 | EOSM.203 | 7D.203 | 70D.112 | 100D.101 | EOSM2.* | 50D.109

dfort

Just wondering about your 1/4 stop exposure level increments--aren't cameras set up for either 1/2 stop or 1/3 stop increments?

dmilligan

We are all just "weekend hack" programmers here. Personally, I don't see any reason not to include your changes, but that decision is ultimately up to the lead maintainer (who has been out for some time). Are you familiar with the pull request process?

Binary size is extremely limited on certain cameras (512K) so every bit of space we can save is good. Assuming the menu items are in order is not a bad one to make.

I'm also not sure why the upper/lowercase ISO selection. I would probably recommend just making it always uppercase. Doing a "blame" might give you a clue (commit message) as to why it's the way it is now.

jkruser

dfort, I wasn't exactly looking for 1/4 increments on the time/ISO variables, but to split the EV increment (I usually use 2, but have used 3 when pushed to do so) with most of the EV set with time adjustments and a smaller portion set with ISO changes.  I believe 1/2 or 1/3 (depending on settings) increments are all that are available on most models, but I think ML  does math in 1/8th increments based on the /8 statements and the naming of the ev_x8 variable.  I haven't traced out the code to figure out what is actually saved in max_auto_iso, iso0, ev_x8, but I was hoping someone could point out the fallacy of my idea.

dmilligan, I am familiar with rev management systems in theory only, I have wanted to set one up for my projects, but never could justify the time.  I'm pretty sure I won't be able to set up the build test environment to validate my changes that I would need to do before I would feel comfortable submitting a pull.

I am likewise unfamiliar with the blame process to figure out why the menu details display is iso/ISO.  I don't know the character limit of this display on other cameras, but on the 70D I think Full, Half, Quarter or ISO, 1/2 ISO, 1/4 ISO would be appropriate.

If you or anyone else is already set up to try this change out and submit a pull, I would love to see it submitted.  While it would be fun to do it myself, I just don't think it will happen.

dfort

Setting up a system to compile ML shouldn't take you too much time. Editing the code and submitting a pull request is well documented. The only issue I see is that it appears you're using a 70D and that's not a supported platform yet so you'll have to fork off of a developer's repository in order to test your theory.

I'd be glad to help you out by compiling some test builds and submitting a pull request but you should contact nikfreak to find out which 70D branch to use for your tests.

Audionut

Most of the useful threads regarding development, compiling and submitting pull requests should be compiled in this thread.

jkruser

I'm going to take the plunge and get virtual box set up.  I'm still a little fuzzy on the details as many of the well written guides are a bit contradictory, but I guess that is part of the fun.  At least the 70D branch still uses the unified shoot.c so I won't be fighting between 2 versions.

dfort

Looking forward to your contributions. I take it you are also jkruser on bitbucket?

As you probably found out there is more than one way to set up a developing environment.

jkruser

Oh the joys of setting up a build environment.  Sadly dmilligans cloud compile setup doesn't work for new non paying users due to changes in the TOS and the pre-built virtual box image from a few years ago hits a critical error loading linux mid way thru and very little is shown about the message.

I guess I get to roll my own from scratch.  There are a plethora of setup guides on this site, but after reading and following many of them from Audionuts link, I'm still a little in the dark about building under Windows, I gave up Linux in the infancy of 2.6.x

DeafEyeJedi

Well if I have an active account on the CC since I signed up for it earlier before their updated TOS -- would that help if I can just go ahead and do it for you @jkruser?
5D3.113 | 5D3.123 | EOSM.203 | 7D.203 | 70D.112 | 100D.101 | EOSM2.* | 50D.109

dfort

Cygwin is fairly easy to setup a system that will compile ML.

jkruser

DeafEyeJedi, you are welcome to try a build with this.  I am on the road and wont be able to put up a complete .c file with the changes on bitbucket, but between my post and dmilligan's bit shift, it may all come together.

On a slight tangent, is scripting still left out of modern builds?  I have a shoot on the 27th and am looking at backup options in case I cant compile for the 70D in time.

jkruser

Never mind about the scripting, I finally found dmilligan's thread for his LUA work at http://magiclantern.fm/forum/index.php?topic=14828.0

I'm still going to work on getting a build environment for the ISO ramping, but it looks like I have something else to play with.

DeafEyeJedi

5D3.113 | 5D3.123 | EOSM.203 | 7D.203 | 70D.112 | 100D.101 | EOSM2.* | 50D.109

nikfreak

[size=8pt]70D.112 & 100D.101[/size]

dfort

Hey jkruser--here is yet another setup to compile ML:

http://magiclantern.fm/forum/index.php?topic=15845.msg154356#msg154356

Checked out your bitbucket page but there is nothing to see there:

https://bitbucket.org/jkruser/

jkruser

dfort, thanks for the link, I'm installing it now.  I haven't put anything up on bitbucket yet, my hobby time has been scouting locations for the upcoming eclipse.  I'll post when I get something that at least has a chance of compiling.

dfort

Great--we should probably get back on topic with HDR ISO Shifting and post any compiling and merging issues on their appropriate topics. It looks like you've got a new platform, the 70D, with lua scripting, compiled with a setup that requires some tweaks to the code to get it working. Lots of pieces need to come together!

jkruser

Ok, it compiles.  I had one typo in my above code, the comma needs to be removed from the end of the line

"Half: Bracket with both ISO (50%) and main variable (50%).\n"

Total code increase is 192 bytes, most of that is in the additional menu item text, but the bit shift was less efficient than I had hoped.

I'm still figuring out how to get shoot.c into my bit bucket Repository (it is public now)

dfort

Are you following dmilligan's Mercurial Tips? I learned the hard way that you should fork the main repository then make your own working branches. Make sure not to commit to unified.

If you are using the SourceTree app it is very easy to commit and push your changes to bitbucket. However it is also possible to do it all in the command line. I think the commands you are looking for are "hg commit" and "hg push" check out the QuickStart for the impatient. My first few pull requests I just edited online then re-cloned the branch to see if it compiled. Rather kindergarten but it worked.

jkruser

dmiligan's tips are useful as always, but I'm in the boat of trying to figure out how to fork the 70D repository while committing to the central nightly project.  I don't want to get too far off topic, but any ideas?

dfort

There's probably a right way, a wrong way and another way to do it.

You're only modifying one source file, shoot.c , right? The problem is that nikfreak's also modified shoot.c in his 70D pull request and you need my Windows pull requests to get it to compile.

I suppose that the right way would be to fork the main repository (https://bitbucket.org/hudson/magic-lantern) make a new branch from unified and apply the 70D pull request and my Windows pull requests to your new branch (sorry, I don't know how to apply pull requests across repositories) then make another branch off of unified and do your HDR ISO Shifting work in that branch. Finally, go back to your personal 70D branch and merge in your HDR ISO Shifting work, compile and test. When it comes time to do the pull request, use the branch you made just for your HDR ISO Shifting work.

The wrong way would be to merge both the main repository and nikfreak's repository into your workspace. This is possible but: Merging unrelated histories will generally result in an incoherent history and is not recommended.

Another way is simply not to worry about keeping things in order. Grab the 70D code, replace those few Windows specific files I pointed out in this post and do your work on that. When you decide to submit your pull request you will need to make a fork of the main repository, branch off unified and put your finished shoot.c in there. Oh, and just copy/paste over the 70D stuff so it doesn't show up on your pull request.

If you are planning to make a pull request in the next few days or weeks, go with the last method because it has been a while since any pull request was merged and it seems that will continue a while longer. If there were more merges going on then it would be better to do things the right way so you can pull and update the new commits to your working branches and avoid conflicts when you get around to making your pull request.

Disclaimer, I'm just learning this stuff so sorry if this sounds a bit confusing or maybe even wrong.