On Fri, Jul 2, 2021 at 12:54 AM Jan Ekström <jee...@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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
