CMOS/ADTG/Digic register investigation on ISO

[timbytheriver]

You have a discrepancy between one of your comments and one of your lines of code.  You say "ADTG4 0xFE gain", but your code has "adtg_new[21] = (struct adtg_new) {2, 0xFE, 0};", which is trying to modify ADTG2 (the mask, which is the first field in the struct, is 2). 

@names_are_hard Great catch here, thank-you! I had meant to target ADTG4 as per my comment!

In your added code it looks like you always want to set ADTG2 and ADTG4 to the same values. 

Not necessarily: the values in adtg_gui differ by a few points over the 8 registers. That's why I'm still wondering how iso_regs sums these same values into just a single figure. I'm sure it's important!

[names_are_hard]

Not necessarily: the values in adtg_gui differ by a few points over the 8 registers

Not in the added code that you showed   If they differ elsewhere, I can't see it.

Again, I don't understand your description of "sum into a single figure".  I'm going to take another guess about your meaning, but if I'm wrong it would really help me if you could describe what you mean in more detail.  Do you mean that the UI / menu for some settings takes a single number, but when you look at the code you see the setting seems to be related to 4 lines of code that each set a value?  If so, I think that is because some menu settings are intuitively "one value", but the camera hardware requires 4.  The menus choose to only show one value, and then map to the equivalent 4 values.  The menu *could* allow the user to pick each value individually, but generally this wouldn't help the user, so the choice has been made to simplify.

As an example, guessing you're familiar with RGB colour, imagine a graphical colour picker.  You click on *one* colour.  But the code needs 3 values, R, G and B.  The code translates the chosen single value to three values.

For the specific code you're interested in I guessed the registers were RGGB related, hence 4 channels.  Now I've found comments in the code referencing columns, so maybe it's not colours and the sensor has 4 individually controllable amps.  Either way, the code assigns 4 values to control each individually - but the user wants all the ISO gains the same, not for every 4th column to be brighter than the other 3 (we'll ignore dual-ISO for now...).  So for ISO settings the menu takes one value, but behind the scenes still has to set 4, because that's how the hardware works.  Try setting only 1 of the 4 and you should see what actually changes.

Again, a fair chunk of guesswork here.  Hopefully Danne can set me right on mistakes, also hopefully I saved him some typing.

[timbytheriver]

Do you mean that the UI / menu for some settings takes a single number, but when you look at the code you see the setting seems to be related to 4 lines of code that each set a value?  If so, I think that is because some menu settings are intuitively "one value", but the camera hardware requires 4.  The menus choose to only show one value, and then map to the equivalent 4 values.  The menu *could* allow the user to pick each value individually, but generally this wouldn't help the user, so the choice has been made to simplify.

This is exactly what I was trying to describe, yes! But the settings vary between module menus: One user set value in one module; A set of values for another module menu.!

The different values for each reg/column can be seen in this screenshot of adtg_gui:



I'd like to know how all those separate / different values end up being a single value in iso_regs – and whether it even matters at all! 

[names_are_hard]

We found the answer!  Yay!

I'll wait for Danne to tell you where the code lives, but I can tell you the (very likely) *reason* why some menus have 4, some have 1 for the same value.  One of the menus is for fine control, one is for ease of use.  I mean, how often do you want to change the ISO gain for one quarter of the pixels only?  But, it's Magic Lantern, so it should let you if you really really want to.

[Danne]

@timbytheriver
Study iso_reg code for answers but as @names_are_hard tells you a1ex must have connected the dots for a few regs here. I would recommend that you work the adtg_gui regs as a starting point also in crop_rec.c. Please test following. Look for this place:

