Message ID | 20220724101149.50584-1-epirat07@gmail.com |
---|---|
State | Accepted |
Commit | 564d7946de56155d1c42165a8b561fcf5028cbbc |
Headers | show |
Series | [FFmpeg-devel] configure: properly require libx264 if enabled | expand |
Context | Check | Description |
---|---|---|
yinshiyou/make_loongarch64 | success | Make finished |
yinshiyou/make_fate_loongarch64 | success | Make fate finished |
andriy/make_x86 | success | Make finished |
andriy/make_fate_x86 | success | Make fate finished |
On Sun, Jul 24, 2022 at 1:12 PM Marvin Scholz <epirat07@gmail.com> wrote: > > When libx264 can not be found even though it is enabled, it should error > out properly instead of silently disabling it. > --- > configure | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/configure b/configure > index 46f6490a64..6629d14099 100755 > --- a/configure > +++ b/configure > @@ -6673,7 +6673,7 @@ enabled libvpx && { > enabled libwebp && { > enabled libwebp_encoder && require_pkg_config libwebp "libwebp >= 0.2.0" webp/encode.h WebPGetEncoderVersion > enabled libwebp_anim_encoder && check_pkg_config libwebp_anim_encoder "libwebpmux >= 0.4.0" webp/mux.h WebPAnimEncoderOptionsInit; } > -enabled libx264 && check_pkg_config libx264 x264 "stdint.h x264.h" x264_encoder_encode && > +enabled libx264 && require_pkg_config libx264 x264 "stdint.h x264.h" x264_encoder_encode && Looking at the commit message in dd5a0302d5286e66ca03a57770537d5afa4c7775 , support for non-pkg-config usage was removed then, so require_pkg_config (which dies if the check doesn't succeed) seems like the correct option to utilize. LGTM Jan
On Sun, Jul 24, 2022 at 2:04 PM Jan Ekström <jeebjp@gmail.com> wrote: > > On Sun, Jul 24, 2022 at 1:12 PM Marvin Scholz <epirat07@gmail.com> wrote: > > > > When libx264 can not be found even though it is enabled, it should error > > out properly instead of silently disabling it. > > --- > > configure | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/configure b/configure > > index 46f6490a64..6629d14099 100755 > > --- a/configure > > +++ b/configure > > @@ -6673,7 +6673,7 @@ enabled libvpx && { > > enabled libwebp && { > > enabled libwebp_encoder && require_pkg_config libwebp "libwebp >= 0.2.0" webp/encode.h WebPGetEncoderVersion > > enabled libwebp_anim_encoder && check_pkg_config libwebp_anim_encoder "libwebpmux >= 0.4.0" webp/mux.h WebPAnimEncoderOptionsInit; } > > -enabled libx264 && check_pkg_config libx264 x264 "stdint.h x264.h" x264_encoder_encode && > > +enabled libx264 && require_pkg_config libx264 x264 "stdint.h x264.h" x264_encoder_encode && > > Looking at the commit message in > dd5a0302d5286e66ca03a57770537d5afa4c7775 , support for non-pkg-config > usage was removed then, so require_pkg_config (which dies if the check > doesn't succeed) seems like the correct option to utilize. > > LGTM Verified that this now properly works with and without x264 pc file being available (as in, "ERROR: x264 not found using pkg-config" without it, and properly enabling the feature if it is available). Applied as 564d7946de56155d1c42165a8b561fcf5028cbbc . I think this might make sense to back-port onto release/5.1 as dd5a0302d5286e66ca03a57770537d5afa4c7775 is on that branch as well? Jan
On Sun, Jul 24, 2022 at 5:48 PM Jan Ekström <jeebjp@gmail.com> wrote: > > On Sun, Jul 24, 2022 at 2:04 PM Jan Ekström <jeebjp@gmail.com> wrote: > > > > On Sun, Jul 24, 2022 at 1:12 PM Marvin Scholz <epirat07@gmail.com> wrote: > > > > > > When libx264 can not be found even though it is enabled, it should error > > > out properly instead of silently disabling it. > > > --- > > > configure | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/configure b/configure > > > index 46f6490a64..6629d14099 100755 > > > --- a/configure > > > +++ b/configure > > > @@ -6673,7 +6673,7 @@ enabled libvpx && { > > > enabled libwebp && { > > > enabled libwebp_encoder && require_pkg_config libwebp "libwebp >= 0.2.0" webp/encode.h WebPGetEncoderVersion > > > enabled libwebp_anim_encoder && check_pkg_config libwebp_anim_encoder "libwebpmux >= 0.4.0" webp/mux.h WebPAnimEncoderOptionsInit; } > > > -enabled libx264 && check_pkg_config libx264 x264 "stdint.h x264.h" x264_encoder_encode && > > > +enabled libx264 && require_pkg_config libx264 x264 "stdint.h x264.h" x264_encoder_encode && > > > > Looking at the commit message in > > dd5a0302d5286e66ca03a57770537d5afa4c7775 , support for non-pkg-config > > usage was removed then, so require_pkg_config (which dies if the check > > doesn't succeed) seems like the correct option to utilize. > > > > LGTM > > Verified that this now properly works with and without x264 pc file > being available (as in, "ERROR: x264 not found using pkg-config" > without it, and properly enabling the feature if it is available). > > Applied as 564d7946de56155d1c42165a8b561fcf5028cbbc . > > I think this might make sense to back-port onto release/5.1 as > dd5a0302d5286e66ca03a57770537d5afa4c7775 is on that branch as well? Got a positive response from j-b regarding back-porting this to release/5.1 , and I have applied the change to that branch as 6d83c5f87651499dbb5d310f7cbd12a25c7d07f9 . Jan
diff --git a/configure b/configure index 46f6490a64..6629d14099 100755 --- a/configure +++ b/configure @@ -6673,7 +6673,7 @@ enabled libvpx && { enabled libwebp && { enabled libwebp_encoder && require_pkg_config libwebp "libwebp >= 0.2.0" webp/encode.h WebPGetEncoderVersion enabled libwebp_anim_encoder && check_pkg_config libwebp_anim_encoder "libwebpmux >= 0.4.0" webp/mux.h WebPAnimEncoderOptionsInit; } -enabled libx264 && check_pkg_config libx264 x264 "stdint.h x264.h" x264_encoder_encode && +enabled libx264 && require_pkg_config libx264 x264 "stdint.h x264.h" x264_encoder_encode && require_cpp_condition libx264 x264.h "X264_BUILD >= 118" && { [ "$toolchain" != "msvc" ] || require_cpp_condition libx264 x264.h "X264_BUILD >= 158"; } &&