mpv-player / mpv

🎥 Command line video player
https://mpv.io
Other
27.69k stars 2.86k forks source link

Tone mapping: improve handling of very bright scenes/movies #9800

Closed FoLLgoTT closed 1 year ago

FoLLgoTT commented 2 years ago

The masterings of different HDR movies vary a lot in terms of average brightness (not peak!). There are movies or scenes that have a significant amount of content >100 nits. These scenes are displayed too bright with the tone mapping algorithms of MPV when using a lower target-peak (e.g. 150) which usually fits well to most movies.

madVR has an option to handle that which is called "dynamic target nits". From my understanding this controls the strength for increasing the point where to start the tone mapping depending on the average brightness of the picture. In other words: it dynamically shifts the "reference white" to higher nits. Such an option seems to be missing in MPV.

I tried different tone mapping algorithms, but the problem seems to be in all of them. I usually use BT.2339, but Reinhard or BT.2446a have the same problem.

Examples of movies with high average brightness:

Example images ("Meg", chapter 7)

Histogram: histogram

MPV with target-peak=auto (resulting in tone mapping to 203 nits): clipping

madVR (target nits = 100, dynamic target nits = 30, resulting in tone mapping to 1224 nits): madvr

MPV with target-peak=1220: mpv

Expected behavior of the wanted feature

Better adaption on high average brightness.

Log file

log.txt

dyphire commented 2 years ago

It will be great.

quietvoid commented 2 years ago

You should try the gpu-next VO. See https://github.com/mpv-player/mpv/issues/9788

FoLLgoTT commented 2 years ago

You should try the gpu-next VO. See #9788

I already use gpu-next in my config. The problem is the same.

quietvoid commented 2 years ago

Right, I hadn't looked at the log, sorry. This is mostly a limitation of bt.2390's linear section, which madVR has modified.

I can't reproduce the clipping with bt.2446a and spline (param 0.15).

FoLLgoTT commented 2 years ago

I can't reproduce the clipping with bt.2446a and spline (param 0.15).

Yes, bt.2446a with target-peak=auto darkens those ultra bright scenes better. But the picture has less contrast then bt.2339 or madVR. It looks flat and not "natural". This is the reason why I don't use it.

haasn commented 2 years ago

Isn't this literally what --hdr-compute-peak does?

Incidentally, in the past, we used to also measure the frame average brightness and darken the scene even further if needed, but that functionality got scrapped during the gpu-next tone-mapping refactor.

FoLLgoTT commented 2 years ago

Isn't this literally what --hdr-compute-peak does?

I always use this option, but it doesn't make any noticable difference in those bright scenes.

Incidentally, in the past, we used to also measure the frame average brightness and darken the scene even further if needed, but that functionality got scrapped during the gpu-next tone-mapping refactor.

Do you want to bringt back this behaviour? Maybe with an additional parameter which controls the strength?

At the moment I have to use meta data to set the target-peak for those ultra bright movies in a LUA script. And that doesn't count for all problems, since a static target-peak will darken dark scenes unnecessarily. An automatic darkening by analyzing average brightness would be very cool.

FoLLgoTT commented 2 years ago

You cannot be serious right now. Even though HDR is available you do not use it, instead you tonemap ST2084 transfer to SRGB transfer.

Of course I am. In a home theater with a projector it is common to choose the best tone mapping algorithm you can get. And this is not always implemented in the projector, but in an external processor or HTPC. In most cases it is done by madVR (or Envy/Radiance). MPV is a really great player with lots of killer features. Only the tone mapping is not on par with madVR yet. It is good, but not the best you can get.

With a large screen the picture is rather dim in comparison to a LCD TV. On a 4 m wide screen I get only 65 Nits (which is pretty good for that size). So the quality of the tone mapping is essential.

PS: the log was from my test pc. Of course I don't use only BT.709 in my home theater.

FoLLgoTT commented 2 years ago

You do not "tonemap" here, you convert to SDR here, even though HDR is available due to a bug in gpu-next. Use

mpv.com --target-trc=pq --target-prim=bt.2020 --vo=gpu-next --gpu-context=d3d11 jiwqw.mp4

You can verify that gpu-next HDR PQ swapchain works by checking insanely wrong colors with

