avfilter/tonemap_videotoolbox: improve quality #416
Conversation
| + if (is_tone_mode_max) { | ||
| + sig = fmax(fmax3(*r4, *g4, *b4), FLOAT_EPS); | ||
| + } else { | ||
| + sig = fmax((*r4 * .2126f + *g4 * .7152f + *b4 * .0722f), FLOAT_EPS); |
There was a problem hiding this comment.
Shouldn't the BT.2020/BT.2100 primaries (0.2627f; 0.678f; 0.0593f) be used to calculate relative luminance? It happens before the RGB values are tonemapped. I tested it and it works as expected too.
There was a problem hiding this comment.
Probably we should. I use the sRGB primaries because that is what Apple uses in the HDR image processing sample code, but their colorspace could be different which still uses sRGB primaries but extended to HDR spaces.
| + attachments, | ||
| + kCVAttachmentMode_ShouldPropagate); | ||
| + CFRelease(attachments); | ||
| + ff_update_hdr_metadata(output, 100.0f); |
There was a problem hiding this comment.
ff_update_hdr_metadata(output, 100.0f);
I wasn't sure if this was good enough since dovi meta provides per-scene brightness. Add it for cuda/ocl too?
There was a problem hiding this comment.
This is reserved for future use because for DOVI to PQ direct passthrough without tonemapping will still cause washed out color as a lot of dovi scene is mastered at 4000nit, if we want to ever implement this, we need to tonemap it to 1000nit HDR10 range, so this value should always be 1000nit here.
There was a problem hiding this comment.
And in this case, BT2390 would be the only viable method as all other methods lacks a proper way to handle target peak.
| @@ -1807,6 +1850,7 @@ new file mode 100644 | |||
| + { "tonemap_mode", "Tonemap mode selection", OFFSET(tonemap_mode), AV_OPT_TYPE_INT, { .i64 = TONEMAP_MODE_MAX }, TONEMAP_MODE_MAX, TONEMAP_MODE_COUNT - 1, FLAGS, "tonemap_mode" }, | |||
| + { "max", 0, 0, AV_OPT_TYPE_CONST, { .i64 = TONEMAP_MODE_MAX }, 0, 0, FLAGS, "tonemap_mode" }, | |||
| + { "rgb", 0, 0, AV_OPT_TYPE_CONST, { .i64 = TONEMAP_MODE_RGB }, 0, 0, FLAGS, "tonemap_mode" }, | |||
| + { "relative_luminance", 0, 0, AV_OPT_TYPE_CONST, { .i64 = TONEMAP_MODE_RELATIVE_LUMINANCE }, 0, 0, FLAGS, "tonemap_mode" }, | |||
There was a problem hiding this comment.
How about we abbreviate this new method to LUM? (IMHO the shorter the better in cli options - TONEMAP_MODE_LUM and :tonemap_mode=lum)
There was a problem hiding this comment.
My main concern is the ambiguity of the abbreviations. Unlike MAX and RGB which has a very clear and definitive meaning, LUM could be misinterpreted as three terms are very similar: The luminance L measured in cd/m^2, the relative luminance normalized to the percentage of the reference white in linear space, and the luma Y' which is a a weighted sum of nonlinear R'G'B' components. If we are going to make such abbreviation, we need a clear documentation.
There was a problem hiding this comment.
I've thought about this as well, especially the ambiguity between lumi and luma. Only clear documentation can solve this, otherwise we have to make the choice for the user by implementing the auto option and hide these specific methods behind.
If users use ffmpeg -h filter=tonemap_* to query, then things should be much simpler.
{ "max", "Brightest channel based tonemap", 0, AV_OPT_TYPE_CONST, {.i64 = TONEMAP_MODE_MAX}, 0, 0, FLAGS, "tonemap_mode" },
{ "rgb", "Per-channel based tonemap", 0, AV_OPT_TYPE_CONST, {.i64 = TONEMAP_MODE_RGB}, 0, 0, FLAGS, "tonemap_mode" },
{ "lum", "Relative luminance based tonemap", 0, AV_OPT_TYPE_CONST, {.i64 = TONEMAP_MODE_LUM}, 0, 0, FLAGS, "tonemap_mode" },There was a problem hiding this comment.
I added help text to tonemap modes, which should be good enough to allow as to use lum as abbreviation
This includes NEON for ARMv8, SSE for x86-64-v2 and AVX+FMA for x86-64-v3 Test result with 4K HEVC 10bit HLG input, encoding with libx264 veryfast using bt2390: Intel Core i9-12900: tonemapx.c: 57fps tonemapx.sse: 74fps tonemapx.avx: 77fps Apple M1 Max: tonemapx.c:43fps tonemapx.neon: 57fps For comparison, original zscale+tonemap simd results: Intel Core i9-12900: tonemap.avx: 40fps tonemap.sse: 40fps tonemap.c: 32fps Apple M1 Max: tonemap.neon: 44fps tonemap.c: 35fps The original implementation is too memory heavy that dual-channel desktop CPUs are easily memory bounded due to the intermediate RGBF32 framebuffer sharing with zscale. Tonemapx lowered the the bandwidth requirement which brings significant performance gain to bandwidth limited platforms. Even for bandwidth-rich M1 Max it still provides significant performance boost due to better cache hitrate.
xz has moved back to github... |
Co-authored-by: Nyanmisaka <[email protected]>
This clips the input range better to prevent detail loss, also added an unused relative luminance mode as an alternative to the max mode, which calculates the relative luminance with sRGB/BT709 primaries to determine the color scaling factor. This mode works much better than our current RGB mode which does not distort the color and is more visually appealing. This mode is not present in libplacebo.
Changes
Issues