Code Select Expand

    /* should probably be made generic */
    if (is_5D3 || is_100D || is_EOSM || is_700D || is_650D || is_6D)
    {
Then replace with this:

Code Select Expand

    /* should probably be made generic */
    if (is_5D3 || is_100D || is_EOSM || is_700D || is_650D || is_6D)
    {
                    adtg_new[20] = (struct adtg_new) {2, 0x8, 0x60 + reg_8};
                    adtg_new[21] = (struct adtg_new) {2, 0x9, 0x61 + reg_9};
                    adtg_new[22] = (struct adtg_new) {2, 0xA, 0x62 + reg_a};
                    adtg_new[23] = (struct adtg_new) {2, 0xB, 0x60 + reg_b};
                    adtg_new[24] = (struct adtg_new) {2, 0xFE, 04 + reg_fe};
Now also add following to be able to modify above adtg values from crop_rec.c:
Find this place:

Code Select Expand

            {
                .name   = "hdr iso B",
                .priv   = &HDR_iso_b,
                .max    = 6,
                .choices = CHOICES("OFF", "iso100", "iso200", "iso400", "iso800", "iso1600", "iso3200"),
                .help   =  "HDR workaround eosm",
                .advanced = 1,
            },
Replace with:

Code Select Expand

            {
                .name   = "hdr iso B",
                .priv   = &HDR_iso_b,
                .max    = 6,
                .choices = CHOICES("OFF", "iso100", "iso200", "iso400", "iso800", "iso1600", "iso3200"),
                .help   =  "HDR workaround eosm",
                .advanced = 1,
            },
            {
                .name   = "reg_8",
                .priv   = &reg_8,
                .min    = -2000,
                .max    = 2000,
                .unit   = UNIT_DEC,
                .help  = "reg 8",
                .advanced = 1,
            },
            {
                .name   = "reg_9",
                .priv   = &reg_9,
                .min    = -2000,
                .max    = 2000,
                .unit   = UNIT_DEC,
                .help  = "reg 9",
                .advanced = 1,
            },
            {
                .name   = "reg_a",
                .priv   = &reg_a,
                .min    = -2000,
                .max    = 2000,
                .unit   = UNIT_DEC,
                .help  = "reg a",
                .advanced = 1,
            },
            {
                .name   = "reg_b",
                .priv   = &reg_b,
                .min    = -2000,
                .max    = 2000,
                .unit   = UNIT_DEC,
                .help  = "reg b",
                .advanced = 1,
            },
            {
                .name   = "reg_fe",
                .priv   = &reg_fe,
                .min    = -2000,
                .max    = 2000,
                .unit   = UNIT_DEC,
                .help  = "reg fe",
                .advanced = 1,
            },
Lastly look for this:

Code Select Expand

static int isopatchoff = 1;
//static int preset1 = 1;
//static int preset2 = 1;
//static int preset3 = 1;
Replace with:

Code Select Expand

static int isopatchoff = 1;
//static int preset1 = 1;
//static int preset2 = 1;
//static int preset3 = 1;
static int32_t  reg_8 = 0;
static int32_t  reg_9 = 0;
static int32_t  reg_a = 0;
static int32_t  reg_b = 0;
static int32_t  reg_fe = 0;

Or just download my changes here and compile:
https://bitbucket.org/Dannephoto/magic-lantern/downloads/crop_rec.c

Now you should be able to modify above regs from with crop mode sub menu. They´re called reg_8, reg_9 and so on. If you want to add more regs just try to follow my code and add more.
Don´t forget. If you hit this limit:

Code Select Expand

    /* expand this as required */
    struct adtg_new adtg_new[30] = {{0}};
Keep expand it.

EDIT:
Now the starting point will apply on all presets. So if numbers are ok as starting point I expect them to work otherwise not.

[timbytheriver]

Wow @Danne! Christmas just came early. Many thanks! 

I am working from an old build of yours Nov2018 iso-experiments. Should I continue with that build – or update to your latest, and use that cro_rec.c ?

[Danne]

I think this is an old one. But do test it. Or manually add the regs.

I also note a few errors in my manual pastes so double check against crop_rec.c file.

[timbytheriver]

Sure thing. Thanks again!

[timbytheriver]

@Danne

Not working here yet!

I've cloned your most recent crop_rec_4k_mlv_snd_isogain_1x3_presets build and edited the lines you said. I've checked my edits with your crop_rec.c download. They are the same.

The Q menu appears with all the options settable, but the pre-amps are not being adjusted. I can tell this instantly because the Histogram allows for 'OVER'. With the regs working the histo should never be able to go into 'OVER'.

Also the _fe reg needs to allow for a value of 0 to be switched on. I can't select 0 with the present code.

[Danne]

Yes, it usually takes a year or more to achieve something significant in here .

[timbytheriver]

?

EDIT:
Can't see any cut/paste errors! Have compared line by line with your download crop_rec.c
No compile errors either! ?

[names_are_hard]

Danne said:

Now you should be able to modify above regs from with crop mode sub menu.

Sounds like it does that?  Just not every thing you wanted?  I'm pretty sure it was intended as a starting point for you to debug and learn from.  It's good that it compiles and changes the menus, that's a strong start and means you can make changes knowing whether or not you broke things or improved things.

If you want some more general instructions in learning how to write and/or debug C, I'm happy to help, but we should move it out of this thread, too off-topic.

[Danne]

Yes, starting point. That's how I ironed out most eosm presets by porting regs into crop_rec.c. Of course this only works because a1ex layed out the complete groundwork before.

[timbytheriver]

Thank-you gents for the ever-so-slight rap across the knuckles!  I shall persevere.

@names_are_hard C-advise would be welcome. Thanks!

[timbytheriver]

Just about to ask a specific code question when I noticed that all Q menu > advanced, register changes are having no effect on the camera!

So maybe something more generic with the Q menu operation is at fault? 

I tried setting random values on:
8
9
a
b
fe

AND

bit depth
ratios
vertical offset
horiz offest
alter height
alter width
et al...

None of these changes are affecting the camera. Is there some special flag that needs to be set to allow for crop_rec Q menu > advanced to be made active? Maybe I'm doing some schoolboy error?

I'm running Danne's most recent crop_rec_4k_mlv_snd_isogain_1x3_presets on 5D3 1.1.3

Any help pls?

Many thanks.

[Danne]

You need to enable a preset from crop rec of course.

[timbytheriver]

Err, yes!

I have. did! UHD 1:1 > Q > Advanced.

BTW
If I enable the Terminal readout, I see no changes there when I try to alter Q menu settings. Should there be?

[Danne]

Sorry. I'm out. No more time. I suggest you start a new thread about your progress and most important. Keep trying and then try harder to solve basic issues.

[timbytheriver]

Righto. I'm not sure my research is anything other than an extension of the work outlined in this thread, so unless I'm instructed otherwise by a grown-up, I'll keep my posts here.

[histor]

I promised here this summer to provide better statistics on SNR while using reduced gains on 5D2.
A set of cloudy sky flats repeated twice. (Notice also, how much SNR is lost after 6 consequent shots, as the sensor becomes 7 degrees hotter). Numbers calculated with a fast PixInight script.

Code Select Expand


SNR = 26.42 db, K IMG_7671iso100_DFE111
SNR = 21.04 db, K IMG_7672iso200_DFE111
SNR = 16.76 db, K IMG_7673iso400_DFE111
SNR = 15.92 db, K IMG_7674iso100
SNR = 13.52 db, K IMG_7675iso200
SNR = 12.07 db, K IMG_7676iso400

SNR = 23.85 db, K IMG_7677iso100_DFE111
SNR = 19.55 db, K IMG_7678iso200_DFE111
SNR = 15.68 db, K IMG_7679iso400_DFE111
SNR = 15.27 db, K IMG_7680iso100
SNR = 12.92 db, K IMG_7681iso200
SNR = 11.63 db, K IMG_7682iso400


RawDigger produces a bit different results (and I cannot explain why), but the leader stays the same.
You can download the samples and check my math.

So it works, as expected. The only problem now: it's damn cold to tweak registers outdoors 

[timbytheriver]

Thanks @histor for sharing the encouraging test results! Every little advancement helps.

[timbytheriver]

Those users interested in shooting for the cleanest shadows in video might like my new test build for 5D3 1.1.3 only (at present) which includes a crop_rec 'UHD 1:1 Cleaner-ISO' preset. When selected, this makes changes to the iso registers, and yields cleaner, less-noisy shadows – whilst retaining dynamic range – in stock Canon ISOs of 160-800. As video shooters know: even the slightest clean-up in noise can mean the difference between a usable or unusable shot.

You can see this preset in use in @70MM13's 'DARK music video' here https://www.magiclantern.fm/forum/index.php?topic=24690.msg223329;topicseen#msg223329

These register tweaks are based on tools and data contributed to this thread by: @a1ex, @Danne, @Audionut, @michael84, and others.

My hope is that we can maybe move these presets into the Q sub-menu of the crop_rec preset (see a few posts back). But until that happens, here are the results so far.

Disclaimer: I am noob-developer! What I have picked up so far is from reading, and re-reading mainly this thread – and with help along the way from y'all. The usual risks apply if you want to test this code, but so far my own 5D3 is in fine shape!

Download is here: https://bitbucket.org/rivertim/magic-lantern/downloads/(5D3 1.1.3 only)

[names_are_hard]

Congrats on getting it working, Tim!  Good persistance!

[Danne]

Nice @timbytheriver  .
Trying to get into the source code but seems your repo is password protected. Could you include your changes in your code please.

[timbytheriver]

Thanks. 

seems your repo is password protected

Really? If I log out and try to access Source > Modules > foo.c I can look at the raw code. I thought I'd already committed my changes... will check.

EDIT
Sourcetree reports 'Nothing to Commit'. The bitbucket repo has my updates in crop_rec.c