diff mbox series

[FFmpeg-devel,v4] libx264: Set min build version to 158

Message ID pull.30.v4.ffstaging.FFmpeg.1653476739751.ffmpegagent@gmail.com
State New
Headers show
Series [FFmpeg-devel,v4] libx264: Set min build version to 158 | expand

Checks

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
andriy/make_armv7_RPi4 success Make finished
andriy/make_fate_armv7_RPi4 success Make fate finished

Commit Message

Aman Karmani May 25, 2022, 11:05 a.m. UTC
From: Matt Oliver <protogonoi@gmail.com>

Was "[PATCH] libx264: Do not explicitly set X264_API_IMPORTS"

Setting X264_API_IMPORTS only affects msvc builds and it breaks
linking to static builds (although is required for shared builds).
This flag is set by x264 in its pkgconfig as required since build
158 (a615f027ed172e2dd5380e736d487aa858a0c4ff) from July 2019.
So this patch updates configure to require a newer x264 build that
correctly sets the imports flag.

The requirement for 158 is applied for msvc builds only,
no change is made for all other cases.

Co-authored-by: softworkz <softworkz@hotmail.com>
Signed-off-by: softworkz <softworkz@hotmail.com>
Signed-off-by: Matt Oliver <protogonoi@gmail.com>
---
    libx264: Set min build version to 158
    
    I'm submitting this patch on behalf of Matt with his permission.
    
    There was agreement that the >= 158 version requirement should be
    applied to MSVC builds only.
    
    v2: restrict the version requirement to msvc builds
    v3: fix unintended author change
    v4: add missing braces

Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-30%2Fsoftworkz%2Fsubmit_x264_api_imports_matt-v4
Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-30/softworkz/submit_x264_api_imports_matt-v4
Pull-Request: https://github.com/ffstaging/FFmpeg/pull/30

