ilia3101 / MLV-App

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

Highlight reconstruction problems #90

Closed masc4ii closed 6 years ago

masc4ii commented 6 years ago

@ilia3101 : I am short before going crazy with highlight reconstruction. Last week we solved a problem with dual iso in relation to highlight reconstruction. Today I got some 7D clips (non dual iso) and here highlight reconstruction does not work - not with your algorithm and not with mine. @ all:

For the 7D clips both does not work. I tested a bit around and got the highlights reconstructed with this: (before)

if (tmp1b == processing->highest_green) //does not work for 7D clips
{...}

(after)

if (tmp1b >= processing->highest_green - 1500 && tmp1b <= processing->highest_green)
{...}

So I added a negative range around the theoretical highest green. Note: that works only for profile "Tonemapped" - in "Linear" the pink is there again! πŸ˜›

Do you have other ideas how to fix this? Are there completely different algorithms we could try out?

Another question is: what is different in clipped green values between EOS 5D2 and 7D???

EDIT: now we got another file from a 700D from theBilalFakhouri@ml-forum - here this "1500" must be a 15000!!! to reconstruct the highlights! In my 5D2 clips all green gets brown with this value. Idea: Should we add a "Highlight Reconstruction Range" slider? So the range could be adjusted for the needs...

masc4ii commented 6 years ago

Some analysis: look how huge the green bar (sky) is in waveform scope (not just a single value / small line), and where it changes when correcting the highlights (with range = 15000): bildschirmfoto 2018-06-13 um 22 33 08 bildschirmfoto 2018-06-13 um 22 33 21

bouncyball-git commented 6 years ago

You know what, I'm just curious why we are reconstructing only green. What if blue or red is clipped?

Edit: I never saw better reconstructing than ACR does. Edit2: slider idea is good :)

masc4ii commented 6 years ago

You know what, I'm just curious why we are reconstructing only green. What if blue or red is clipped?

Yes, I also thought about that. But in 99% of the cases I saw the green channel clipping. The blue one clippes e.g. for blue lights, but that does not look so ugly.

Edit: I never saw better reconstructing than ACR does.

Really? bildschirmfoto 2018-06-14 um 17 45 13

I have another idea: I limited the colors for which the reconstruction is made. So the range can be really huge (see above, e.g. 15000), but I forbid to reconstuct green values - so the trees stay green πŸ˜„ - that seems to work, if e.g. no pink flowers are in the picture...

Edit: so a kind of "aggressive" mode could be possible too.

masc4ii commented 6 years ago

Ilia wrote to the ML-Forum:

highlight reconstruction is for fixing pink highlights, it just has the requirement of a correct white level.

Does that mean, for all clips where it does not work, we just have to adjust the white level manually? Is that possible in the deeps of raw processing in MLV App?

masc4ii commented 6 years ago

In https://github.com/ilia3101/MLV-App/commit/be243f9dd6eaefdcac4afef79e4d28e9c07e87db I commited some code for an "aggressive highlight reconstruction mode". This kills all pink in the approx. 1/4 highest green. It works e.g. for the 700D MLV above or for this one: https://www.dropbox.com/s/iy95x9zhupesa4n/M13-1329_7Dpink.MLV?dl=0 Just comment out the if above and comment in the few lines I commited.

bouncyball-git commented 6 years ago

We should add white and black level sliders in raw correction part.

masc4ii commented 6 years ago

What range should they have? 0..2^14 or more because of dual iso? I hope it is possible to fit the right values... Are you able to set the values in Raw Correction module? I do the Gui and Receipt things... And I somehow need the default values (saved in the mlv) to realize the slider reset.

bouncyball-git commented 6 years ago

Yes default slider values should be levels from mlv.

for black and white we can use whole ranges as usual:

10bit 0-1023 <- uncompressed 12bit 0-4095 <- uncompressed 14bit 0-16383 <- uncompressed and lossless 16bit 0-65535 <- dual iso

bouncyball-git commented 6 years ago

Note: 16bit range is not necessary b/c we have to correct original white level 10/12/14bits. Even when clip is lossless there is only 14bits and b/w levels will be automatically rescaled by dual iso code.

masc4ii commented 6 years ago

Thanks for the info, that makes it easier again. I'll try out next if the function processingSetBlackAndWhiteLevel(...) could do the job already.

masc4ii commented 6 years ago

Please try out latest commit. The sliders are there and (hopefully) working. I found out some strange stuff: in most <14bit clips from any camera, the black level and white level is very often not correct. The problem is now, that these wrong values are loaded explicitely. That leads to a black or white picture. bildschirmfoto 2018-06-18 um 21 17 27 (This dual iso clip is black before pressing dual iso button. Look at the values - they are wrong.) @dannephoto : this is one of your 11bit dual iso clips.

