On Mon, 25 Jul 2016, Diego Biurrun wrote:
On Fri, Jul 15, 2016 at 01:43:16PM +0300, Martin Storsjö wrote:
This simplifies use of the patent license, simplifying use with a
library that has been downloaded at runtime (making it possible
to actually load and run libavcodec before the corresponding
OpenH264 library exists).
---
This patch was sent for review earlier, and got some feedback, but it was
deferred back then due not knowing how much interest there would be in it
- see https://patches.libav.org/patch/56287/.
Now there seems to be interest
in it, see https://lists.libav.org/pipermail/libav-devel/2016-July/078139.html,
so I'm resending it, with the feedback from the previous round applied,
and updated on top of the latest master.
---
configure | 8 ++++--
libavcodec/Makefile | 1 +
libavcodec/libopenh264.c | 4 +--
libavcodec/libopenh264.h | 5 +++-
libavcodec/libopenh264dec.c | 61 ++++++++++++++++++++++++++++++++++++++++++---
libavcodec/libopenh264enc.c | 49 +++++++++++++++++++++++++++++++++---
6 files changed, 115 insertions(+), 13 deletions(-)
I'm undecided about the general merits of this patch.
Some implementation remarks below...
--- a/configure
+++ b/configure
@@ -4594,6 +4596,8 @@ enabled libopencore_amrnb && require libopencore_amrnb
opencore-amrnb/interf_dec
enabled libopencv && require_pkg_config opencv opencv/cv.h
cvCreateImageHeader
enabled libopenh264 && require_pkg_config openh264 wels/codec_api.h
WelsGetCodecVersion
+enabled libopenh264_dyn && { { check_header wels/codec_ver.h && enabled
dyn_lib_open; } ||
+ die "ERROR: OpenH264 1.3 header not found, or
dlopen/LoadLibrary not found"; }
Not sure if mentioning the version number here is a good idea looking
forward.
Right, removed.
index 8eb7d36..f639ac5 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -771,6 +771,7 @@ SKIPHEADERS += %_tablegen.h
\
SKIPHEADERS-$(CONFIG_D3D11VA) += d3d11va.h dxva2_internal.h
SKIPHEADERS-$(CONFIG_DXVA2) += dxva2.h dxva2_internal.h
SKIPHEADERS-$(CONFIG_LIBSCHROEDINGER) += libschroedinger.h
+SKIPHEADERS-$(CONFIG_LIBOPENH264) += libopenh264.h
This will also skip the header if LIBOPENH264_DYN is set, which I think
is not what you intend.
Hmm, indeed. Fixing that properly is a bit harder...
--- a/libavcodec/libopenh264dec.c
+++ b/libavcodec/libopenh264dec.c
@@ -23,6 +23,9 @@
#include <wels/codec_ver.h>
#include "libavutil/common.h"
+#if CONFIG_LIBOPENH264_DYN
+#include "libavutil/dyn_lib_open.h"
+#endif
You are missing a config.h #include.
Sure
@@ -239,4 +291,5 @@ AVCodec ff_libopenh264_decoder = {
.capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_DR1,
.caps_internal = FF_CODEC_CAP_SETS_PKT_DTS | FF_CODEC_CAP_INIT_THREADSAFE
|
FF_CODEC_CAP_INIT_CLEANUP,
+ .priv_class = &class,
unrelated?
No, it's for hooking up the options (for setting the library name to load;
previously the decoder wrapper didn't have any options).
--- a/libavcodec/libopenh264enc.c
+++ b/libavcodec/libopenh264enc.c
@@ -24,6 +24,9 @@
#include "libavutil/attributes.h"
#include "libavutil/common.h"
+#if CONFIG_LIBOPENH264_DYN
+#include "libavutil/dyn_lib_open.h"
+#endif
#include "libavutil/opt.h"
#include "libavutil/internal.h"
#include "libavutil/intreadwrite.h"
You're missing a config.h #include.
--- a/libavcodec/libopenh264dec.c
+++ b/libavcodec/libopenh264dec.c
@@ -67,15 +93,41 @@ static av_cold int svc_decode_init(AVCodecContext *avctx)
WelsTraceCallback callback_function;
+ int (*create_func)(ISVCDecoder **ppDecoder);
+ OpenH264Version (*get_version_func)(void);
+
+#if CONFIG_LIBOPENH264_DYN
+ if (!s->libname) {
+ av_log(avctx, AV_LOG_ERROR, "No library name provided\n");
+ return AVERROR(EINVAL);
+ }
+ s->lib = load_library(s->libname);
+ if (!s->lib) {
+ av_log(avctx, AV_LOG_ERROR, "Unable to load %s\n", s->libname);
+ return AVERROR(EINVAL);
+ }
+#endif
- if ((err = ff_libopenh264_check_version(avctx)) < 0)
+ if ((err = ff_libopenh264_check_version(avctx, get_version_func)) < 0)
return err;
--- a/libavcodec/libopenh264enc.c
+++ b/libavcodec/libopenh264enc.c
@@ -97,11 +112,37 @@ static av_cold int svc_encode_init(AVCodecContext *avctx)
WelsTraceCallback callback_function;
AVCPBProperties *props;
+ int (*create_func)(ISVCEncoder **ppEncoder);
+ OpenH264Version (*get_version_func)(void);
+
+#if CONFIG_LIBOPENH264_DYN
+ if (!s->libname) {
+ av_log(avctx, AV_LOG_ERROR, "No library name provided\n");
+ return AVERROR(EINVAL);
+ }
+ s->lib = load_library(s->libname);
+ if (!s->lib) {
+ av_log(avctx, AV_LOG_ERROR, "Unable to load %s\n", s->libname);
+ return AVERROR(EINVAL);
+ }
+#endif
- if ((err = ff_libopenh264_check_version(avctx)) < 0)
+ if ((err = ff_libopenh264_check_version(avctx, get_version_func)) < 0)
return err;
This looks similar enough that it could be shared between decoder and
encoder.
No, I don't think it's really worth it, since it'd still end up something
like this:
#if CONFIG_LIBOPENH264_DYN
if (load_library_ifdef_dyn_and_warn_if_not(s->libname))
return AVERROR(EINVAL);
create_func = get_function()
#endif
You save a reused load_library call and error logging, nothing more, both
which are trivial enough.
// Martin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