Range-diff vs v3:

 1:  374130a09e ! 1:  0d1bee35b0 libx264: Set min build version to 158
     @@ configure: enabled libvpx            && {
      -                             require_cpp_condition libx264 x264.h "X264_BUILD >= 118" &&
      -                             check_cpp_condition libx262 x264.h "X264_MPEG2"
      +enabled libx264           && check_pkg_config libx264 x264 "stdint.h x264.h" x264_encoder_encode &&
     -+                             require_cpp_condition libx264 x264.h "X264_BUILD >= 158" ||
     -+                             { "$toolchain" != msvc && require_cpp_condition libx264 x264.h "X264_BUILD >= 118"; }
     ++                             { require_cpp_condition libx264 x264.h "X264_BUILD >= 158" ||
     ++                             { "$toolchain" != msvc && require_cpp_condition libx264 x264.h "X264_BUILD >= 118"; }; }
       enabled libx265           && require_pkg_config libx265 x265 x265.h x265_api_get &&
                                    require_cpp_condition libx265 x265.h "X265_BUILD >= 70"
       enabled libxavs           && require libxavs "stdint.h xavs.h" xavs_encoder_encode "-lxavs $pthreads_extralibs $libm_extralibs"


 configure            | 8 +++-----
 libavcodec/libx264.c | 4 ----
 2 files changed, 3 insertions(+), 9 deletions(-)


base-commit: b033913d1c5998a29dfd13e9906dd707ff6eff12

Comments

Michael Niedermayer May 25, 2022, 3:15 p.m. UTC | #1
On Wed, May 25, 2022 at 11:05:39AM +0000, Matt Oliver wrote:
> From: Matt Oliver <protogonoi@gmail.com>
> 
> Was "[PATCH] libx264: Do not explicitly set X264_API_IMPORTS"
> 
> Setting X264_API_IMPORTS only affects msvc builds and it breaks
> linking to static builds (although is required for shared builds).
> This flag is set by x264 in its pkgconfig as required since build
> 158 (a615f027ed172e2dd5380e736d487aa858a0c4ff) from July 2019.
> So this patch updates configure to require a newer x264 build that
> correctly sets the imports flag.
> 
> The requirement for 158 is applied for msvc builds only,
> no change is made for all other cases.
> 
> Co-authored-by: softworkz <softworkz@hotmail.com>
> Signed-off-by: softworkz <softworkz@hotmail.com>
> Signed-off-by: Matt Oliver <protogonoi@gmail.com>
> ---
>     libx264: Set min build version to 158
>     
>     I'm submitting this patch on behalf of Matt with his permission.
>     
>     There was agreement that the >= 158 version requirement should be
>     applied to MSVC builds only.
>     
>     v2: restrict the version requirement to msvc builds
>     v3: fix unintended author change
>     v4: add missing braces
> 
> Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-30%2Fsoftworkz%2Fsubmit_x264_api_imports_matt-v4
> Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-30/softworkz/submit_x264_api_imports_matt-v4
> Pull-Request: https://github.com/ffstaging/FFmpeg/pull/30
> 
> Range-diff vs v3:
> 
>  1:  374130a09e ! 1:  0d1bee35b0 libx264: Set min build version to 158
>      @@ configure: enabled libvpx            && {
>       -                             require_cpp_condition libx264 x264.h "X264_BUILD >= 118" &&
>       -                             check_cpp_condition libx262 x264.h "X264_MPEG2"
>       +enabled libx264           && check_pkg_config libx264 x264 "stdint.h x264.h" x264_encoder_encode &&
>      -+                             require_cpp_condition libx264 x264.h "X264_BUILD >= 158" ||
>      -+                             { "$toolchain" != msvc && require_cpp_condition libx264 x264.h "X264_BUILD >= 118"; }
>      ++                             { require_cpp_condition libx264 x264.h "X264_BUILD >= 158" ||
>      ++                             { "$toolchain" != msvc && require_cpp_condition libx264 x264.h "X264_BUILD >= 118"; }; }
>        enabled libx265           && require_pkg_config libx265 x265 x265.h x265_api_get &&
>                                     require_cpp_condition libx265 x265.h "X265_BUILD >= 70"
>        enabled libxavs           && require libxavs "stdint.h xavs.h" xavs_encoder_encode "-lxavs $pthreads_extralibs $libm_extralibs"
> 
> 
>  configure            | 8 +++-----
>  libavcodec/libx264.c | 4 ----
>  2 files changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/configure b/configure
> index f115b21064..129473c75c 100755
> --- a/configure
> +++ b/configure
> @@ -6656,11 +6656,9 @@ 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 ||
> -                               { require libx264 "stdint.h x264.h" x264_encoder_encode "-lx264 $pthreads_extralibs $libm_extralibs" &&
> -                                 warn "using libx264 without pkg-config"; } } &&
> -                             require_cpp_condition libx264 x264.h "X264_BUILD >= 118" &&
> -                             check_cpp_condition libx262 x264.h "X264_MPEG2"
> +enabled libx264           && check_pkg_config libx264 x264 "stdint.h x264.h" x264_encoder_encode &&
> +                             { require_cpp_condition libx264 x264.h "X264_BUILD >= 158" ||
> +                             { "$toolchain" != msvc && require_cpp_condition libx264 x264.h "X264_BUILD >= 118"; }; }

On ubuntu LTS:

ERROR: X264_BUILD >= 158 not satisfied

If you think configure made a mistake, make sure you are using the latest
version from Git.  If the latest version fails, report the problem to the
ffmpeg-user@ffmpeg.org mailing list or IRC #ffmpeg on irc.libera.chat.
Include the log file "ffbuild/config.log" produced by configure as this will help
solve the problem.

[...]
Andreas Rheinhardt May 25, 2022, 10:53 p.m. UTC | #2
Michael Niedermayer:
> On Wed, May 25, 2022 at 11:05:39AM +0000, Matt Oliver wrote:
>> From: Matt Oliver <protogonoi@gmail.com>
>>
>> Was "[PATCH] libx264: Do not explicitly set X264_API_IMPORTS"
>>
>> Setting X264_API_IMPORTS only affects msvc builds and it breaks
>> linking to static builds (although is required for shared builds).
>> This flag is set by x264 in its pkgconfig as required since build
>> 158 (a615f027ed172e2dd5380e736d487aa858a0c4ff) from July 2019.
>> So this patch updates configure to require a newer x264 build that
>> correctly sets the imports flag.
>>
>> The requirement for 158 is applied for msvc builds only,
>> no change is made for all other cases.
>>
>> Co-authored-by: softworkz <softworkz@hotmail.com>
>> Signed-off-by: softworkz <softworkz@hotmail.com>
>> Signed-off-by: Matt Oliver <protogonoi@gmail.com>
>> ---
>>     libx264: Set min build version to 158
>>     
>>     I'm submitting this patch on behalf of Matt with his permission.
>>     
>>     There was agreement that the >= 158 version requirement should be
>>     applied to MSVC builds only.
>>     
>>     v2: restrict the version requirement to msvc builds
>>     v3: fix unintended author change
>>     v4: add missing braces
>>
>> Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-30%2Fsoftworkz%2Fsubmit_x264_api_imports_matt-v4
>> Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-30/softworkz/submit_x264_api_imports_matt-v4
>> Pull-Request: https://github.com/ffstaging/FFmpeg/pull/30
>>
>> Range-diff vs v3:
>>
>>  1:  374130a09e ! 1:  0d1bee35b0 libx264: Set min build version to 158
>>      @@ configure: enabled libvpx            && {
>>       -                             require_cpp_condition libx264 x264.h "X264_BUILD >= 118" &&
>>       -                             check_cpp_condition libx262 x264.h "X264_MPEG2"
>>       +enabled libx264           && check_pkg_config libx264 x264 "stdint.h x264.h" x264_encoder_encode &&
>>      -+                             require_cpp_condition libx264 x264.h "X264_BUILD >= 158" ||
>>      -+                             { "$toolchain" != msvc && require_cpp_condition libx264 x264.h "X264_BUILD >= 118"; }
>>      ++                             { require_cpp_condition libx264 x264.h "X264_BUILD >= 158" ||
>>      ++                             { "$toolchain" != msvc && require_cpp_condition libx264 x264.h "X264_BUILD >= 118"; }; }
>>        enabled libx265           && require_pkg_config libx265 x265 x265.h x265_api_get &&
>>                                     require_cpp_condition libx265 x265.h "X265_BUILD >= 70"
>>        enabled libxavs           && require libxavs "stdint.h xavs.h" xavs_encoder_encode "-lxavs $pthreads_extralibs $libm_extralibs"
>>
>>
>>  configure            | 8 +++-----
>>  libavcodec/libx264.c | 4 ----
>>  2 files changed, 3 insertions(+), 9 deletions(-)
>>
>> diff --git a/configure b/configure
>> index f115b21064..129473c75c 100755
>> --- a/configure
>> +++ b/configure
>> @@ -6656,11 +6656,9 @@ 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 ||
>> -                               { require libx264 "stdint.h x264.h" x264_encoder_encode "-lx264 $pthreads_extralibs $libm_extralibs" &&
>> -                                 warn "using libx264 without pkg-config"; } } &&
>> -                             require_cpp_condition libx264 x264.h "X264_BUILD >= 118" &&
>> -                             check_cpp_condition libx262 x264.h "X264_MPEG2"
>> +enabled libx264           && check_pkg_config libx264 x264 "stdint.h x264.h" x264_encoder_encode &&
>> +                             { require_cpp_condition libx264 x264.h "X264_BUILD >= 158" ||
>> +                             { "$toolchain" != msvc && require_cpp_condition libx264 x264.h "X264_BUILD >= 118"; }; }
> 
> On ubuntu LTS:
> 
> ERROR: X264_BUILD >= 158 not satisfied
> 
> If you think configure made a mistake, make sure you are using the latest
> version from Git.  If the latest version fails, report the problem to the
> ffmpeg-user@ffmpeg.org mailing list or IRC #ffmpeg on irc.libera.chat.
> Include the log file "ffbuild/config.log" produced by configure as this will help
> solve the problem.
> 

That is because require_cpp_condition has a "|| die "ERROR: $condition
not satisfied" at the end, so ordinary short-circuiting logic does not
apply.
require_cpp_condition libx264 x264.h "X264_BUILD >= 118" && {
"$toolchain" != msvc || require_cpp_condition libx264 x264.h "X264_BUILD
>= 158"; }
would avoid this and might actually work.

- Andreas
Soft Works May 26, 2022, 7:26 a.m. UTC | #3
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Andreas
> Rheinhardt
> Sent: Thursday, May 26, 2022 12:53 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v4] libx264: Set min build version to
> 158
> 
> Michael Niedermayer:
> > On Wed, May 25, 2022 at 11:05:39AM +0000, Matt Oliver wrote:
> >> From: Matt Oliver <protogonoi@gmail.com>
> >>
> >> Was "[PATCH] libx264: Do not explicitly set X264_API_IMPORTS"
> >>
> >> Setting X264_API_IMPORTS only affects msvc builds and it breaks
> >> linking to static builds (although is required for shared builds).
> >> This flag is set by x264 in its pkgconfig as required since build
> >> 158 (a615f027ed172e2dd5380e736d487aa858a0c4ff) from July 2019.
> >> So this patch updates configure to require a newer x264 build that
> >> correctly sets the imports flag.
> >>
> >> The requirement for 158 is applied for msvc builds only,
> >> no change is made for all other cases.
> >>
> >> Co-authored-by: softworkz <softworkz@hotmail.com>
> >> Signed-off-by: softworkz <softworkz@hotmail.com>
> >> Signed-off-by: Matt Oliver <protogonoi@gmail.com>
> >> ---
> >>     libx264: Set min build version to 158
> >>
> >>     I'm submitting this patch on behalf of Matt with his permission.
> >>
> >>     There was agreement that the >= 158 version requirement should be
> >>     applied to MSVC builds only.
> >>
> >>     v2: restrict the version requirement to msvc builds
> >>     v3: fix unintended author change
> >>     v4: add missing braces
> >>
> >> Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-
> ffstaging-30%2Fsoftworkz%2Fsubmit_x264_api_imports_matt-v4
> >> Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-
> ffstaging-30/softworkz/submit_x264_api_imports_matt-v4
> >> Pull-Request: https://github.com/ffstaging/FFmpeg/pull/30
> >>
> >> Range-diff vs v3:
> >>
> >>  1:  374130a09e ! 1:  0d1bee35b0 libx264: Set min build version to 158
> >>      @@ configure: enabled libvpx            && {
> >>       -                             require_cpp_condition libx264
> x264.h "X264_BUILD >= 118" &&
> >>       -                             check_cpp_condition libx262 x264.h
> "X264_MPEG2"
> >>       +enabled libx264           && check_pkg_config libx264 x264
> "stdint.h x264.h" x264_encoder_encode &&
> >>      -+                             require_cpp_condition libx264
> x264.h "X264_BUILD >= 158" ||
> >>      -+                             { "$toolchain" != msvc &&
> require_cpp_condition libx264 x264.h "X264_BUILD >= 118"; }
> >>      ++                             { require_cpp_condition libx264
> x264.h "X264_BUILD >= 158" ||
> >>      ++                             { "$toolchain" != msvc &&
> require_cpp_condition libx264 x264.h "X264_BUILD >= 118"; }; }
> >>        enabled libx265           && require_pkg_config libx265 x265
> x265.h x265_api_get &&
> >>                                     require_cpp_condition libx265
> x265.h "X265_BUILD >= 70"
> >>        enabled libxavs           && require libxavs "stdint.h xavs.h"
> xavs_encoder_encode "-lxavs $pthreads_extralibs $libm_extralibs"
> >>
> >>
> >>  configure            | 8 +++-----
> >>  libavcodec/libx264.c | 4 ----
> >>  2 files changed, 3 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/configure b/configure
> >> index f115b21064..129473c75c 100755
> >> --- a/configure
> >> +++ b/configure
> >> @@ -6656,11 +6656,9 @@ 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 ||
> >> -                               { require libx264 "stdint.h x264.h"
> x264_encoder_encode "-lx264 $pthreads_extralibs $libm_extralibs" &&
> >> -                                 warn "using libx264 without pkg-
> config"; } } &&
> >> -                             require_cpp_condition libx264 x264.h
> "X264_BUILD >= 118" &&
> >> -                             check_cpp_condition libx262 x264.h
> "X264_MPEG2"
> >> +enabled libx264           && check_pkg_config libx264 x264 "stdint.h
> x264.h" x264_encoder_encode &&
> >> +                             { require_cpp_condition libx264 x264.h
> "X264_BUILD >= 158" ||
> >> +                             { "$toolchain" != msvc &&
> require_cpp_condition libx264 x264.h "X264_BUILD >= 118"; }; }
> >
> > On ubuntu LTS:
> >
> > ERROR: X264_BUILD >= 158 not satisfied
> >
> > If you think configure made a mistake, make sure you are using the
> latest
> > version from Git.  If the latest version fails, report the problem to
> the
> > ffmpeg-user@ffmpeg.org mailing list or IRC #ffmpeg on irc.libera.chat.
> > Include the log file "ffbuild/config.log" produced by configure as this
> will help
> > solve the problem.
> >
> 
> That is because require_cpp_condition has a "|| die "ERROR: $condition
> not satisfied" at the end, so ordinary short-circuiting logic does not
> apply.
> require_cpp_condition libx264 x264.h "X264_BUILD >= 118" && {
> "$toolchain" != msvc || require_cpp_condition libx264 x264.h "X264_BUILD
> >= 158"; }
> would avoid this and might actually work.

Cool, thanks!

@Michael - thanks for testing. It's not that I hadn't tested myself 
but I tested MinGW with existing libx264 and Ubuntu, where I don't have
libx264 installed.

softworkz
diff mbox series

Patch

diff --git a/configure b/configure
index f115b21064..129473c75c 100755
--- a/configure
+++ b/configure
@@ -6656,11 +6656,9 @@  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 ||
-                               { require libx264 "stdint.h x264.h" x264_encoder_encode "-lx264 $pthreads_extralibs $libm_extralibs" &&
-                                 warn "using libx264 without pkg-config"; } } &&
-                             require_cpp_condition libx264 x264.h "X264_BUILD >= 118" &&
-                             check_cpp_condition libx262 x264.h "X264_MPEG2"
+enabled libx264           && check_pkg_config libx264 x264 "stdint.h x264.h" x264_encoder_encode &&
+                             { require_cpp_condition libx264 x264.h "X264_BUILD >= 158" ||
+                             { "$toolchain" != msvc && require_cpp_condition libx264 x264.h "X264_BUILD >= 118"; }; }
 enabled libx265           && require_pkg_config libx265 x265 x265.h x265_api_get &&
                              require_cpp_condition libx265 x265.h "X265_BUILD >= 70"
 enabled libxavs           && require libxavs "stdint.h xavs.h" xavs_encoder_encode "-lxavs $pthreads_extralibs $libm_extralibs"
diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
index 4ce3791ae8..14177b3016 100644
--- a/libavcodec/libx264.c
+++ b/libavcodec/libx264.c
@@ -37,10 +37,6 @@ 
 #include "atsc_a53.h"
 #include "sei.h"
 
-#if defined(_MSC_VER)
-#define X264_API_IMPORTS 1
-#endif
-
 #include <x264.h>
 #include <float.h>
 #include <math.h>