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

[Jan Ekström]

On Fri, Jul 2, 2021 at 12:54 AM Jan Ekström <jeebjp at gmail.com> wrote:
>
> 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
>   require_cpp_condition.
>
> - 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
>
> 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(-)
>
> --
> 2.31.1
>

Just as an additional note, I just checked the referenced ticket in
the x264rgb addition (https://trac.ffmpeg.org/ticket/658), and as far
as I can tell the requested behavior was to make 4:2:0 get picked by
default.

This might have been the case when the wrapper was added, but looking
at f.ex. https://trac.ffmpeg.org/ticket/9132 the current behavior is
leading to 4:4:4, which is similarly supported by decoders as BGR (as
they differ only by the metadata).

Jan