Beep6581 / RawTherapee

A powerful cross-platform raw photo processing program
https://rawtherapee.com
GNU General Public License v3.0
2.81k stars 318 forks source link

DCP profile size inflated with redundant data #6467

Open Entropy512 opened 2 years ago

Entropy512 commented 2 years ago

In the process of making a DCP profile for my Sony A7M4 (will be uploading the shots and creating a separate issue for that later this week), I realized that every single dual-illuminant DCP profile likely has identical tone curve and LookTable settings (since the LookTable is tied to the tone curve, and all of the profiles were generated from the same tone curve). I plan on confirming this assumption later this week. DCP profiles without tone curve and NTRO are only 65k, vs 1MB per file.

This is really redundant, especially since RT defaults to not using the tone curve in the file, and using its own tone curve.

The LookTables are used by the "Standard Film Curve" profiles, however I think that this approach is not a good idea: If the user changes the tone curve settings, the LookTable will no longer match the tone curve, but will still be turned on.

It appears that the idea is to get the behavior of the Perceptual tone curve mode without the performance hit, as (from a rough initial check of the source), Perceptual appears to be similar to if not identical to dcamprof's default NTRO configuration.

@heckflosse what are your thoughts on trying to speed up Perceptual in the same way as the current "precalculated LookTable" setup, by recalculating an HSV LUT when the tone curve is changed?

This would allow skipping the tone curve and LookTable in any future DCP profiles.

Entropy512 commented 2 years ago

It looks like all profiles copied from ART are ones with no tone curve and no LookTable, so @agriggio appears to already be going this route.

Thanatomanic commented 2 years ago

For reference, you need the -L flag: dcamprof.exe make-dcp -n "camera id string" -d "camera id string" -c "RawTherapee CC0" -L tungsten.json daylight.json "camera id string.dcp"

I think this is a pretty reasonable idea btw, especially considering that the Auto-Matched Tone Curve usually does a pretty good job to get a decent tone curve going by default.

Entropy512 commented 2 years ago

-L (matrix-only) is not what we want - there are two HueSatMap tables in the profiles that contain a tone curve: A 2.5D HueSatMap LUT that is exposure-independent and only performs color corrections, applied after the matrix but before the tone curve. -L disables this, we do not want to disable this

A 3D HueSatMap LUT that is exposure-dependent and is tied to the tone curve. It's my opinion that we do not want this, since it will no longer be properly applicable the moment someone changes the tone curve. It appears to be intended to perform the same function as the "Perceptual" tone curve option, but faster since it is LUT-based. Passing "-t linear" will disable this.

I think it should be possible to dynamically generate a HueSatMap LUT on the fly whenever the tone curve changes, getting the performance benefits of the current approach when the tone curve is appropriate, but without the profile data being dependent on a tone curve that can change. A 90x30x30 3D HueSatMap LUT is 81,000 points and would only be recalculated when the tone curve changes, vs tens of millions of pixels each having to have the color adjustment calculated.

Thanatomanic commented 2 years ago

Hmm, passing -t linear still resulted in files of ~1 MB when I just made a few dcp's for https://github.com/Beep6581/RawTherapee/pull/6553

Not sure what I am doing wrong then, but certainly my PR needs to be updated.

Thanatomanic commented 2 years ago

image

results in

image

Entropy512 commented 2 years ago

I remember getting things to work as desired did require some fiddling.

dcamprof says "-o <neutral | standard | custom.json>, tone reproduction operator (default: neutral). Will only be applied if a non-linear curve is applied (-t parameter)." - but it may be that the comment is wrong and "-o standard" is also needed for a linear curve, and/or the option ordering mattered.

I'll see if I can reproduce on those K-70 shots someone provided, very slight possibility of tonight, more likely tomorrow night unless we can reschedule our Sunday flight earlier in the morning. I'll be spending most of tonight packing for vacation, but if we don't reschedule our flight, Sunday should be quiet.

Thanatomanic commented 2 years ago

Ah, it seems -t linear -o standard is the way that leads to 64k files. The ways of DCamProf are sometimes a bit confusing... I'll update the DCPs.

Thanks for the quick reply!

Entropy512 commented 2 years ago

Yeah, looks like the dcamprof documentation needs a minor update for -o - it implies -t linear should alone force -o standard, but it does not. I assume you meant -t linear -o standard?

-t none might force -o standard without being explicit?

Thanatomanic commented 2 years ago

Yes, linear, apologies.. :-)

Entropy512 commented 2 years ago

I now cannot remember if I did any detailed experiments between -t linear and -t none. For the most part this shouldn't matter with the current default behaviors in RT though.