mpv.com --vo=gpu-next --gpu-context=d3d11 jiwqw.mp4

I don't understand what exactly you mean. What is the bug in gpu-next? I don't have this specific file, but on my files nothing looks insanely wrong. Can you explan it? I see only a slight oversaturation with gpu-next in comparison with gpu.

And what is the difference between tone mapping and SDR conversion? In fact all owners of home theaters with a projecter watch only downconverted SDR. 1.000 or 10.000 Nits are not possible. In most times not event 200 Nits. So everything we watch is in fact SDR (SDR never had an absolute standard with 100 Nits peak at home).

This is not a valid assumption:

No. Projectors use what is called very dark room with 2.6 gamma for SDR, so everything above 48 nits is HDR. And what would be good is 108 nits reference HDR master of Dolby Cinema with Dolby Vision. Of course that still needs be matched to whater reference monitor it was mastered upon.

You are talking about digital cinema (DCI). This is a completely different standard. Cinema doen't play UHD Blu-rays or stream from Netflix. DCI standard is not our goal in home theater. The goal is SDR with BT.2020 (or native projector) primaries using gamma 2.2 or similar EOTF's and the peak white of our projection.

MadVR removed support for BT.2390 long time ago and use histogram approach.

Yes, because everyone noticed that the strict scientific approach doesn't work satisfactorily. But good to read that SDR conversion will be improved even further in MPV. :-)

I just try to help improving it. I think MPV has great potential in the home theater community and I really hope that we get a strong madVR competitor one day in terms of SDR conversion.

FoLLgoTT commented 2 years ago

Would you consider it a bug that PQ surface is selected in d3d11 context, but not in vulkan for gpu-next? I do. Would you consider it a bug that a surface that 100% defined the colorspace and primaries does not get automatic target-trc and target-prim treatment, that does happen in vo gpu, but not in gpu-next? I do.

Yes, me too. There are several small bugs that keep me from using gpu-next at the moment. When they are fixed I hope the result will be better than with gpu.

There are HDR10 projectors

That's right. But all these projectors have (heavy) tone mapping implemented, because otherwise the picture would not be watchable. The physical properties of the panel and the light source are limited to SDR. This counts for all consumer projectors. So in fact we are never watching HDR (high contrast and high peak luminance) with a projector. Not to speak from even lower ADL contrast. It doesn't matter if the tone mapping is performed outside or inside the projector.

FoLLgoTT commented 2 years ago

In the meantime I watched a few more movies and I can still confirm that very bright scenes are a problem for MPV. "Black Widow" is a good example for a recent movie with a large portion of the picture placed in highlights. I was not able to get a good picture along the whole movie. Most scenes look good with BT.2390, but not the bright ones (e.g. the snow scene around 01:00:00). These are clipped.

gpu-next changes a bit here, but BT.2446 doesn't look good at all to my eyes (less contrasty). And with gpu-next the saturation is a bit too high, making the picture to look unnaturally. That counts for all tone mapping algorithms.

I think we really need histogram based tone mapping like in madVR or any other solution to handle those ultra bright scenes.

FoLLgoTT commented 1 year ago

I don't want to be impolite, but are there any news regarding tone mapping and these problematic scenes? :-)

For me (and others) it is a show stopper of using MPV. This is sad, because it is such a great player and I really want to use it. But several movies have such scenes and madVR still handle them much better. They just don't "bleed out" there.

haasn commented 1 year ago

I don't want to be impolite, but are there any news regarding tone mapping and these problematic scenes? :-)

