diff mbox series

[FFmpeg-devel] lavc/libopenh264: Drop openh264 runtime version checks

Message ID 20231209210731.201099-1-klember@redhat.com
State Accepted
Commit b391fdbf1aedbf9a77ef3079607f5d54f9d78309
Headers show
Series [FFmpeg-devel] lavc/libopenh264: Drop openh264 runtime version checks | 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

Commit Message

Kalev Lember Dec. 9, 2023, 9:07 p.m. UTC 
With the way the runtime checks are currently set up, every single
openh264 release, no matter how minor, is considered an ABI break and
requires ffmpeg recompilation. This is unnecessarily strict because it
doesn't allow downstream distributions to ship any openh264 bug fix
version updates without breaking ffmpeg's openh264 support.

Years ago, at the time when ffmpeg's openh264 support was merged,
openh264 releases were done without a versioned soname (the library was
just libopenh264.so, unversioned). Since then, starting with version
1.3.0, openh264 has started using versioned sonames and the intent has
been to bump the soname every time there's a new release with an ABI
change.

This patch drops the exact version check and instead adds a minimum
requirement on 1.3.0 to the configure script.

Signed-off-by: Kalev Lember <klember@redhat.com>
---
 configure                   |  2 +-
 libavcodec/libopenh264.c    | 15 ---------------
 libavcodec/libopenh264.h    |  2 --
 libavcodec/libopenh264dec.c |  4 ----
 libavcodec/libopenh264enc.c |  4 ----
 5 files changed, 1 insertion(+), 26 deletions(-)

Comments

Martin Storsjö Dec. 18, 2023, 10:15 p.m. UTC | #1 
On Sat, 9 Dec 2023, Kalev Lember wrote:

> With the way the runtime checks are currently set up, every single
> openh264 release, no matter how minor, is considered an ABI break and
> requires ffmpeg recompilation. This is unnecessarily strict because it
> doesn't allow downstream distributions to ship any openh264 bug fix
> version updates without breaking ffmpeg's openh264 support.
>
> Years ago, at the time when ffmpeg's openh264 support was merged,
> openh264 releases were done without a versioned soname (the library was
> just libopenh264.so, unversioned). Since then, starting with version
> 1.3.0, openh264 has started using versioned sonames and the intent has
> been to bump the soname every time there's a new release with an ABI
> change.
>
> This patch drops the exact version check and instead adds a minimum
> requirement on 1.3.0 to the configure script.
>
> Signed-off-by: Kalev Lember <klember@redhat.com>
> ---

Thanks, pushed now!

// Martin
diff mbox series

Patch

diff --git a/configure b/configure
index 7d2ee66000..29b0e463a1 100755
--- a/configure
+++ b/configure
@@ -6799,7 +6799,7 @@  enabled libopencv         && { check_headers opencv2/core/core_c.h &&
                                { check_pkg_config libopencv opencv opencv2/core/core_c.h cvCreateImageHeader ||
                                  require libopencv opencv2/core/core_c.h cvCreateImageHeader -lopencv_core -lopencv_imgproc; } ||
                                require_pkg_config libopencv opencv opencv/cxcore.h cvCreateImageHeader; }
-enabled libopenh264       && require_pkg_config libopenh264 openh264 wels/codec_api.h WelsGetCodecVersion
+enabled libopenh264       && require_pkg_config libopenh264 "openh264 >= 1.3.0" wels/codec_api.h WelsGetCodecVersion
 enabled libopenjpeg       && { check_pkg_config libopenjpeg "libopenjp2 >= 2.1.0" openjpeg.h opj_version ||
                                { require_pkg_config libopenjpeg "libopenjp2 >= 2.1.0" openjpeg.h opj_version -DOPJ_STATIC && add_cppflags -DOPJ_STATIC; } }
 enabled libopenmpt        && require_pkg_config libopenmpt "libopenmpt >= 0.2.6557" libopenmpt/libopenmpt.h openmpt_module_create -lstdc++ && append libopenmpt_extralibs "-lstdc++"
diff --git a/libavcodec/libopenh264.c b/libavcodec/libopenh264.c
index 0f6d28ed88..c80c85ea8b 100644
--- a/libavcodec/libopenh264.c
+++ b/libavcodec/libopenh264.c
@@ -46,18 +46,3 @@  void ff_libopenh264_trace_callback(void *ctx, int level, const char *msg)
     int equiv_ffmpeg_log_level = libopenh264_to_ffmpeg_log_level(level);
     av_log(ctx, equiv_ffmpeg_log_level, "%s\n", msg);
 }
-
-int ff_libopenh264_check_version(void *logctx)
-{
-    // Mingw GCC < 4.7 on x86_32 uses an incorrect/buggy ABI for the WelsGetCodecVersion
-    // function (for functions returning larger structs), thus skip the check in those
-    // configurations.
-#if !defined(_WIN32) || !defined(__GNUC__) || !ARCH_X86_32 || AV_GCC_VERSION_AT_LEAST(4, 7)
-    OpenH264Version libver = WelsGetCodecVersion();
-    if (memcmp(&libver, &g_stCodecVersion, sizeof(libver))) {
-        av_log(logctx, AV_LOG_ERROR, "Incorrect library version loaded\n");
-        return AVERROR(EINVAL);
-    }
-#endif
-    return 0;
-}
diff --git a/libavcodec/libopenh264.h b/libavcodec/libopenh264.h
index dbb9c5d429..0b462d6fdc 100644
--- a/libavcodec/libopenh264.h
+++ b/libavcodec/libopenh264.h
@@ -34,6 +34,4 @@ 
 
 void ff_libopenh264_trace_callback(void *ctx, int level, const char *msg);
 
-int ff_libopenh264_check_version(void *logctx);
-
 #endif /* AVCODEC_LIBOPENH264_H */
diff --git a/libavcodec/libopenh264dec.c b/libavcodec/libopenh264dec.c
index 7d650ae03e..b6a9bba2dc 100644
--- a/libavcodec/libopenh264dec.c
+++ b/libavcodec/libopenh264dec.c
@@ -52,13 +52,9 @@  static av_cold int svc_decode_init(AVCodecContext *avctx)
 {
     SVCContext *s = avctx->priv_data;
     SDecodingParam param = { 0 };
-    int err;
     int log_level;
     WelsTraceCallback callback_function;
 
-    if ((err = ff_libopenh264_check_version(avctx)) < 0)
-        return AVERROR_DECODER_NOT_FOUND;
-
     if (WelsCreateDecoder(&s->decoder)) {
         av_log(avctx, AV_LOG_ERROR, "Unable to create decoder\n");
         return AVERROR_UNKNOWN;
diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
index f518d0894e..6f231d22b2 100644
--- a/libavcodec/libopenh264enc.c
+++ b/libavcodec/libopenh264enc.c
@@ -110,14 +110,10 @@  static av_cold int svc_encode_init(AVCodecContext *avctx)
 {
     SVCContext *s = avctx->priv_data;
     SEncParamExt param = { 0 };
-    int err;
     int log_level;
     WelsTraceCallback callback_function;
     AVCPBProperties *props;
 
-    if ((err = ff_libopenh264_check_version(avctx)) < 0)
-        return AVERROR_ENCODER_NOT_FOUND;
-
     if (WelsCreateSVCEncoder(&s->encoder)) {
         av_log(avctx, AV_LOG_ERROR, "Unable to create encoder\n");
         return AVERROR_UNKNOWN;