PR #25307 opened by Marti2203 URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25307 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25307.patch
`mobius()` in vf_tonemap.c computes the curve as `(b * b + 2.0f * b * j + j * j) / (b - a) * (in + a) / (in + b)`. When the transition point `j` (the `param` option) approaches 1, `a` and `b` both approach -1, so `b - a` and `(b + j)^2` are differences of nearly equal numbers and the result loses all precision. At `param=1`, which the option range allows and which the documentation describes as making mobius behave like clip, it is exactly 0/0, so every value above 1.0 becomes NaN, and the conversion to an integer format then turns highlights black instead of white: ``` # 2x2 gbrpf32le frame with (r,g,b) = (2.0, 0.5, 1.5) ffmpeg -f rawvideo -pix_fmt gbrpf32le -s 2x2 -i in.raw -vf tonemap=mobius:param=1:peak=4:desat=0,format=gbrp -f rawvideo - ``` Output for one pixel on master, as (g, b, r): ``` param=0.99 float (0.249975, 0.749926, 0.999901) 8-bit (64, 192, 255) param=0.999 float (0.235294, 0.705882, 0.941176) 8-bit (60, 181, 241) param=0.9999 float (inf, inf, inf) 8-bit (255, 255, 255) param=1 float (nan, nan, nan) 8-bit (0, 0, 0) ``` So it is not only `param=1`: `0.9999` gives inf, and at `0.999` the curve already drops below `j` (0.94 for an input of 2.0, where it should be close to 1). The patch keeps the same curve but computes it in a form that does not cancel: `a + j = j (1 - j) (peak - j) / (j^2 - 2j + peak)`, `b + j = (1 - j) (peak - j) / (peak - 1)`, and the scale `(b + j)^2 / (b - a)` simplifies to `(j^2 - 2j + peak) / (peak - 1)`. For `j == 1` this gives `a = b = -1` and a scale of 1, i.e. a clip at 1.0, which is the limit of the curve. With the patch the same pixel is (0.249984, 0.749951, 0.999935) at 0.99, (0.25, 0.75, 0.999999) at 0.999 and (0.25, 0.75, 1.0) at 0.9999 and 1. For other values of `param` the output only changes by float rounding. I compared the old and new expressions against a long double evaluation of the curve over random `j`, `peak` and inputs: around `j` = 0.9 the old code is within 1.4e-5 and the new within 1.6e-6; around 0.99 the old error grows to 1.3e-3 and the new is 6e-6; for `j` within 1e-4 of 1 the new code stays within about 1e-3 (the curve only spans [j, 1] there) where the old one returns inf or NaN. The OpenCL kernel in libavfilter/opencl/tonemap.cl has the same formula and gets the same change. I could only check it with a float-only C copy of the kernel, not on an OpenCL device; in that copy the old code already returns non-finite values at `j` = 0.999, and the new one stays within 5e-4 of the reference up to `j` = 1. The second commit adds `fate-filter-tonemap-mobius-param1`: testsrc2 converted to gbrpf32, scaled by 4 with the exposure filter so part of the frame is above 1.0, tone mapped with `mobius:param=1` and converted back to gbrp. The pixels that end up at 1.0 or that pass through unchanged are exact, so the output is not sensitive to float details. It fails on master and passes with the fix (THREADS=1 and 4, gcc and clang builds); `make fate-vfilter` (503 tests without samples) passes. PR #25097 refactors this function to compute the mobius constants once per frame. This patch only touches the lines that compute `a`, `b` and the scale, so it should be easy to rebase on top of it (or the other way round); in that version the scale would just become `mobius_k`. Note that `peak` close to 1 still behaves poorly (the code clamps `peak - 1` to 1e-6), but that is a separate and much rarer case. This issue was found by an LLM-Enhanced Static Analysis Engine and verified manually. 🤖 Generated with [Claude Code](https://claude.com/claude-code) From f43dbd387765aa7da2dbd2f69c6abc49f1729b6e Mon Sep 17 00:00:00 2001 From: Martin Mirchev <[email protected]> Date: Wed, 21 Oct 2026 19:03:52 +0800 Subject: [PATCH 1/2] avfilter/vf_tonemap: fix mobius for param close to 1 The mobius curve computed b - a and (b + j)^2 as differences of nearly equal terms. Both go to 0 when the transition point j approaches 1, so the result loses all precision: with param=1, a = b = -1 and every value above 1.0 becomes 0/0 = NaN, which the conversion to an integer format turns into black instead of white. param=0.9999 gives inf, and already at param=0.999 the curve drops below j. The documentation says that values near 1.0 make mobius behave like clip, and the option range allows 1.0. Compute a + j, b + j and the scale (b + j)^2 / (b - a) = (j^2 - 2j + peak) / (peak - 1) in closed form instead, which is the same curve but well-conditioned; for j == 1 it reduces to a clip at 1.0. Apply the same change to the OpenCL kernel, which has the same formula. Signed-off-by: Martin Mirchev <[email protected]> --- libavfilter/opencl/tonemap.cl | 8 +++++--- libavfilter/vf_tonemap.c | 8 +++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/libavfilter/opencl/tonemap.cl b/libavfilter/opencl/tonemap.cl index 9448ba4552e..3a65d14a648 100644 --- a/libavfilter/opencl/tonemap.cl +++ b/libavfilter/opencl/tonemap.cl @@ -70,10 +70,12 @@ float mobius(float s, float peak) { if (s <= j) return s; - a = -j * j * (peak - 1.0f) / (j * j - 2.0f * j + peak); - b = (j * j - 2.0f * j * peak + peak) / max(peak - 1.0f, 1e-6f); + // written so that it does not cancel out for j close to 1, + // see mobius() in vf_tonemap.c + a = j * ((1.0f - j) * (peak - j) / (j * j - 2.0f * j + peak) - 1.0f); + b = (1.0f - j) * (peak - j) / max(peak - 1.0f, 1e-6f) - j; - return (b * b + 2.0f * b * j + j * j) / (b - a) * (s + a) / (s + b); + return (j * j - 2.0f * j + peak) / max(peak - 1.0f, 1e-6f) * (s + a) / (s + b); } // detect peak/average signal of a frame, the algorithm was ported from: diff --git a/libavfilter/vf_tonemap.c b/libavfilter/vf_tonemap.c index c5be4b15102..53828c1861f 100644 --- a/libavfilter/vf_tonemap.c +++ b/libavfilter/vf_tonemap.c @@ -100,10 +100,12 @@ static float mobius(float in, float j, double peak) if (in <= j) return in; - a = -j * j * (peak - 1.0f) / (j * j - 2.0f * j + peak); - b = (j * j - 2.0f * j * peak + peak) / FFMAX(peak - 1.0f, 1e-6); + /* a + j, b + j and the scale (b + j)^2 / (b - a) are written in a form + * that does not cancel out for j close to 1; j == 1 then is a clip. */ + a = j * ((1.0f - j) * (peak - j) / (j * j - 2.0f * j + peak) - 1.0f); + b = (1.0f - j) * (peak - j) / FFMAX(peak - 1.0f, 1e-6) - j; - return (b * b + 2.0f * b * j + j * j) / (b - a) * (in + a) / (in + b); + return (j * j - 2.0f * j + peak) / FFMAX(peak - 1.0f, 1e-6) * (in + a) / (in + b); } #define MIX(x,y,a) (x) * (1 - (a)) + (y) * (a) -- 2.54.0 From 0bde4ef33f550d992dfa9c184312b54d1df2b245 Mon Sep 17 00:00:00 2001 From: Martin Mirchev <[email protected]> Date: Wed, 21 Oct 2026 19:05:01 +0800 Subject: [PATCH 2/2] tests/fate/filter-video: add a test for tonemap mobius with param=1 Scale testsrc2 up by 4 with the exposure filter so that part of the frame is above 1.0, and tone map it with mobius at param=1, where the result must be a clip at 1.0. Before the previous commit these pixels came out as NaN, i.e. black after the conversion to gbrp. Signed-off-by: Martin Mirchev <[email protected]> --- tests/fate/filter-video.mak | 3 +++ tests/ref/fate/filter-tonemap-mobius-param1 | 6 ++++++ 2 files changed, 9 insertions(+) create mode 100644 tests/ref/fate/filter-tonemap-mobius-param1 diff --git a/tests/fate/filter-video.mak b/tests/fate/filter-video.mak index 4a065ec0320..73666ebc110 100644 --- a/tests/fate/filter-video.mak +++ b/tests/fate/filter-video.mak @@ -232,6 +232,9 @@ fate-filter-gainmap-luma: CMD = framecrc -lavfi "$(FATE_FILTER_GAINMAP_SRC)gainm fate-filter-gainmap-maxrgb: CMD = framecrc -lavfi "$(FATE_FILTER_GAINMAP_SRC)gainmap=mode=maxrgb,scale" -pix_fmt gray fate-filter-gainmap-fixed: CMD = framecrc -lavfi "$(FATE_FILTER_GAINMAP_SRC)gainmap=min=0:max=2.3:gamma=1:base_nits=203:alt_nits=1000,scale" -pix_fmt gbrp +FATE_FILTER-$(call FILTERFRAMECRC, TESTSRC2 FORMAT EXPOSURE TONEMAP, SCALE_FILTER) += fate-filter-tonemap-mobius-param1 +fate-filter-tonemap-mobius-param1: CMD = framecrc -lavfi testsrc2=s=64x48:r=1:d=1,scale,format=gbrpf32,exposure=exposure=2,tonemap=mobius:param=1:peak=4:desat=0,scale -pix_fmt gbrp + FATE_FILTER_VSYNTH_PGMYUV-$(CONFIG_GRADFUN_FILTER) += fate-filter-gradfun fate-filter-gradfun: CMD = framecrc -c:v pgmyuv -i $(SRC) -vf gradfun diff --git a/tests/ref/fate/filter-tonemap-mobius-param1 b/tests/ref/fate/filter-tonemap-mobius-param1 new file mode 100644 index 00000000000..ad52a513119 --- /dev/null +++ b/tests/ref/fate/filter-tonemap-mobius-param1 @@ -0,0 +1,6 @@ +#tb 0: 1/1 +#media_type 0: video +#codec_id 0: rawvideo +#dimensions 0: 64x48 +#sar 0: 1/1 +0, 0, 0, 1, 9216, 0x6902e177 -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
