Author Topic: CMOS/ADTG/Digic register investigation on ISO  (Read 589748 times)

timbytheriver

  • Senior
  • ****
  • Posts: 439
Re: CMOS/ADTG/Digic register investigation on ISO
« Reply #1175 on: December 05, 2019, 02:23:13 PM »

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!

Quote
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!
5D3 1.1.3
5D2 2.1.2

names_are_hard

  • Contributor
  • Member
  • *****
  • Posts: 153
  • 200D idiot
Re: CMOS/ADTG/Digic register investigation on ISO
« Reply #1176 on: December 05, 2019, 02:47:55 PM »
Quote
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

  • Senior
  • ****
  • Posts: 439
Re: CMOS/ADTG/Digic register investigation on ISO
« Reply #1177 on: December 05, 2019, 02:56:08 PM »
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!  :P

5D3 1.1.3
5D2 2.1.2

names_are_hard

  • Contributor
  • Member
  • *****
  • Posts: 153
  • 200D idiot
Re: CMOS/ADTG/Digic register investigation on ISO
« Reply #1178 on: December 05, 2019, 03:10:09 PM »
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

  • Contributor
  • Hero Member
  • *****
  • Posts: 6271
Re: CMOS/ADTG/Digic register investigation on ISO
« Reply #1179 on: December 05, 2019, 04:03:58 PM »
@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]
    /* should probably be made generic */
    if (is_5D3 || is_100D || is_EOSM || is_700D || is_650D || is_6D)
    {
Then replace with this:
Code: [Select]
    /* 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]
            {
                .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]
            {
                .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]
static int isopatchoff = 1;
//static int preset1 = 1;
//static int preset2 = 1;
//static int preset3 = 1;
Replace with:
Code: [Select]
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 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

  • Senior
  • ****
  • Posts: 439
Re: CMOS/ADTG/Digic register investigation on ISO
« Reply #1180 on: December 05, 2019, 05:01:57 PM »
Wow @Danne! Christmas just came early. Many thanks!  :D

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 ?

5D3 1.1.3
5D2 2.1.2

Danne

  • Contributor
  • Hero Member
  • *****
  • Posts: 6271
Re: CMOS/ADTG/Digic register investigation on ISO
« Reply #1181 on: December 05, 2019, 05:04:54 PM »
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

  • Senior
  • ****
  • Posts: 439
Re: CMOS/ADTG/Digic register investigation on ISO
« Reply #1182 on: December 05, 2019, 05:08:56 PM »
Sure thing. Thanks again!
5D3 1.1.3
5D2 2.1.2

timbytheriver

  • Senior
  • ****
  • Posts: 439
Re: CMOS/ADTG/Digic register investigation on ISO
« Reply #1183 on: December 05, 2019, 06:40:00 PM »
@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.
5D3 1.1.3
5D2 2.1.2

Danne

  • Contributor
  • Hero Member
  • *****
  • Posts: 6271
Re: CMOS/ADTG/Digic register investigation on ISO
« Reply #1184 on: December 05, 2019, 06:50:17 PM »
Yes, it usually takes a year or more to achieve something significant in here ;).

timbytheriver

  • Senior
  • ****
  • Posts: 439
Re: CMOS/ADTG/Digic register investigation on ISO
« Reply #1185 on: December 05, 2019, 06:54:13 PM »
? :o

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

names_are_hard

  • Contributor
  • Member
  • *****
  • Posts: 153
  • 200D idiot
Re: CMOS/ADTG/Digic register investigation on ISO
« Reply #1186 on: December 05, 2019, 07:19:02 PM »
Danne said:
Quote
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

  • Contributor
  • Hero Member
  • *****
  • Posts: 6271
Re: CMOS/ADTG/Digic register investigation on ISO
« Reply #1187 on: December 05, 2019, 07:23:36 PM »
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

  • Senior
  • ****
  • Posts: 439
Re: CMOS/ADTG/Digic register investigation on ISO
« Reply #1188 on: December 05, 2019, 07:31:43 PM »
Thank-you gents for the ever-so-slight rap across the knuckles!  :) I shall persevere.

@names_are_hard C-advise would be welcome. Thanks! :)
5D3 1.1.3
5D2 2.1.2

timbytheriver

  • Senior
  • ****
  • Posts: 439
Re: CMOS/ADTG/Digic register investigation on ISO
« Reply #1189 on: December 06, 2019, 11:40:49 AM »
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?  :o

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.
5D3 1.1.3
5D2 2.1.2

Danne

  • Contributor
  • Hero Member
  • *****
  • Posts: 6271
Re: CMOS/ADTG/Digic register investigation on ISO
« Reply #1190 on: December 06, 2019, 12:30:36 PM »
You need to enable a preset from crop rec of course.

timbytheriver

  • Senior
  • ****
  • Posts: 439
Re: CMOS/ADTG/Digic register investigation on ISO
« Reply #1191 on: December 06, 2019, 12:36:36 PM »
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?
5D3 1.1.3
5D2 2.1.2

Danne

  • Contributor
  • Hero Member
  • *****
  • Posts: 6271
Re: CMOS/ADTG/Digic register investigation on ISO
« Reply #1192 on: December 06, 2019, 02:29:32 PM »
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

  • Senior
  • ****
  • Posts: 439
Re: CMOS/ADTG/Digic register investigation on ISO
« Reply #1193 on: December 06, 2019, 03:17:18 PM »
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.
5D3 1.1.3
5D2 2.1.2

histor

  • Freshman
  • **
  • Posts: 57
Re: CMOS/ADTG/Digic register investigation on ISO
« Reply #1194 on: December 06, 2019, 08:27:57 PM »
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]
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

  • Senior
  • ****
  • Posts: 439
Re: CMOS/ADTG/Digic register investigation on ISO
« Reply #1195 on: December 10, 2019, 11:06:00 AM »
Thanks @histor for sharing the encouraging test results! Every little advancement helps. :D

5D3 1.1.3
5D2 2.1.2

timbytheriver

  • Senior
  • ****
  • Posts: 439
Re: CMOS/ADTG/Digic register investigation on ISO
« Reply #1196 on: December 10, 2019, 11:47:23 AM »
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)
5D3 1.1.3
5D2 2.1.2

names_are_hard

  • Contributor
  • Member
  • *****
  • Posts: 153
  • 200D idiot
Re: CMOS/ADTG/Digic register investigation on ISO
« Reply #1197 on: December 11, 2019, 12:21:30 AM »
Congrats on getting it working, Tim!  Good persistance! :)

Danne

  • Contributor
  • Hero Member
  • *****
  • Posts: 6271
Re: CMOS/ADTG/Digic register investigation on ISO
« Reply #1198 on: December 11, 2019, 06:55:35 AM »
Nice @timbytheriver  :D.
Trying to get into the source code but seems your repo is password protected. Could you include your changes in your code please.

timbytheriver

  • Senior
  • ****
  • Posts: 439
Re: CMOS/ADTG/Digic register investigation on ISO
« Reply #1199 on: December 11, 2019, 09:44:19 AM »
Thanks.  :)

Quote
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
5D3 1.1.3
5D2 2.1.2