masc4ii commented 6 years ago

Are the white and black values listed in the box calculated to 14bits? This could explain a lot...

dannephoto commented 6 years ago

Dual iso is toooo tricky. LetΒ΄s test regular footage first. But heeey. How cool is it with sliders for this. Awesome. Will test tomorrow or when I get some time over here.

masc4ii commented 6 years ago

Whaaaaa... I corrected it, that the MLV values are seen in 14bit, but now other files don't work - I have a 10bit file where BL=128 and WL=1013.... hmmmmmmm So what is right now?!

masc4ii commented 6 years ago

Now I got it... so have fun trying it out πŸ˜„

masc4ii commented 6 years ago

@bouncyball-git : when dual iso is applied to the clip, it seems as if dual iso algorithm is setting up white level (and maybe also black level?). This makes problems between dual iso and the new sliders:

Should I deactivate (gray out) the sliders for dual iso?

bouncyball-git commented 6 years ago

Are the white and black values listed in the box calculated to 14bits? This could explain a lot...

Nope. They ARE actually 14 bits!!! (does not matter it is dual iso or not). ALL lossless video actually IS 14bits with so called amplified or restricted white level (a1ex's trick to get imaginary 10-12 bit lossless data and get more compression out of it).

So the info panel shows correct values!!!

The lossless clip bit depth shown on info panel is a calculated imaginary depth and not actual one (which is always 14 bits) and black = 2047 and white = 5586 is absolutely correct values for restricted to 12 bit white.

bouncyball-git commented 6 years ago

So what is right now?!

Everything was right out of the box no need to recalculate anything.

10bit uncompressed 128-1013 so called 10bit lossless 2047-3817

I wanted you to understand that uncompressed 10bits and the lossless 10bits are very different things!!!

That is why when I'm scaling the values of dual iso for uncompressed data and lossless data do it in two very different ways!

dannephoto commented 6 years ago

Oh lawdy lawd. We're in deep pink shit now.

masc4ii commented 6 years ago

Everything was right out of the box no need to recalculate anything. 10bit uncompressed 128-1013 so called 10bit lossless 2047-3817 I wanted you to understand that uncompressed 10bits and the lossless 10bits are very different things!!!

Yes, I think I got that right now. I must use getMlvBitDepth() instead of what we are using in the info box. Then everything is fine and fits.

The question is now only, what do the sliders when dual iso is on - because the shown slider values are wrong (range is okay) after enabling dual iso.

bouncyball-git commented 6 years ago

I added set raw black level in the on_horizontalSliderRawBlack_valueChanged (also same for white level) because theoretically if we change raw levels we should set them in the mlv object itself.

It helped a bit for non dual iso but not for dual iso.

masc4ii commented 6 years ago

Did you have any problem for non dual iso clips? For me at least this was working πŸ˜„ Edit: where in mlv object do you set that? I don't find an entry. Edit2: do you mean in MLV header? If we do so, resetting to defaults will be broken... (I use this value for that)

bouncyball-git commented 6 years ago

Right resetting will be broken. It was just a quick hack, better use some dedicated var and do not touch the original MLV values. However this will require some changes here and there.

It seems for non dual iso this works. Changing white level by 1 and then back to original seems to have repeatable effect, not for dual iso though...

masc4ii commented 6 years ago

Yes, I also wouldn't touch the original MLV header values, to make everything always repeatable. The only place in the code where something was done with the white and black level is the processing module (for non dual iso part). For dual iso I need more time to have a look where the levels are calculated and where I can grab them to set my sliders πŸ‘» .

The bad thing will be: I can grab such values AFTER the picture is calculated... -> multithread problem. Or we leave it as it is and just block the sliders for dual iso (highlight reconstruction does not work with white level slider for dual iso anyway, but with my modified algorithm).

masc4ii commented 6 years ago

Autsch... I see @bouncyball-git : you wanted to set the levels in the raw_info structure, for a correct dng and mlv export, and for all the corrections, right? That doesn't work indeed with my current try 😞 Oh my dear... this is a lot of work to do!!!

masc4ii commented 6 years ago

@bouncyball-git : please try latest commit. All corrections + mlv + dng export should do their job. MLV header can now be changed, because I made 2 variables in mlv_object to save the original default values.

bouncyball-git commented 6 years ago

you wanted to set the levels in the raw_info structure, for a correct dng and mlv export, and for all the corrections, right?

Yes! but not only for this.

As you know MLV w/b values are grabbed and used in various places in the code (raw corrections) including dual iso processing... so if code finds and uses already changed raw levels it would be better.

bouncyball-git commented 6 years ago

MLV header can now be changed, because I made 2 variables in mlv_object to save the original default values.

Cool! Will try.