ilia3101 / MLV-App

All in one MLV processing app.
https://mlv.app/
GNU General Public License v3.0
283 stars 29 forks source link

DualIso -> black image #89

Closed masc4ii closed 6 years ago

masc4ii commented 6 years ago

From @dannephoto (in ML Forum): I tried enabling dualiso on this file but it turns completely black(eosm)? https://bitbucket.org/Dannephoto/magic-lantern/downloads/M24-0848_10frames.MLV

I can reproduce here, but have no idea. @bouncyball-git , could you please have a look?

bouncyball-git commented 6 years ago

I'l take a look

bouncyball-git commented 6 years ago

I've rewritten the code but not quite satisfied with the result (pink highlights). I guess I have to set processing object white and black levels accordingly. Theoretically this should help to fix the dual iso preview output too. Need more time.

To be continued...

dannephoto commented 6 years ago

Dualiso neverending code story :)

bouncyball-git commented 6 years ago

Yup ;)

bouncyball-git commented 6 years ago

Hey guys try last commit and share your findings :)

High quality 20bit mode should work.

I tried to trace the bug but unfortunately Dual ISO preview is still half broken (pink shades, etc). I think it is no big deal. This is the low quality preview after all and it works in all other modes.

masc4ii commented 6 years ago

A lot of discussion was here: https://github.com/ilia3101/MLV-App/commit/5e31f96baca5a8f54657c7365a6c167c7a44c8ba

masc4ii commented 6 years ago

The pink highlight problem comes from here: raw_processing.c, line 341 if (tmp1b == processing->highest_green) (make it ...>= highest_green/2 and the pink highlights + some more areas are gone), and the place where highest_green is calculated (that seems not be correct for dualIso, just for non-dualIso). We should think about, how to get this value for dualIso... :)

dannephoto commented 6 years ago

Great progress!

bouncyball-git commented 6 years ago

I'll take a look but please provide me with the sample to reproduce. On all clips I have there are no pinks at all (that is why I dug into processing black and white levels)

bouncyball-git commented 6 years ago

You are right, if tone mapping is turned off pinks are clearly showing up on low exposure values (not for every clip though)

masc4ii commented 6 years ago

Take the clips from Danne you gave me yesterday, set exposure to -2, and then the sky is pink ;)

bouncyball-git commented 6 years ago

if (tmp1b <= processing->highest_green/2) does not help here. Does not matter highligts reconstruction turned on or off.

masc4ii commented 6 years ago
bildschirmfoto

Turned on only. But you're right... hm... what else did I change yesterday?! Strange :(

masc4ii commented 6 years ago

It was >= not <=. dualiso_m02-1025_frame_1

But look the tree is brown now ;)

>= highest_green * 0.9 looks not bad 😄 But somehow we should find a correct highest_green.

dannephoto commented 6 years ago

Haha, brown

masc4ii commented 6 years ago

Played around... >= highest_green * 0.95 does still work, at >= highest_green * 0.97 it stopps working.

bouncyball-git commented 6 years ago

Eh... those pinks of course are overexposed clipped areas. Normally exposed part were also pink before I made my changes. Now we need to go deeper into highligh reconstruction ;). I wonder when these is gonna be ended :D

masc4ii commented 6 years ago

Ah okay... never had different pink areas than those. 😄

bouncyball-git commented 6 years ago

It is working for me for max *0.93.

bouncyball-git commented 6 years ago

could we just put *0.9 and test other non dual iso footage if it works?

masc4ii commented 6 years ago

The right way should be to scan the picture/clip for the highest (possible?) green. It is calculated in another function (you'll find with project search) and I think this is different for dualIso clips. Bringing the value just down a bit, creates artefacts for clips, which do not need the lower value.

bouncyball-git commented 6 years ago

However there is a strange behavior. If you lower the exposure gradually with slow pace you will see that on some values pinks are not present then lowering more they showing up and then after some more lowering they disappearing again ;) F..k! I don't like this...

bouncyball-git commented 6 years ago

What I really can not understand why present code is not working. All processing operations are done on 16bit data which already debayered no matter it was dual iso corrected or not. All present calculations should be valid for any source data.

masc4ii commented 6 years ago

Yes we always highlight-correct the processed picture and not the raw. This may happen (with correct highest_green it was nearly never visible, and with this factor 0.9x it is worse now. If you switch to Linear it is worst.) But it works for non-dualIso.

bouncyball-git commented 6 years ago

So what we're gonna do now? Find highest green separately? Hasn't it (code I mean) always tried to do so for every fed in footage?

masc4ii commented 6 years ago

I try to understand what @ilia3101 implemented. For me it looks like he precalculates a matrix for all possible input values, calculates with these matrix the highest possible output green, and this one gets corrected later. (max green was overexposed and cut at maximal input value) But maybe the highest possible matrix green (maximal input value) is higher than the real green, because the real one is moved down darker in the dualIso function (so maximal input value is never reached). Could that be? The behaviour between possbile dualIso green values and possbile normal green values should be different.

masc4ii commented 6 years ago

I got a solution, but it is slow. I search through all the actual picture for the highest green (raw_processing.c, line 308, add this):

    /* find highest green in actual picture */
    uint16_t highestGreen = 0;
    for (uint16_t * pix = img; pix < img_end; pix += 3)
    {
        uint16_t pix1 = processing->pre_calc_gamma[ LIMIT16(pm[3][pix[0]] + pm[4][pix[1]] + pm[5][pix[2]]) ];
        if( highestGreen < pix1 ) highestGreen = pix1;
    }

