Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#9195 closed defect (wontfix)

vibrance (video filter) incorrect defaults for luma

Reported by: Jim DeLaHunt Owned by:
Priority: normal Component: avfilter
Version: git-master Keywords:
Cc: Jim DeLaHunt Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

Summary of the bug: The "vibrance" video filter appears to have incorrect default values for the luma coefficient parameters "rlum" and "blum". They appear to be swapped.

How to reproduce:

% ffmpeg -h filter=vibrance
ffmpeg version 4.4 Copyright (c) 2000-2021 the FFmpeg developers
  built with Apple LLVM version 10.0.0 (clang-1000.10.44.4)
  configuration: --prefix=/opt/local --enable-swscale --enable-avfilter --enable-avresample --enable-libmp3lame --enable-libvorbis --enable-libopus --enable-librsvg --enable-libtheora --enable-libopenjpeg --enable-libmodplug --enable-libvpx --enable-libsoxr --enable-libspeex --enable-libass --enable-libbluray --enable-lzma --enable-gnutls --enable-fontconfig --enable-libfreetype --enable-libfribidi --disable-libjack --disable-libopencore-amrnb --disable-libopencore-amrwb --disable-indev=jack --enable-opencl --disable-outdev=xv --enable-audiotoolbox --enable-videotoolbox --enable-sdl2 --disable-securetransport --mandir=/opt/local/share/man --enable-shared --enable-pthreads --cc=/usr/bin/clang --enable-libdav1d --arch=x86_64 --enable-x86asm --enable-libx265 --enable-gpl --enable-postproc --enable-libx264 --enable-libxvid
  libavutil      56. 70.100 / 56. 70.100
  libavcodec     58.134.100 / 58.134.100
  libavformat    58. 76.100 / 58. 76.100
  libavdevice    58. 13.100 / 58. 13.100
  libavfilter     7.110.100 /  7.110.100
  libavresample   4.  0.  0 /  4.  0.  0
  libswscale      5.  9.100 /  5.  9.100
  libswresample   3.  9.100 /  3.  9.100
  libpostproc    55.  9.100 / 55.  9.100
Filter vibrance
  Boost or alter saturation.
    slice threading supported
    Inputs:
       #0: default (video)
    Outputs:
       #0: default (video)
vibrance AVOptions:
  intensity         <float>      ..FV.....T. set the intensity value (from -2 to 2) (default 0)
  rbal              <float>      ..FV.....T. set the red balance value (from -10 to 10) (default 1)
  gbal              <float>      ..FV.....T. set the green balance value (from -10 to 10) (default 1)
  bbal              <float>      ..FV.....T. set the blue balance value (from -10 to 10) (default 1)
  rlum              <float>      ..FV.....T. set the red luma coefficient (from 0 to 1) (default 0.072186)
  glum              <float>      ..FV.....T. set the green luma coefficient (from 0 to 1) (default 0.715158)
  blum              <float>      ..FV.....T. set the blue luma coefficient (from 0 to 1) (default 0.212656)
  alternate         <boolean>    ..FV.....T. use alternate colors (default false)

This filter has support for timeline through the 'enable' option.

The vibrance filter is documented at https://ffmpeg.org/ffmpeg-filters.html#vibrance .

It looks like coefficients "rlum", "glum", and "blum" are supposed to match the luma computation as defined in BT.709-6 (https://en.wikipedia.org/wiki/Rec._709) or something similar. Wikipedia gives this formula as:

R’G’B’ coefficients 0.2126, 0.7152, and 0.0722 (together they add to 1)

So, luma = 0.2126 * r + 0.7152 * g + 0.0722 * b, a value in [0.0, 1.0] for r, g, b in [0.0, 1.0]

But the default values I see in the FFmpeg vibrance filter are (per output above):

rlum (default 0.072186), glum (default 0.715158), blum (default 0.212656)

Thus it looks like the default values for rlum and blum are swapped.

Vibrance would still compute a luma a value in a range of [0.0, 1.0], but it wouldn't be the correct value per BT.709. Since the vibrance adjustments are relative to luma, they might be slightly off.

I believe the fix would be the simple, obvious exchange of default values in ffmpeg/libavfilter/vf_vibrance.c:372-374. But I haven't tried this.

Present code:

    { "rlum", "set the red luma coefficient",   OFFSET(lcoeffs[2]), AV_OPT_TYPE_FLOAT, {.dbl=0.072186}, 0,  1, VF },
    { "glum", "set the green luma coefficient", OFFSET(lcoeffs[0]), AV_OPT_TYPE_FLOAT, {.dbl=0.715158}, 0,  1, VF },
    { "blum", "set the blue luma coefficient",  OFFSET(lcoeffs[1]), AV_OPT_TYPE_FLOAT, {.dbl=0.212656}, 0,  1, VF },

Code with possible fix:

    { "rlum", "set the red luma coefficient",   OFFSET(lcoeffs[2]), AV_OPT_TYPE_FLOAT, {.dbl=0.212656}, 0,  1, VF },
    { "glum", "set the green luma coefficient", OFFSET(lcoeffs[0]), AV_OPT_TYPE_FLOAT, {.dbl=0.715158}, 0,  1, VF },
    { "blum", "set the blue luma coefficient",  OFFSET(lcoeffs[1]), AV_OPT_TYPE_FLOAT, {.dbl=0.072186}, 0,  1, VF },

Change History (4)

comment:1 by Balling, 3 years ago

or something similar

BT.601 matrix coeff. was derived from BT.470 System M primaries which are not in use in any BT.601 matrix content.

Also, 0.212656 is technically incorrect, as it will round to 0.2127. SMPTE 177 tells how to correctly derive the values (from BT.709 primaries and D65 white point).

comment:2 by Elon Musk, 3 years ago

Resolution: wontfix
Status: newclosed

comment:3 by Jim DeLaHunt, 3 years ago

I'm confused. Why the "wontfix"? Is there a claim that the current behaviour is correct?

I read Balling's comment as meaning, "current behaviour is wrong, but wrong in a different way than the bug description says, so the fix should be different than what the bug description proposes". Not, "current behaviour is right".

So, why the "wontfix"?

comment:4 by Jim DeLaHunt, 3 years ago

No reply so far to my question, why the "wontfix". I think it is a reasonable question, and deserves an answer. The developer should clarify why the current behaviour is correct, or if incorrect, why the ticket should be ignored.

It seems that richardpl might be the same person as Paul D Mahol, author of the vibrance filter. (A clue: https://github.com/richardpl .) If that is true, then the author of the filter is declining to respond to a bug report about the filter.

That makes me despair of getting the filter corrected (if incorrect). I think that is a sad situation for the project.

Note: See TracTickets for help on using tickets.