mbox series

[FFmpeg-devel,v2,0/3] libx264 configure check clean-up, removal of libx264rgb

Message ID 20210702112513.36348-1-jeebjp@gmail.com
Headers show
Series libx264 configure check clean-up, removal of libx264rgb | expand


Jan Ekström July 2, 2021, 11:25 a.m. UTC
Changes compared to v1:
- As the ff_libx264rgb_encoder extern usage depended on the configure define
  being set to a falsie, move the actual removal of the configure entry for it
  to the change which removes libx264rgb altogether. Overall resulting diff
  between v1 and v2 is zero.

Done in three separate change sets, as this way if the last change
is deemed too controversial, the earlier changes should be applicable
by themselves.

- The first change fixes libx264rgb enablement without having x264.h
  in the system default include path, such as with custom prefixes.

- The second change removes the separate X264_CSP_BGR check as x264.h
  has this define unconditionally defined with the required X264_BUILD
  118 or newer (it was added a few X264_BUILD versions before).

  This change was checked by bumping the require_cpp_condition
  check to X264_BUILD >= 255 and checking with both pkg-config
  as well as by not having PKG_CONFIG_PATH defined as well as
  making the non-pkg-config check pass with
  `--extra-cflags="-I/prefix/include" --extra-ldflags="-L/prefix/lib -ldl"`
  So the X264_BUILD check should properly fail the enablement in
  case X264_BUILD is older than the one requested in the relevant

- The third and last change is probably the most controversial,
  as in the removal of the separate libx264rgb wrapper.

  This is due to no other encoder wrapper in libavcodec (such as
  libx265 for example) being given such treatment, as well as due to
  the default behavior with ffmpeg.c, RGB input and the libx264 wrapper
  now being not something commonly supported - such as 4:2:0 YCbCr - but
  rather 4:4:4 YCbCr.

  Thus, it is clear that users should rather just define what they
  require (which with RGB input they already seem to be required to do),
  rather than trying to make the separation "do the right thing",
  which it does not currently seem to be leading to.

  Of course, I might be completely incorrect with regards to why the
  split was originally done, but I would expect that if it was for
  more supported color spaces, that would be 4:2:0 and not just
  "normal" 4:4:4 (which in H.264 coding-wise only differs by its
  metadata to BGR).

Best regards,

Jan Ekström (3):
  configure: move x264_csp_bgr check under general libx264 checks
  {configure,avcodec/libx264}: remove separate x264_csp_bgr check
  avcodec/libx264: remove separate libx264rgb RGB wrapper

 configure              |  3 ---
 doc/encoders.texi      |  7 +++---
 libavcodec/allcodecs.c |  1 -
 libavcodec/libx264.c   | 48 ++++++------------------------------------
 libavcodec/version.h   |  2 +-
 5 files changed, 11 insertions(+), 50 deletions(-)