And now change these lines (a bit below, same file):

        /* Now highlilght reconstruction */
        if (/*processing->exposure_stops < 1.0 && highestGreen < 65535 &&*/ processing->highlight_reconstruction)
        {
            /* Check if its the highest green value possible */
            if (tmp1b == highestGreen)
            {
                pix[1] = (pix[0] + pix[2]) / 2;
            }
        }

For me that always seems to work. For you as well?

Edit: if I had the information if dualIso is enabled in processing module, I could drive my algorithm, else use Ilias faster solution.

bouncyball-git commented 6 years ago

Hehe thanx for this investigation. I'll take a look later.

Edit: where is the code when Ilia searches highest green? How he gets the value of highest green in the original code.

bouncyball-git commented 6 years ago

When MLV is loading and processing object is initialized default mode is dualiso off. So calculation as you said should be done on original striped dualiso debayered (not processed as dual iso) image and green is different for sure.

masc4ii commented 6 years ago

Ilia does it in processing.c; processing_update_highest_green(). But I think this is a theoretical method, because it is calculated without the clip, just using the slider settings (which are precalculated in a matrix which will be applied on the clip). It calculates the value of the (nearly) clipped green.

masc4ii commented 6 years ago

The highest_green I calculate on picture data is different in every frame and every clip... it seems to be dynamic what dualIso function calculates!

bouncyball-git commented 6 years ago

I think your actual green value calculator will give different result for every frame and every clip in case of non dualiso too.

bouncyball-git commented 6 years ago

What alex's dual iso processing actually does is: upsample 14bit data to 20bits then combine even and odd lines according to alex's algorithm (some kind of interpolation is done) and downsample again the data to true 16bit raw values.

Then we debayer this 16bit raw to 16bit per channel RGB and after that processing begins. In any case (14 bit or dual iso 16bit raw) the RGB data max is the 65535 per channel.

That is why I can not understand why there is the difference between dualiso RGB and non dual iso RGB data?!

masc4ii commented 6 years ago

I think your actual green value calculator will give different result for every frame and every clip in case of non dualiso too.

Yes, that is why I was asking about how to get if dualIso is on in processing module, to drive Ilias algorithm for non-dualIso.

I think our problem is in alex's algorithm: I think he will search somehow the area where the lightness curves are overlapping (or however he combines low and high Iso). What I saw in my tests: in every frame the highest green is different, +-5 for the frames I tested (while for non-dualIso each frame was identical). That is not much, but enough to let not work a algorithm which works with a static highest green. I took one of the highest green values I found and implemented it as static for a quick test: the corresponding frame was good, all other frames had the pink area (as expected). But these different highest green values bring me to the result, that alex is doing something dynamic in his algorithm, otherwise it would always be the same highest green (because of clipping).

14bit -> 16bit must have another contrast than 14bit+14bit (with some overlapping) to 20bit to 16bit. The second one must have less contrast (more dynamic), otherwise you can't bring all color values to 16bit. So I think the second one won't reach practically the 65535 - it should be less.

bouncyball-git commented 6 years ago

But these different highest green values bring me to the result, that alex is doing something dynamic in his algorithm, otherwise it would always be the same highest green (because of clipping).

You are right. It seems to be so.

bouncyball-git commented 6 years ago

The dualiso footage sometimes flickers from frame to frame... that is the simple proof of your hypothesis.

Edit: it's simply not adapted to image sequences, alex just meant this processing to be for the single photo image.

bouncyball-git commented 6 years ago

Anyway I never thought the dual iso were good for video ;) And now this all 10/12bit and lossless dualiso stuff drives me crazy LOL.

We should adapt your highest green calc to the present code. How exactly can I help you?

dannephoto commented 6 years ago

Flicker is due to white balance figures right?

masc4ii commented 6 years ago

How exactly can I help you?

I need a pipeline from llrawprocObject_t->dual_iso into processing module, function apply_processing_object, maybe via processingObject_t (this solution is not really good, information is doubled, or we create a pointer here, which points to llrawprocObject_t->dual_iso variable) or maybe you have a better idea... it would be nice to have all seperated and clean, so this is not easy here.

masc4ii commented 6 years ago

I think I got it... will commit.

bouncyball-git commented 6 years ago

Flicker is due to white balance figures right?

I guess so.

masc4ii commented 6 years ago

Please try https://github.com/ilia3101/MLV-App/commit/45ee8c49743aba778a0fe530dfc4b9f50b06ff19

bouncyball-git commented 6 years ago

Fantastic job man!!!

I know you do not like doing things like this ( linking pointers etc ;) ), but this is the simplest working way it could be done ever. Thank you :)

Edit: the dualiso processing is so slow already that this additional green searching for every frame is not noticeable at all ;)

dannephoto commented 6 years ago

If you apply whote balance calculations straight as is they will vary and cause flicker from time to time. Solution is to set the same as the first to all files.

Green issue solved masc? If so, great work!

bouncyball-git commented 6 years ago

@dannephoto : I guess I was wrong in the prev message to you.

White balance is not varying troughout the frames in the same clip. The flickering should be caused with the dynamic processing nature of dualiso @masc4ii mentioned above.

masc4ii commented 6 years ago

F***, it does not work if AMaZE debayer was selected... it works only for bilinear. WHY?????

bouncyball-git commented 6 years ago

Right :(

bouncyball-git commented 6 years ago

Should we ask for help @ilia3101?

masc4ii commented 6 years ago

Yeah... why not... Is it possible that the low ISO somehow affects the clipped high ISO values?! With our old good cheat factor 0.96 it works for me 😝 (but some leaves get brown)

dannephoto commented 6 years ago

"The shitty brown leave effect" 😁