Nope :( Better tone-mapping is unfortunately not currently at the top of my priority list..

pillepalle1234 commented 1 year ago

That is sad... ;(

It is the only thing holding me back switching from MadVR and i know lots of other folks who would happily join the MPV community if tonemapping was better.

haasn commented 1 year ago

I wrote code to measure a frame histogram in libplacebo, does anybody have a clue what madVR actually does with this information?

Edit: Seems like the common thing to do is to generate a LUT by reversing the intensities to spread the pixels evenly throughout the display range.

FoLLgoTT commented 1 year ago

I don't know exactly what madVR is doing internally. But there is a thread where several things were discussed. In the beginning the dynamic tone mapping was even based on a measurement file. Maybe there are some information that help to understand the concept behind madVR's tone mapping.

https://www.avsforum.com/threads/improving-madvr-hdr-to-sdr-mapping-for-projector-no-support-questions.2954506/

natural-harmonia-gropius commented 1 year ago

Some of my opinions:

The whole tone-mapping process should be carried out according to this, the order cannot be changed. image Then, do bt2020 to bt709 gamut-mapping

Tone-mapping is more important to bring a sense of layering than to produce an underexposed image. for me the madvr always tries to darken the picture (as 0.5), which is not a good thing, we need a bright feeling. The brightness of 4 feels more natural than 0.5, doesn't it? exposure 0.5 exposure 1 exposure 4
image image image

In my implementation, crosstalk does not do the highlight desaturation. chroma correction do that. See https://github.com/Natural-Harmonia-Gropius/mpv_config/tree/next/portable_config/shaders/hdr-toys

lut doesn't fix hue-skew/hue-shift. I don't know how madvr works, but in gpu-next with my workflow gamut-mapping lut be applied first, then tone-mapping shaders. See the screenshot in https://github.com/Natural-Harmonia-Gropius/mpv_config/issues/8. Looks like it's fixed, but the colors are completely wrong. As written above, the order cannot be changed.

In my practice, the highlights part of xyY.Y is higher but flatter than relative luminance. We should probably improve the luma and then drop the hybrid and sat, these two are too weird.

Let's get back to the topic of how to make the highlights look better. image Take this curve as an example. If the value in the histogram exceeds Y_sdr by more, then ip should decreases accordingly and vice versa, k3 decreases with the height of the excess and vice versa. If the histogram represents nothing out of the sdr range, then the curve will become the same as clip

ACES tone mapping issue can be closed. https://code.videolan.org/videolan/libplacebo/-/issues/180 I think it's more suited to being used for color grading than plackback. most of videos are already graded, unlike cameras or game engines where lighting is physically based. It does gamut-mapping, highlight desaturation, apply a high contrast curve, and gamut-mapping-inv, No surprises, and also has hue-skew issues.

And ICtCp tone-mapping. I haven't looked it in depth, but I think JzAzBz, the one also based on LMS, is better suited for this. https://code.videolan.org/videolan/libplacebo/-/issues/198

About Histogram-based tone mapping. The easiest way is to adjust the linear exposure of the median of Y_hdr into the range where the curve maps well. as 3 pictures above.

pillepalle1234 commented 1 year ago

It is kind of strange that development is split up over two information sources:

Here is the relevant post from Haasn in the libplacebo "git": https://code.videolan.org/videolan/libplacebo/-/issues/236

mdannp commented 1 year ago

looking forward to this :)

quietvoid commented 1 year ago

Some new tone mapping functions (st2094-40, st2094-10) landed in master, they should be a pretty big improvement if the video has HDR10+ or Dolby Vision metadata (Dolby Vision needs libplacebo with libdovi for now).

Eventually they'll be useful with peak detection as well, but it's not ready yet.

dyphire commented 1 year ago

Some new tone mapping functions (st2094-40, st2094-10) landed in master, they should be a pretty big improvement if the video has HDR10+ or Dolby Vision metadata (Dolby Vision needs libplacebo with libdovi for now).

Eventually they'll be useful with peak detection as well, but it's not ready yet.

Why do you think tone mapping of st2094 a pretty big improvement? In my test on Dolby Vision profile5, the result is obviously not so.

mpv version: https://github.com/shinchiro/mpv-winbuild-cmake/releases/tag/20230221 test video: http://www.makemkv.com/download/dvtest/P81_GlassBlowing2_3840x2160@59_94fps_15200kbps.mkv bt.2390 bt.2446a st2094-10 st2094-40
image

You can clearly see that its picture of st2094 is overexposed and details are lost, even worse than bt.2390.

quietvoid commented 1 year ago

bt.2446a is especially aggressive at dimming highlights more, this is somewhat expected. With more regular content graded under 1000 nits, it is sometimes way too dark.

For now my recommendations are:

For the typical SDR display (1000:1 contrast), just avoid st2094-10. The new methods should also handle high average brightness better as they use the measured frame average brightness, so this would improve the original issue's example.

Also, I don't quite consider this detail loss but that's subjective. There's just no obvious clipping.

dyphire commented 1 year ago

Without the metadata, it is suboptimal at retaining highlight detail.

Not only suboptimal, but the tone mapping effect is very poor without the metadata. test video: https://www.youtube.com/watch?v=Leii5VE1zBU

bt.2390 bt.2446a st2094-10 st2094-40

Also, I don't quite consider this detail loss but that's subjective. There's just no obvious clipping.

I don't think this level of detail loss is acceptable, even worse than bt.2390. This is exactly what the original issue wants to avoid.

The new methods should also handle high average brightness better as they use the measured frame average brightness, so this would improve the original issue's example.

I just doubt that st2094-40 can become a good choice under the new method, given that it is currently very similar to the performance of bt.2390 in bright scenes.

I think only histogram tone mapping can improve it very well.

quietvoid commented 1 year ago

Not only suboptimal, but the tone mapping effect is very poor without the metadata.

This is only because peak detection doesn't provide the brightness yet. Of course without peak detection, it'll probably look just as bad. These are methods meant to rely on dynamic brightness info.

st2094-10 is at least better than bt.2390 for high APL, but it's not really suitable for SDR tone mapping.

In the end tone mapping such bright files is about compromises, either APL or highlight detail has to be sacrificed. st2094-40 should probably be avoided if the file isn't HDR10+ for now.

To do it all, we probably need custom curves.

dyphire commented 1 year ago

These are methods meant to rely on dynamic brightness info.

That's the issue I want to say. In the above test on https://github.com/mpv-player/mpv/issues/9800#issuecomment-1438034265, even if dynamic brightness info of Dolby Vision is applied, the performance of st2094-40 in highlight details is still worse than that of bt.2390, which is disappointing.

Comparison of another group: test video: http://www.makemkv.com/download/dvtest/P5_Dolby_Amaze.mkv bt.2390 bt.2446a st2094-40

In this group of tests, the st2094-40 overexposure is more serious, and the picture has been distorted. Still didn't see the improvement brought by dynamic brightness info of Dolby Vision.

haasn commented 1 year ago

So, st2094-40 relies on signalled metadata (OOTF curves). When you are testing with only brightness info alone known (DV RPU structs or peak detection), we currently fall back to an arbitrary hard-coded OOTF. This OOTF is very bright, it was (deliberately) chosen to do so based on a desire to match reference masters of the HDR10+ sample clips I threw at it. But this was testing on a very limited dataset

It's entirely possible that one or all of the following is true:

  1. The default OOTF needs to be improved significantly.
  2. We should switch to a different default tone-mapping curve in the absence of OOTF metadata, probably in addition to:
  3. The other curves (bt.2390, spline, etc). should start taking average brightness data into account. We now have easy access to this metadata, so it's a simple process of plugging it into the curves.
  4. The st2094_pick_knee function should be improved, possibly in conjunction with the other functions learning to make use of it. It's based on pretty hacky logic right now.

As a side note, note that in branch https://code.videolan.org/videolan/libplacebo/-/merge_requests/374, I made the default st2094-40 ootf even brighter! Now would definitely be the time to make the necessary changes.

FoLLgoTT commented 1 year ago

Thank you @haasn for the additional tone mapping algorithms.

I tested st2094-10 a bit and I doesn't look bad at all. It handles bright parts better than bt2390, but it darkens the low end a bit. This looks more contrasty, but hides shadow details.

And like all other algorithms, it fails at extremely bright scenes like in my first post or in "Aquaman" starting at 01:09:00 (walking out of the sea). I think without evaluating the histogram there is no real chance to get these scenes acceptable.

Btw, there is a bug in taking screenshot with "s" with gpu-next. The colors/gamma is completely wrong. "ctrl+s" works, though.

haasn commented 1 year ago

So, st2094-40 relies on signalled metadata (OOTF curves). When you are testing with only brightness info alone known (DV RPU structs or peak detection), we currently fall back to an arbitrary hard-coded OOTF. This OOTF is very bright, it was (deliberately) chosen to do so based on a desire to match reference masters of the HDR10+ sample clips I threw at it. But this was testing on a very limited dataset

It's entirely possible that one or all of the following is true:

1. The default OOTF needs to be improved significantly.

2. We should switch to a different default tone-mapping curve in the absence of OOTF metadata, probably in addition to:

3. The other curves (bt.2390, spline, etc). should start taking average brightness data into account. We now have easy access to this metadata, so it's a simple process of plugging it into the curves.

4. The `st2094_pick_knee` function should be improved, possibly in conjunction with the other functions learning to make use of it. It's based on pretty hacky logic right now.

As a side note, note that in branch https://code.videolan.org/videolan/libplacebo/-/merge_requests/374, I made the default st2094-40 ootf even brighter! Now would definitely be the time to make the necessary changes.

I rewrote the code in that MR branch to implement 2, 3 and 4. st2094_pick_knee now has 5 new parameters to play with, and I rewrote spline to take into account the scene average brightness. I still plan on implementing a full histogram evaluation to measure the true median instead of merely the average, but this should be a big improvement over status quo.

Let me know what you think. I also made nu-spline the new default with peak detection / scene metadata.

dyphire commented 1 year ago

I rewrote the code in that MR branch to implement 2, 3 and 4. st2094_pick_knee now has 5 new parameters to play with, and I rewrote spline to take into account the scene average brightness. I still plan on implementing a full histogram evaluation to measure the true median instead of merely the average, but this should be a big improvement over status quo.

Let me know what you think. I also made nu-spline the new default with peak detection / scene metadata.

In my test, the performance of the new spline has been greatly improved with peak detection / dynamic brightness metadata. the st2094-40 has also improved.

bt.2446a st2094-40 spline
image image
bt.2446a st2094-40 spline
image image image
bt.2446a st2094-40 spline
image image image
dyphire commented 1 year ago

I rewrote the code in that MR branch to implement 2, 3 and 4. st2094_pick_knee now has 5 new parameters to play with, and I rewrote spline to take into account the scene average brightness. I still plan on implementing a full histogram evaluation to measure the true median instead of merely the average, but this should be a big improvement over status quo.

Let me know what you think. I also made nu-spline the new default with peak detection / scene metadata.

After more tests, I found that the new spline always like to clamp highlights. It does like a worse bt.2446a. The new st.2094-40 always performs well under normal peak brightness scenes with dynamic brightness metadata, but it will still be failed with very higher peak brightness scenes on Dolby Vision.

  1. The default OOTF needs to be improved significantly.

I think further realization of 1 may be a better choice and improvement. st2094_pick_knee can now handle very high peak brightness scenes with peak detection under hdr10 well. The new OOTF should also probably be tested and adjusted on Dolby Vision, because Dolby Vision has more samples and peak ranges than hdr10+.

haasn commented 1 year ago

To be clear, are you testing with or without peak detection / dynamic metadata?

dyphire commented 1 year ago

To be clear, are you testing with or without peak detection / dynamic metadata?

I use this mpv(build with libdovi: 20230223) to test on Dolby Vision, so it use dynamic metadata. I also use hdr-compute-peak to test on hdr10.

dyphire commented 1 year ago

I noticed that the new implementation of st2094_pick_knee seems fine when use peak detection in most scenarios, but in some scenarios it will be too bright and lose details. I'm curious why.

test video: Atomic_Blonde_2017_test_00-00-43.919-00-00-48.131.zip

bt.2446a st2094-40 spline
image image image

The new spline alsolose a little details in this scene, and it is always too dark in lower brightness scenes.

dyphire commented 1 year ago

I tried the following patch to libplacebo, which can fix these issues of too bright red in the new st2094_pick_knee and the new spline too dark in the lower brightness scenes for me.

---
 src/tone_mapping.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/tone_mapping.c b/src/tone_mapping.c
index 376bd2da..8269fb61 100644
--- a/src/tone_mapping.c
+++ b/src/tone_mapping.c
@@ -287,7 +287,7 @@ static void st2094_pick_knee(float *out_src_knee, float *out_dst_knee,
                              const struct pl_tone_map_params *params)
 {
     const float default_avg = 10.0f; // default avg input brightness in nits
-    const float max_slope   = 1.1f;  // maximum brightness boost on dark scenes
+    const float max_slope   = 1.0f;  // maximum brightness boost on dark scenes
     const float max_knee    = 0.8f;  // maximum relative brightness of knee point
     const float adaptation  = 0.5f;  // balance between src and dst avg
     const float target_avg  = 0.4f;  // balance between dst_min and dst_max
@@ -300,7 +300,7 @@ static void st2094_pick_knee(float *out_src_knee, float *out_dst_knee,
     // Infer the average from scene metadata if present, or default to 10 nits
     // as this is a common industry value that produces good results on average
     float src_avg = pl_hdr_rescale(PL_HDR_NITS, PL_HDR_PQ,
-                                   PL_DEF(params->hdr.scene_avg, default_avg));
+                                   fmaxf(params->hdr.scene_avg, default_avg));
     src_avg = PL_CLAMP(src_avg, src_min, max_knee * src_max);

     // Use a fixed point on the output display as the default display target,
-- 
haasn commented 1 year ago

After more testing and feedback I decided to rewrite the spline function, in particular its corresponding knee point selection logic, a second time.

That version can be found at https://code.videolan.org/videolan/libplacebo/-/merge_requests/391

haasn commented 1 year ago

@dyphire can you try the above branch?

dyphire commented 1 year ago

After more testing and feedback I decided to rewrite the spline function, in particular its corresponding knee point selection logic, a second time.

That version can be found at https://code.videolan.org/videolan/libplacebo/-/merge_requests/391

The new spline has better performance when using 1.0 parameters in my test on the MR, and the default value performs poorly in both lower brightness scenes and higher brightness scenes. spline 0.7 spline 1.0
1 2
spline 0.7 spline 1.0
3 4

In fact, this also applies to the new st2094-40 in high-brightness scenes. I suggest setting default value to 1.0 for spline at least, because when use --tone-mapping=auto, I cannot manually adjust the parameters for spline in mpv.

quietvoid commented 1 year ago

The new spline is definitely too dark regardless of the param.

st2094-10 spline
01 02
src_min = 0.000038 (0.000000), src_max: = 0.805454 (1634.922729), dst_min = 0.083327 (0.202998), dst_max = 0.580690 (203.004471)
using scene_avg = 0.175219 (1.599190)
src_avg = 0.175219 (1.599200), dst_avg = 0.191506 (2.113408)
adaptation 0.700000 -> target dst avg = 0.186620 (1.947158)
clampted dst_avg = 0.186620 (1.947158)
haasn commented 1 year ago

After more testing and feedback I decided to rewrite the spline function, in particular its corresponding knee point selection logic, a second time. That version can be found at https://code.videolan.org/videolan/libplacebo/-/merge_requests/391

The new spline has better performance when using 1.0 parameters in my test on the MR, and the default value performs poorly in both lower brightness scenes and higher brightness scenes. spline 0.7 spline 1.0 1 2 spline 0.7 spline 1.0 3 4

In fact, this also applies to the new st2094-40 in high-brightness scenes. I suggest setting default value to 1.0 for spline at least, because when use --tone-mapping=auto, I cannot manually adjust the parameters for spline in mpv.

But spline with param=1.0 is just linear, maybe you want to use that curve?

dyphire commented 1 year ago

But spline with param=1.0 is just linear, maybe you want to use that curve?

This is only for the test results in that MR. In fact, I don't like the new spline. As @quietvoid said, the new spline is too dark. I prefer the existing spline with applying the patch I mentioned above: https://github.com/mpv-player/mpv/issues/9800#issuecomment-1448456523.

haasn commented 1 year ago

@dyphire @quietvoid we can make the nu-spline a bit steeper again:

diff --git a/src/tone_mapping.c b/src/tone_mapping.c
index b7c1c317..3b51c794 100644
--- a/src/tone_mapping.c
+++ b/src/tone_mapping.c
@@ -609,8 +609,10 @@ static void spline(float *lut, const struct pl_tone_map_params *params)
     st2094_pick_knee(&src_pivot, &dst_pivot, params);

     // Tune the knee point at the slope slightly
-    const float slope = (params->output_max - params->output_min) /
-                        (params->input_max - params->input_min);
+    const float slope_gamma = 0.5f;
+    float slope = (params->output_max - params->output_min) /
+                  (params->input_max - params->input_min);
+    slope = powf(slope, slope_gamma); // make a bit steeper

     // Normalize everything the pivot to make the math easier
     const float in_min = params->input_min - src_pivot;

As slope_gamma goes to 0.0, the slope gets forced to be linear at the knee point. This basically raises the contrast of the function. We could maybe ship a lower default for this, to differentiate it a bit from linear and bt2446a (which it currently exists in the same class as, sort of)

haasn commented 1 year ago

But spline with param=1.0 is just linear, maybe you want to use that curve?

This is only for the test results in that MR. In fact, I don't like the new spline. As @quietvoid said, the new spline is too dark. I prefer the existing spline with applying the patch I mentioned above: #9800 (comment).

To clarify, are you testing with HDR10+ metadata / peak detection enabled or not?

Edit: Oops, I already asked you this. I feel silly now..

dyphire commented 1 year ago

But spline with param=1.0 is just linear, maybe you want to use that curve?

This is only for the test results in that MR. In fact, I don't like the new spline. As @quietvoid said, the new spline is too dark. I prefer the existing spline with applying the patch I mentioned above: #9800 (comment).

To clarify, are you testing with HDR10+ metadata / peak detection enabled or not?

Always use --hdr-compute-peak for testing, it should apply the peak detection.

haasn commented 1 year ago

I made some more changes in my branch, which take things into an interesting direction. The basic challenge is how to balance making the curve behave more like linear for very bright scenes while behaving more like clip for very dark ones.

I decided to put very harsh limits on the detected scene avg in order to avoid picking a knee point that's too large. In addition to this clamp, I changed the way the spline slope is calculated, and made it tunable. (Play with it, it has an interesting effect on the output!)

The sacrifice here is that the adaptation point is no longer tunable for spline, and I fear that 0.7 might be a too-low value for those very bright scenes. That said, this might be the best we're getting for the time being (no histogram, to-be-added soon).

Edit: Oops, I mistakenly had the adaptation set to 0.5 instead of the intended 0.7. Fixed.

haasn commented 1 year ago

@quietvoid you were complaining about the slope not being close enough to 1:1 for dark scenes, while at the same time, we want it to be more flat for bright scenes. This patch is an attempt at naively resolving both constraints:

diff --git a/src/tone_mapping.c b/src/tone_mapping.c
index 51005a49..c3b9fa29 100644
--- a/src/tone_mapping.c
+++ b/src/tone_mapping.c
@@ -620,11 +620,22 @@ static void spline(float *lut, const struct pl_tone_map_params *params)
     const float adaptation = 0.70f;
     st2094_pick_knee(&src_pivot, &dst_pivot, params, adaptation);

-    // Tune the slope at the knee point slightly
-    const float slope_gamma = 1.0f - params->param;
+    // Solve for linear knee (Pa = 0)
     float slope = (dst_pivot - params->output_min) /
                   (src_pivot - params->input_min);
-    slope = powf(slope, slope_gamma);
+
+    // Tune the slope at the knee point slightly: raise it to a user-provided
+    // gamma exponent, multiplied by an extra tuning coefficient designed to
+    // make the slope closer to 1.0 when the difference in peaks is low, and
+    // closer to linear when the difference between peaks is high.
+    const float slope_tuning_strength = 1.5f;
+    const float slope_tuning_offset   = 0.2f;
+    const float slope_gamma = 1.0f - params->param;
+    float ratio = params->input_max / params->output_max - 1.0f;
+    ratio = PL_CLAMP(slope_tuning_strength * ratio,
+                     slope_tuning_offset,
+                     1.0f + slope_tuning_offset);
+    slope = powf(slope, slope_gamma * ratio);

     // Normalize everything the pivot to make the math easier
     const float in_min = params->input_min - src_pivot;

How do you feel about it? My main worry is that behavior may end up too unpredictable as a result of max_peak fluctuations even where average brightness remains similar.

dyphire commented 1 year ago
Although the new spline improves the performance of low-brightness scenes in my test, it actively too dark in higher brightness scenes, look like bt.2446a. This is still not the right improvement. test video: https://www.youtube.com/watch?v=NfHC8fVnOnA spline https://github.com/haasn/libplacebo/commit/7b203c6f5a23ed76e1f4a5019b89df7dc5c6d75c spline https://github.com/haasn/libplacebo/commit/4f597287abcd24ffa9c827fafe310d41c1238535
1 2
quietvoid commented 1 year ago

That sample is 10 000 nits. It's probably necessary here to have a histogram or just an improvement in taking into account the average brightness.
The problem with frames this extreme, it's complicated to know if ignoring the real peak would end up clipping anything..

dyphire commented 1 year ago

That sample is 10 000 nits. It's probably necessary here to have a histogram or just an improvement in taking into account the average brightness. The problem with frames this extreme, it's complicated to know if ignoring the real peak would end up clipping anything..

What needs to be improved is the handling of the slope. Except for the poor performance of the dark scenes, most scenes of the original spline(https://github.com/haasn/libplacebo/commit/7b203c6f5a23ed76e1f4a5019b89df7dc5c6d75c) are better than the new spline, the above comparison chart is only one example.

It is worth mentioning that in addition to the issues in dark scenes, in my tests of various brightness scenes, its performance of original spline is very close to the histogram mapping in madvr. Similarly, it can handle the original issue's example well. So in my opinion, this is maybe the best tone mapping we have obtained before implementing histogram mapping(if fix the problem of dark scenes).

haasn commented 1 year ago

@dyphire how about this? It preserves some of both the old and the new slope calculation (adjustable by the user), and also decreases the target adaptation strength to bring it closer to the original spline.

diff --git a/src/tone_mapping.c b/src/tone_mapping.c
index c3b9fa29..4644f1c9 100644
--- a/src/tone_mapping.c
+++ b/src/tone_mapping.c
@@ -617,25 +617,27 @@ const struct pl_tone_map_function pl_tone_map_bt2446a = {
 static void spline(float *lut, const struct pl_tone_map_params *params)
 {
     float src_pivot, dst_pivot;
-    const float adaptation = 0.70f;
+    const float adaptation = 0.50f;
     st2094_pick_knee(&src_pivot, &dst_pivot, params, adaptation);

-    // Solve for linear knee (Pa = 0)
-    float slope = (dst_pivot - params->output_min) /
-                  (src_pivot - params->input_min);
-
-    // Tune the slope at the knee point slightly: raise it to a user-provided
-    // gamma exponent, multiplied by an extra tuning coefficient designed to
-    // make the slope closer to 1.0 when the difference in peaks is low, and
-    // closer to linear when the difference between peaks is high.
+    // Mix between a fully linear slope and a high-contrast slope based on the
+    // user-provided contrast parameter.
+    const float slope_linear = (dst_pivot - params->output_min) /
+                               (src_pivot - params->input_min);
+    const float slope_contrast = dst_pivot / src_pivot;
+    float slope = PL_MIX(slope_linear, slope_contrast, params->param);
+
+    // Tune the slope at the knee point slightly: raise it by an extra tuning
+    // coefficient designed to make the slope closer to 1.0 when the difference
+    // in peaks is low, and closer to linear when the difference between peaks
+    // is high.
     const float slope_tuning_strength = 1.5f;
     const float slope_tuning_offset   = 0.2f;
-    const float slope_gamma = 1.0f - params->param;
     float ratio = params->input_max / params->output_max - 1.0f;
     ratio = PL_CLAMP(slope_tuning_strength * ratio,
                      slope_tuning_offset,
                      1.0f + slope_tuning_offset);
-    slope = powf(slope, slope_gamma * ratio);
+    slope = powf(slope, ratio);

     // Normalize everything the pivot to make the math easier
     const float in_min = params->input_min - src_pivot;
@@ -672,7 +674,7 @@ const struct pl_tone_map_function pl_tone_map_spline = {
     .description = "Single-pivot polynomial spline",
     .param_desc = "Contrast",
     .param_min = 0.00f,
-    .param_def = 0.50f,
+    .param_def = 0.80f,
     .param_max = 1.50f,
     .scaling = PL_HDR_PQ,
     .map = spline,
dyphire commented 1 year ago

@dyphire how about this? It preserves some of both the old and the new slope calculation (adjustable by the user), and also decreases the target adaptation strength to bring it closer to the original spline.

I used patch to test the above example, and I can see some improvement, which is not very obvious. If I setting the parameter to 1.5(not sure this is a good choice), it seems a little close to the original spline level.

original spline spline(master) spline 0.8(patch) spline 1.5(patch)
1 2 3 4