[FFmpeg-devel] [PATCH] lavc/amfenc: moving amf common code (library and context) to lavu/hwcontext_amf from amfenc to be reused in other amf components
This patch moves AMF common parts from amfenc to hwcontext_amf.
Now av_hwdevice_ctx API is used for AMF context creation/destroying.
This patch does not change component behaviour.
it contains only restructurization for further patches with new amf components
---
Sending updated patch based on Mark's review
1) moved device free code from amf_device_uninit to amf_device_free
2) added comments for AVAMFDeviceContext structure
3) Fixed trailing whitespace warnings
libavcodec/amfenc.c| 247 +---
libavcodec/amfenc.h| 27 +
libavutil/Makefile | 2 +
libavutil/hwcontext.c | 4 +
libavutil/hwcontext.h | 1 +
libavutil/hwcontext_amf.c | 253 +
libavutil/hwcontext_amf.h | 54 +
libavutil/hwcontext_internal.h | 1 +
8 files changed, 350 insertions(+), 239 deletions(-)
create mode 100644 libavutil/hwcontext_amf.c
create mode 100644 libavutil/hwcontext_amf.h
diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c
index 384d8efc92..5a0fb90433 100644
--- a/libavcodec/amfenc.c
+++ b/libavcodec/amfenc.c
@@ -21,13 +21,7 @@
#include "libavutil/avassert.h"
#include "libavutil/imgutils.h"
#include "libavutil/hwcontext.h"
-#if CONFIG_D3D11VA
-#include "libavutil/hwcontext_d3d11va.h"
-#endif
-#if CONFIG_DXVA2
-#define COBJMACROS
-#include "libavutil/hwcontext_dxva2.h"
-#endif
+
#include "libavutil/mem.h"
#include "libavutil/pixdesc.h"
#include "libavutil/time.h"
@@ -35,14 +29,12 @@
#include "amfenc.h"
#include "internal.h"
-#if CONFIG_D3D11VA
-#include
+#if CONFIG_DXVA2
+#include
#endif
-#ifdef _WIN32
-#include "compat/w32dlfcn.h"
-#else
-#include
+#if CONFIG_D3D11VA
+#include
#endif
#define FFMPEG_AMF_WRITER_ID L"ffmpeg_amf"
@@ -88,34 +80,18 @@ static enum AMF_SURFACE_FORMAT amf_av_to_amf_format(enum
AVPixelFormat fmt)
return AMF_SURFACE_UNKNOWN;
}
-static void AMF_CDECL_CALL AMFTraceWriter_Write(AMFTraceWriter *pThis,
-const wchar_t *scope, const wchar_t *message)
-{
-AmfTraceWriter *tracer = (AmfTraceWriter*)pThis;
-av_log(tracer->avctx, AV_LOG_DEBUG, "%ls: %ls", scope, message); // \n is
provided from AMF
-}
-static void AMF_CDECL_CALL AMFTraceWriter_Flush(AMFTraceWriter *pThis)
-{
-}
-
-static AMFTraceWriterVtbl tracer_vtbl =
-{
-.Write = AMFTraceWriter_Write,
-.Flush = AMFTraceWriter_Flush,
-};
-
-static int amf_load_library(AVCodecContext *avctx)
+static int amf_init_context(AVCodecContext *avctx)
{
-AmfContext*ctx = avctx->priv_data;
-AMFInit_Fn init_fun;
-AMFQueryVersion_Fn version_fun;
-AMF_RESULT res;
+AmfContext *ctx = avctx->priv_data;
+AVAMFDeviceContext *amf_ctx;
+int ret;
ctx->delayed_frame = av_frame_alloc();
if (!ctx->delayed_frame) {
return AVERROR(ENOMEM);
}
+
// hardcoded to current HW queue size - will realloc in
timestamp_queue_enqueue() if too small
ctx->timestamp_list = av_fifo_alloc((avctx->max_b_frames + 16) *
sizeof(int64_t));
if (!ctx->timestamp_list) {
@@ -123,119 +99,9 @@ static int amf_load_library(AVCodecContext *avctx)
}
ctx->dts_delay = 0;
-
-ctx->library = dlopen(AMF_DLL_NAMEA, RTLD_NOW | RTLD_LOCAL);
-AMF_RETURN_IF_FALSE(ctx, ctx->library != NULL,
-AVERROR_UNKNOWN, "DLL %s failed to open\n", AMF_DLL_NAMEA);
-
-init_fun = (AMFInit_Fn)dlsym(ctx->library, AMF_INIT_FUNCTION_NAME);
-AMF_RETURN_IF_FALSE(ctx, init_fun != NULL, AVERROR_UNKNOWN, "DLL %s failed
to find function %s\n", AMF_DLL_NAMEA, AMF_INIT_FUNCTION_NAME);
-
-version_fun = (AMFQueryVersion_Fn)dlsym(ctx->library,
AMF_QUERY_VERSION_FUNCTION_NAME);
-AMF_RETURN_IF_FALSE(ctx, version_fun != NULL, AVERROR_UNKNOWN, "DLL %s
failed to find function %s\n", AMF_DLL_NAMEA, AMF_QUERY_VERSION_FUNCTION_NAME);
-
-res = version_fun(>version);
-AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "%s failed with
error %d\n", AMF_QUERY_VERSION_FUNCTION_NAME, res);
-res = init_fun(AMF_FULL_VERSION, >factory);
-AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "%s failed with
error %d\n", AMF_INIT_FUNCTION_NAME, res);
-res = ctx->factory->pVtbl->GetTrace(ctx->factory, >trace);
-AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "GetTrace()
failed with error %d\n", res);
-res = ctx->factory->pVtbl->GetDebug(ctx->factory, >debug);
-AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "GetDebug()
failed with error %d\n", res);
-return 0;
-}
-
-#if CONFIG_D3D11VA
-static int amf_init_from_d3d11_device(AVCodecContext *avctx,
AVD3D11VADeviceContext *hwctx)
-{
-AmfContext *ctx = avctx->priv_data;
-AMF_RESULT res;
-
-res = ctx->context->pVtbl->InitDX11(ctx->context, hwctx->device,
AMF_DX11_1);
-if (res != AMF_OK) {
-if (res == AMF_NOT_SUPPORTED)
-av_log(avctx, AV_LOG_ERROR, "AMF via D3D11 is not supported
[FFmpeg-devel] [PATCH] lavc/amfenc: moving amf common code (library and context) to lavu/hwcontext_amf from amfenc to be reused in other amf components
This patch moves AMF common parts from amfenc to hwcontext_amf.
Now av_hwdevice_ctx API is used for AMF context creation/destroying.
This patch does not change component behaviour.
it contains only restructurization for further patches with new amf components
---
Sending updated patch based on Mark's review
1) moved device free code from amf_device_uninit to amf_device_free
2) added comments for AVAMFDeviceContext structure
libavcodec/amfenc.c| 247 +---
libavcodec/amfenc.h| 27 +
libavutil/Makefile | 2 +
libavutil/hwcontext.c | 4 +
libavutil/hwcontext.h | 1 +
libavutil/hwcontext_amf.c | 253 +
libavutil/hwcontext_amf.h | 54 +
libavutil/hwcontext_internal.h | 1 +
8 files changed, 350 insertions(+), 239 deletions(-)
create mode 100644 libavutil/hwcontext_amf.c
create mode 100644 libavutil/hwcontext_amf.h
diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c
index 384d8efc92..4c907ca3bc 100644
--- a/libavcodec/amfenc.c
+++ b/libavcodec/amfenc.c
@@ -21,13 +21,7 @@
#include "libavutil/avassert.h"
#include "libavutil/imgutils.h"
#include "libavutil/hwcontext.h"
-#if CONFIG_D3D11VA
-#include "libavutil/hwcontext_d3d11va.h"
-#endif
-#if CONFIG_DXVA2
-#define COBJMACROS
-#include "libavutil/hwcontext_dxva2.h"
-#endif
+
#include "libavutil/mem.h"
#include "libavutil/pixdesc.h"
#include "libavutil/time.h"
@@ -35,14 +29,12 @@
#include "amfenc.h"
#include "internal.h"
-#if CONFIG_D3D11VA
-#include
+#if CONFIG_DXVA2
+#include
#endif
-#ifdef _WIN32
-#include "compat/w32dlfcn.h"
-#else
-#include
+#if CONFIG_D3D11VA
+#include
#endif
#define FFMPEG_AMF_WRITER_ID L"ffmpeg_amf"
@@ -88,34 +80,18 @@ static enum AMF_SURFACE_FORMAT amf_av_to_amf_format(enum
AVPixelFormat fmt)
return AMF_SURFACE_UNKNOWN;
}
-static void AMF_CDECL_CALL AMFTraceWriter_Write(AMFTraceWriter *pThis,
-const wchar_t *scope, const wchar_t *message)
-{
-AmfTraceWriter *tracer = (AmfTraceWriter*)pThis;
-av_log(tracer->avctx, AV_LOG_DEBUG, "%ls: %ls", scope, message); // \n is
provided from AMF
-}
-
-static void AMF_CDECL_CALL AMFTraceWriter_Flush(AMFTraceWriter *pThis)
-{
-}
-static AMFTraceWriterVtbl tracer_vtbl =
-{
-.Write = AMFTraceWriter_Write,
-.Flush = AMFTraceWriter_Flush,
-};
-
-static int amf_load_library(AVCodecContext *avctx)
+static int amf_init_context(AVCodecContext *avctx)
{
-AmfContext*ctx = avctx->priv_data;
-AMFInit_Fn init_fun;
-AMFQueryVersion_Fn version_fun;
-AMF_RESULT res;
+AmfContext *ctx = avctx->priv_data;
+AVAMFDeviceContext *amf_ctx;
+int ret;
ctx->delayed_frame = av_frame_alloc();
if (!ctx->delayed_frame) {
return AVERROR(ENOMEM);
}
+
// hardcoded to current HW queue size - will realloc in
timestamp_queue_enqueue() if too small
ctx->timestamp_list = av_fifo_alloc((avctx->max_b_frames + 16) *
sizeof(int64_t));
if (!ctx->timestamp_list) {
@@ -123,119 +99,9 @@ static int amf_load_library(AVCodecContext *avctx)
}
ctx->dts_delay = 0;
-
-ctx->library = dlopen(AMF_DLL_NAMEA, RTLD_NOW | RTLD_LOCAL);
-AMF_RETURN_IF_FALSE(ctx, ctx->library != NULL,
-AVERROR_UNKNOWN, "DLL %s failed to open\n", AMF_DLL_NAMEA);
-
-init_fun = (AMFInit_Fn)dlsym(ctx->library, AMF_INIT_FUNCTION_NAME);
-AMF_RETURN_IF_FALSE(ctx, init_fun != NULL, AVERROR_UNKNOWN, "DLL %s failed
to find function %s\n", AMF_DLL_NAMEA, AMF_INIT_FUNCTION_NAME);
-
-version_fun = (AMFQueryVersion_Fn)dlsym(ctx->library,
AMF_QUERY_VERSION_FUNCTION_NAME);
-AMF_RETURN_IF_FALSE(ctx, version_fun != NULL, AVERROR_UNKNOWN, "DLL %s
failed to find function %s\n", AMF_DLL_NAMEA, AMF_QUERY_VERSION_FUNCTION_NAME);
-
-res = version_fun(>version);
-AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "%s failed with
error %d\n", AMF_QUERY_VERSION_FUNCTION_NAME, res);
-res = init_fun(AMF_FULL_VERSION, >factory);
-AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "%s failed with
error %d\n", AMF_INIT_FUNCTION_NAME, res);
-res = ctx->factory->pVtbl->GetTrace(ctx->factory, >trace);
-AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "GetTrace()
failed with error %d\n", res);
-res = ctx->factory->pVtbl->GetDebug(ctx->factory, >debug);
-AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "GetDebug()
failed with error %d\n", res);
-return 0;
-}
-
-#if CONFIG_D3D11VA
-static int amf_init_from_d3d11_device(AVCodecContext *avctx,
AVD3D11VADeviceContext *hwctx)
-{
-AmfContext *ctx = avctx->priv_data;
-AMF_RESULT res;
-
-res = ctx->context->pVtbl->InitDX11(ctx->context, hwctx->device,
AMF_DX11_1);
-if (res != AMF_OK) {
-if (res == AMF_NOT_SUPPORTED)
-av_log(avctx, AV_LOG_ERROR, "AMF via D3D11 is not supported on the
given device.\n");
-
Re: [FFmpeg-devel] [PATCH] lavc/amfenc: moving amf common code (library and context) to lavu/hwcontext_amf from amfenc to be reused in other amf components
Hi Mark,
Thank you for your comments.
Could you see my comments bellow
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of Mark
> Thompson
> Sent: Sunday, May 13, 2018 1:41 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] lavc/amfenc: moving amf common code
> (library and context) to lavu/hwcontext_amf from
> amfenc to be reused in other amf components
>
> On 12/05/18 09:48, Alexander Kravchenko wrote:
> > This patch moves AMF common parts from amfenc to hwcontext_amf.
> > Now av_hwdevice_ctx API is used for AMF context creation/destroying.
> > This patch does not change component behaviour.
> > it contains only restructurization for further patches with new amf
> > components
> >
> > ---
> > Sending updated patch based on Mark's review
> > 1) simplificated library loading/unloading logic
> > 2) minor fixes
> >
> >
> > libavcodec/amfenc.c| 247
> > +---
> > libavcodec/amfenc.h| 27 +
> > libavutil/Makefile | 2 +
> > libavutil/hwcontext.c | 4 +
> > libavutil/hwcontext.h | 1 +
> > libavutil/hwcontext_amf.c | 252
> > +
> > libavutil/hwcontext_amf.h | 43 +++
> > libavutil/hwcontext_internal.h | 1 +
> > 8 files changed, 338 insertions(+), 239 deletions(-) create mode
> > 100644 libavutil/hwcontext_amf.c create mode 100644
> > libavutil/hwcontext_amf.h
> >
> > -
> > -static int amf_load_library(AVCodecContext *avctx)
> > +static int amf_init_context(AVCodecContext *avctx)
> > {
> > -AmfContext*ctx = avctx->priv_data;
> > -AMFInit_Fn init_fun;
> > -AMFQueryVersion_Fn version_fun;
> > -AMF_RESULT res;
> > +AmfContext *ctx = avctx->priv_data;
> > +AVAMFDeviceContext *amf_ctx;
> > +int ret;
> >
> > ctx->delayed_frame = av_frame_alloc();
> > if (!ctx->delayed_frame) {
> > return AVERROR(ENOMEM);
> > }
> > +
>
> Stray change?
>
amf_load_library was moved to hwcontext; code which is unrelated to library
moved to amf_init_context (delayed_frame object allocation)
> > +
> > +static void amf_device_uninit(AVHWDeviceContext *ctx) {
> > +AVAMFDeviceContext *amf_ctx = ctx->hwctx;
> > +AMFDeviceContextPrivate *priv = ctx->internal->priv;
> > +if (amf_ctx->context) {
> > +amf_ctx->context->pVtbl->Terminate(amf_ctx->context);
> > +amf_ctx->context->pVtbl->Release(amf_ctx->context);
> > +amf_ctx->context = NULL;
> > +}
> > +if(priv->library) {
> > +dlclose(priv->library);
> > +}
>
> This stuff shouldn't be in the uninit function, since this runs on all
> devices rather than just those created internally. You want to make
> a function to set as AVHWDeviceContext.free.
>
OK, will be fixed in coming patch
> > +#include "frame.h"
> > +#include "AMF/core/Context.h"
> > +#include "AMF/core/Factory.h"
> > +
> > +
> > +/**
> > + * This struct is allocated as AVHWDeviceContext.hwctx */ typedef
> > +struct AVAMFDeviceContext {
> > +AMFContext *context;
> > +AMFFactory *factory;
>
> Might be nice to have a bit more documentation than that.
>
I will add comments to header in coming patch
> > +} AVAMFDeviceContext;
> > +
> > +
Thanks,
Alexander
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc/amfenc: moving amf common code (library and context) to lavu/hwcontext_amf from amfenc to be reused in other amf components
On 12/05/18 09:48, Alexander Kravchenko wrote:
> This patch moves AMF common parts from amfenc to hwcontext_amf.
> Now av_hwdevice_ctx API is used for AMF context creation/destroying.
> This patch does not change component behaviour.
> it contains only restructurization for further patches with new amf components
>
> ---
> Sending updated patch based on Mark's review
> 1) simplificated library loading/unloading logic
> 2) minor fixes
>
>
> libavcodec/amfenc.c| 247 +---
> libavcodec/amfenc.h| 27 +
> libavutil/Makefile | 2 +
> libavutil/hwcontext.c | 4 +
> libavutil/hwcontext.h | 1 +
> libavutil/hwcontext_amf.c | 252
> +
> libavutil/hwcontext_amf.h | 43 +++
> libavutil/hwcontext_internal.h | 1 +
> 8 files changed, 338 insertions(+), 239 deletions(-)
> create mode 100644 libavutil/hwcontext_amf.c
> create mode 100644 libavutil/hwcontext_amf.h
>
> diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c
> index 384d8efc92..4c907ca3bc 100644
> --- a/libavcodec/amfenc.c
> +++ b/libavcodec/amfenc.c
> @@ -21,13 +21,7 @@
> #include "libavutil/avassert.h"
> #include "libavutil/imgutils.h"
> #include "libavutil/hwcontext.h"
> -#if CONFIG_D3D11VA
> -#include "libavutil/hwcontext_d3d11va.h"
> -#endif
> -#if CONFIG_DXVA2
> -#define COBJMACROS
> -#include "libavutil/hwcontext_dxva2.h"
> -#endif
> +
> #include "libavutil/mem.h"
> #include "libavutil/pixdesc.h"
> #include "libavutil/time.h"
> @@ -35,14 +29,12 @@
> #include "amfenc.h"
> #include "internal.h"
>
> -#if CONFIG_D3D11VA
> -#include
> +#if CONFIG_DXVA2
> +#include
> #endif
>
> -#ifdef _WIN32
> -#include "compat/w32dlfcn.h"
> -#else
> -#include
> +#if CONFIG_D3D11VA
> +#include
> #endif
>
> #define FFMPEG_AMF_WRITER_ID L"ffmpeg_amf"
> @@ -88,34 +80,18 @@ static enum AMF_SURFACE_FORMAT amf_av_to_amf_format(enum
> AVPixelFormat fmt)
> return AMF_SURFACE_UNKNOWN;
> }
>
> -static void AMF_CDECL_CALL AMFTraceWriter_Write(AMFTraceWriter *pThis,
> -const wchar_t *scope, const wchar_t *message)
> -{
> -AmfTraceWriter *tracer = (AmfTraceWriter*)pThis;
> -av_log(tracer->avctx, AV_LOG_DEBUG, "%ls: %ls", scope, message); // \n
> is provided from AMF
> -}
> -
> -static void AMF_CDECL_CALL AMFTraceWriter_Flush(AMFTraceWriter *pThis)
> -{
> -}
>
> -static AMFTraceWriterVtbl tracer_vtbl =
> -{
> -.Write = AMFTraceWriter_Write,
> -.Flush = AMFTraceWriter_Flush,
> -};
> -
> -static int amf_load_library(AVCodecContext *avctx)
> +static int amf_init_context(AVCodecContext *avctx)
> {
> -AmfContext*ctx = avctx->priv_data;
> -AMFInit_Fn init_fun;
> -AMFQueryVersion_Fn version_fun;
> -AMF_RESULT res;
> +AmfContext *ctx = avctx->priv_data;
> +AVAMFDeviceContext *amf_ctx;
> +int ret;
>
> ctx->delayed_frame = av_frame_alloc();
> if (!ctx->delayed_frame) {
> return AVERROR(ENOMEM);
> }
> +
Stray change?
> // hardcoded to current HW queue size - will realloc in
> timestamp_queue_enqueue() if too small
> ctx->timestamp_list = av_fifo_alloc((avctx->max_b_frames + 16) *
> sizeof(int64_t));
> if (!ctx->timestamp_list) {
> @@ -123,119 +99,9 @@ static int amf_load_library(AVCodecContext *avctx)
> }
> ctx->dts_delay = 0;
>
> -
> -ctx->library = dlopen(AMF_DLL_NAMEA, RTLD_NOW | RTLD_LOCAL);
> -AMF_RETURN_IF_FALSE(ctx, ctx->library != NULL,
> -AVERROR_UNKNOWN, "DLL %s failed to open\n", AMF_DLL_NAMEA);
> -
> -init_fun = (AMFInit_Fn)dlsym(ctx->library, AMF_INIT_FUNCTION_NAME);
> -AMF_RETURN_IF_FALSE(ctx, init_fun != NULL, AVERROR_UNKNOWN, "DLL %s
> failed to find function %s\n", AMF_DLL_NAMEA, AMF_INIT_FUNCTION_NAME);
> -
> -version_fun = (AMFQueryVersion_Fn)dlsym(ctx->library,
> AMF_QUERY_VERSION_FUNCTION_NAME);
> -AMF_RETURN_IF_FALSE(ctx, version_fun != NULL, AVERROR_UNKNOWN, "DLL %s
> failed to find function %s\n", AMF_DLL_NAMEA,
> AMF_QUERY_VERSION_FUNCTION_NAME);
> -
> -res = version_fun(>version);
> -AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "%s failed with
> error %d\n", AMF_QUERY_VERSION_FUNCTION_NAME, res);
> -res = init_fun(AMF_FULL_VERSION, >factory);
> -AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "%s failed with
> error %d\n", AMF_INIT_FUNCTION_NAME, res);
> -res = ctx->factory->pVtbl->GetTrace(ctx->factory, >trace);
> -AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "GetTrace()
> failed with error %d\n", res);
> -res = ctx->factory->pVtbl->GetDebug(ctx->factory, >debug);
> -AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "GetDebug()
> failed with error %d\n", res);
> -return 0;
> -}
> -
> -#if CONFIG_D3D11VA
> -static int amf_init_from_d3d11_device(AVCodecContext *avctx,
> AVD3D11VADeviceContext *hwctx)
> -{
> -AmfContext *ctx =
Re: [FFmpeg-devel] [PATCH] lavc/amfenc: moving amf common code (library and context) to lavu/hwcontext_amf from amfenc to be reused in other amf components
On 11/05/18 19:37, Alexander Kravchenko wrote:
> Hi Mark,
> Thank you for your comments.
> Could you see my comments and questions bellow
>
>> -Original Message-
>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of
>> Mark Thompson
>> Sent: Thursday, May 10, 2018 11:43 PM
>> To: ffmpeg-devel@ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH] lavc/amfenc: moving amf common code
>> (library and context) to lavu/hwcontext_amf from
>> amfenc to be reused in other amf components
>>
>
>>> +static AVBufferRef *amf_library_ctx = NULL;
>>
>> Global mutable state in libraries is strongly discouraged. Just loading it
>> again should be fine?
>>
>
> Yes, loading it again could be fine.
> I wanted to prevent additional call of library loading and trace
> initializations every time amf_device context is created.
> User can create many instances of encoder in one process.
> Storing amf_library_ctx allows to detect when cleanup functions can be called
> (UnregisterWriter...). Of course we can never call dclose and keep library
> loaded all time.
> What is your opinion?
Is there any later benefit to reusing the pointer, or is it just saving on the
load/initialisation code? If there isn't a strong reason to do so I would
avoid storing it. (Note that doing so would require more machinery to guard
against data races as well.)
>>> +
>>> +#include "frame.h"
>>> +#include "AMF/core/Context.h"
>>> +#include "AMF/core/Factory.h"
>>> +
>>> +
>>> +/**
>>> + * This struct is allocated as AVHWDeviceContext.hwctx */ typedef
>>> +struct AVAMFDeviceContext {
>>> +AMFContext *context;
>>> +AMFFactory *factory;
>>
>> Do you actually need both of these? It feels like you should be able to
>> derive the creating factory (/ library instance) from the context.
>>
>
> AMFContext does not have pointer to AMFFactory. Now AMFFactory ->
> CreateComponent is used in encoder initialization.
> May be AMFFactory is too wide interface on this level. Pointer to function
> like CreateComponent can be published in AVAMFDeviceContext instead of
> factory pointer.
> or did you mean something different?
Since this is ending up as immutable public API, we should be avoiding adding
anything which isn't absolutely needed. If they are both needed then that's
fine - I don't think including the CreateComponent pointer instead would be any
simpler.
> I have another question about publish amf library level options (Trace level,
> trace writers...). This could be in another patch if we decide this option is
> possible.
> What is the best way to add such option? Can command line options be used to
> configure library? Or somehow publish this API?
Maybe use the opts dict passed to av_hwdevice_ctx_create()? (They're
accessible in the ffmpeg utility via -init_hw_device.)
- Mark
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavc/amfenc: moving amf common code (library and context) to lavu/hwcontext_amf from amfenc to be reused in other amf components
This patch moves AMF common parts from amfenc to hwcontext_amf.
Now av_hwdevice_ctx API is used for AMF context creation/destroying.
This patch does not change component behaviour.
it contains only restructurization for further patches with new amf components
---
Sending updated patch based on Mark's review
1) simplificated library loading/unloading logic
2) minor fixes
libavcodec/amfenc.c| 247 +---
libavcodec/amfenc.h| 27 +
libavutil/Makefile | 2 +
libavutil/hwcontext.c | 4 +
libavutil/hwcontext.h | 1 +
libavutil/hwcontext_amf.c | 252 +
libavutil/hwcontext_amf.h | 43 +++
libavutil/hwcontext_internal.h | 1 +
8 files changed, 338 insertions(+), 239 deletions(-)
create mode 100644 libavutil/hwcontext_amf.c
create mode 100644 libavutil/hwcontext_amf.h
diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c
index 384d8efc92..4c907ca3bc 100644
--- a/libavcodec/amfenc.c
+++ b/libavcodec/amfenc.c
@@ -21,13 +21,7 @@
#include "libavutil/avassert.h"
#include "libavutil/imgutils.h"
#include "libavutil/hwcontext.h"
-#if CONFIG_D3D11VA
-#include "libavutil/hwcontext_d3d11va.h"
-#endif
-#if CONFIG_DXVA2
-#define COBJMACROS
-#include "libavutil/hwcontext_dxva2.h"
-#endif
+
#include "libavutil/mem.h"
#include "libavutil/pixdesc.h"
#include "libavutil/time.h"
@@ -35,14 +29,12 @@
#include "amfenc.h"
#include "internal.h"
-#if CONFIG_D3D11VA
-#include
+#if CONFIG_DXVA2
+#include
#endif
-#ifdef _WIN32
-#include "compat/w32dlfcn.h"
-#else
-#include
+#if CONFIG_D3D11VA
+#include
#endif
#define FFMPEG_AMF_WRITER_ID L"ffmpeg_amf"
@@ -88,34 +80,18 @@ static enum AMF_SURFACE_FORMAT amf_av_to_amf_format(enum
AVPixelFormat fmt)
return AMF_SURFACE_UNKNOWN;
}
-static void AMF_CDECL_CALL AMFTraceWriter_Write(AMFTraceWriter *pThis,
-const wchar_t *scope, const wchar_t *message)
-{
-AmfTraceWriter *tracer = (AmfTraceWriter*)pThis;
-av_log(tracer->avctx, AV_LOG_DEBUG, "%ls: %ls", scope, message); // \n is
provided from AMF
-}
-
-static void AMF_CDECL_CALL AMFTraceWriter_Flush(AMFTraceWriter *pThis)
-{
-}
-static AMFTraceWriterVtbl tracer_vtbl =
-{
-.Write = AMFTraceWriter_Write,
-.Flush = AMFTraceWriter_Flush,
-};
-
-static int amf_load_library(AVCodecContext *avctx)
+static int amf_init_context(AVCodecContext *avctx)
{
-AmfContext*ctx = avctx->priv_data;
-AMFInit_Fn init_fun;
-AMFQueryVersion_Fn version_fun;
-AMF_RESULT res;
+AmfContext *ctx = avctx->priv_data;
+AVAMFDeviceContext *amf_ctx;
+int ret;
ctx->delayed_frame = av_frame_alloc();
if (!ctx->delayed_frame) {
return AVERROR(ENOMEM);
}
+
// hardcoded to current HW queue size - will realloc in
timestamp_queue_enqueue() if too small
ctx->timestamp_list = av_fifo_alloc((avctx->max_b_frames + 16) *
sizeof(int64_t));
if (!ctx->timestamp_list) {
@@ -123,119 +99,9 @@ static int amf_load_library(AVCodecContext *avctx)
}
ctx->dts_delay = 0;
-
-ctx->library = dlopen(AMF_DLL_NAMEA, RTLD_NOW | RTLD_LOCAL);
-AMF_RETURN_IF_FALSE(ctx, ctx->library != NULL,
-AVERROR_UNKNOWN, "DLL %s failed to open\n", AMF_DLL_NAMEA);
-
-init_fun = (AMFInit_Fn)dlsym(ctx->library, AMF_INIT_FUNCTION_NAME);
-AMF_RETURN_IF_FALSE(ctx, init_fun != NULL, AVERROR_UNKNOWN, "DLL %s failed
to find function %s\n", AMF_DLL_NAMEA, AMF_INIT_FUNCTION_NAME);
-
-version_fun = (AMFQueryVersion_Fn)dlsym(ctx->library,
AMF_QUERY_VERSION_FUNCTION_NAME);
-AMF_RETURN_IF_FALSE(ctx, version_fun != NULL, AVERROR_UNKNOWN, "DLL %s
failed to find function %s\n", AMF_DLL_NAMEA, AMF_QUERY_VERSION_FUNCTION_NAME);
-
-res = version_fun(>version);
-AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "%s failed with
error %d\n", AMF_QUERY_VERSION_FUNCTION_NAME, res);
-res = init_fun(AMF_FULL_VERSION, >factory);
-AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "%s failed with
error %d\n", AMF_INIT_FUNCTION_NAME, res);
-res = ctx->factory->pVtbl->GetTrace(ctx->factory, >trace);
-AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "GetTrace()
failed with error %d\n", res);
-res = ctx->factory->pVtbl->GetDebug(ctx->factory, >debug);
-AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "GetDebug()
failed with error %d\n", res);
-return 0;
-}
-
-#if CONFIG_D3D11VA
-static int amf_init_from_d3d11_device(AVCodecContext *avctx,
AVD3D11VADeviceContext *hwctx)
-{
-AmfContext *ctx = avctx->priv_data;
-AMF_RESULT res;
-
-res = ctx->context->pVtbl->InitDX11(ctx->context, hwctx->device,
AMF_DX11_1);
-if (res != AMF_OK) {
-if (res == AMF_NOT_SUPPORTED)
-av_log(avctx, AV_LOG_ERROR, "AMF via D3D11 is not supported on the
given device.\n");
-else
-av_log(avctx, AV_LOG_ERROR, "AMF failed
Re: [FFmpeg-devel] [PATCH] lavc/amfenc: moving amf common code (library and context) to lavu/hwcontext_amf from amfenc to be reused in other amf components
Hi Mark,
Thank you for your comments.
Could you see my comments and questions bellow
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of Mark
> Thompson
> Sent: Thursday, May 10, 2018 11:43 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] lavc/amfenc: moving amf common code
> (library and context) to lavu/hwcontext_amf from
> amfenc to be reused in other amf components
>
> > +static AVBufferRef *amf_library_ctx = NULL;
>
> Global mutable state in libraries is strongly discouraged. Just loading it
> again should be fine?
>
Yes, loading it again could be fine.
I wanted to prevent additional call of library loading and trace
initializations every time amf_device context is created.
User can create many instances of encoder in one process.
Storing amf_library_ctx allows to detect when cleanup functions can be called
(UnregisterWriter...). Of course we can never call dclose and keep library
loaded all time.
What is your opinion?
> > +
> > +#include "frame.h"
> > +#include "AMF/core/Context.h"
> > +#include "AMF/core/Factory.h"
> > +
> > +
> > +/**
> > + * This struct is allocated as AVHWDeviceContext.hwctx */ typedef
> > +struct AVAMFDeviceContext {
> > +AMFContext *context;
> > +AMFFactory *factory;
>
> Do you actually need both of these? It feels like you should be able to
> derive the creating factory (/ library instance) from the context.
>
AMFContext does not have pointer to AMFFactory. Now AMFFactory ->
CreateComponent is used in encoder initialization.
May be AMFFactory is too wide interface on this level. Pointer to function like
CreateComponent can be published in AVAMFDeviceContext instead of factory
pointer.
or did you mean something different?
--
I have another question about publish amf library level options (Trace level,
trace writers...). This could be in another patch if we decide this option is
possible.
What is the best way to add such option? Can command line options be used to
configure library? Or somehow publish this API?
Thanks,
Alexander
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavc/amfenc: moving amf common code (library and context) to lavu/hwcontext_amf from amfenc to be reused in other amf components
On 08/05/18 12:00, Alexander Kravchenko wrote:
> This patch moves AMF common parts from amfenc to hwcontext_amf.
> Now av_hwdevice_ctx API is used for AMF context creation/destroying.
> This patch does not change component behaviour.
> it contains only restructurization for further patches with new amf components
>
> ---
> libavcodec/amfenc.c| 250 ---
> libavcodec/amfenc.h| 27 +---
> libavutil/Makefile | 2 +
> libavutil/hwcontext.c | 4 +
> libavutil/hwcontext.h | 1 +
> libavutil/hwcontext_amf.c | 329
> +
> libavutil/hwcontext_amf.h | 43 ++
> libavutil/hwcontext_internal.h | 1 +
> 8 files changed, 418 insertions(+), 239 deletions(-)
> create mode 100644 libavutil/hwcontext_amf.c
> create mode 100644 libavutil/hwcontext_amf.h
>
> diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c
> index 384d8efc92..a8186c7b86 100644
> --- a/libavcodec/amfenc.c
> +++ b/libavcodec/amfenc.c
> @@ -21,13 +21,7 @@
> #include "libavutil/avassert.h"
> #include "libavutil/imgutils.h"
> #include "libavutil/hwcontext.h"
> -#if CONFIG_D3D11VA
> -#include "libavutil/hwcontext_d3d11va.h"
> -#endif
> -#if CONFIG_DXVA2
> -#define COBJMACROS
> -#include "libavutil/hwcontext_dxva2.h"
> -#endif
> +
> #include "libavutil/mem.h"
> #include "libavutil/pixdesc.h"
> #include "libavutil/time.h"
> @@ -35,14 +29,12 @@
> #include "amfenc.h"
> #include "internal.h"
>
> -#if CONFIG_D3D11VA
> -#include
> +#if CONFIG_DXVA2
> +#include
> #endif
>
> -#ifdef _WIN32
> -#include "compat/w32dlfcn.h"
> -#else
> -#include
> +#if CONFIG_D3D11VA
> +#include
> #endif
>
> #define FFMPEG_AMF_WRITER_ID L"ffmpeg_amf"
> @@ -88,34 +80,17 @@ static enum AMF_SURFACE_FORMAT amf_av_to_amf_format(enum
> AVPixelFormat fmt)
> return AMF_SURFACE_UNKNOWN;
> }
>
> -static void AMF_CDECL_CALL AMFTraceWriter_Write(AMFTraceWriter *pThis,
> -const wchar_t *scope, const wchar_t *message)
> -{
> -AmfTraceWriter *tracer = (AmfTraceWriter*)pThis;
> -av_log(tracer->avctx, AV_LOG_DEBUG, "%ls: %ls", scope, message); // \n
> is provided from AMF
> -}
>
> -static void AMF_CDECL_CALL AMFTraceWriter_Flush(AMFTraceWriter *pThis)
> -{
> -}
> -
> -static AMFTraceWriterVtbl tracer_vtbl =
> -{
> -.Write = AMFTraceWriter_Write,
> -.Flush = AMFTraceWriter_Flush,
> -};
> -
> -static int amf_load_library(AVCodecContext *avctx)
> +static int amf_init_context(AVCodecContext *avctx)
> {
> -AmfContext*ctx = avctx->priv_data;
> -AMFInit_Fn init_fun;
> -AMFQueryVersion_Fn version_fun;
> -AMF_RESULT res;
> +AmfContext *ctx = avctx->priv_data;
> +av_unused int ret;
This is now used in every configuration, so it doesn't need the annotation.
>
> ctx->delayed_frame = av_frame_alloc();
> if (!ctx->delayed_frame) {
> return AVERROR(ENOMEM);
> }
> +
Cosmetic change, and it's also trailing whitespace.
> // hardcoded to current HW queue size - will realloc in
> timestamp_queue_enqueue() if too small
> ctx->timestamp_list = av_fifo_alloc((avctx->max_b_frames + 16) *
> sizeof(int64_t));
> if (!ctx->timestamp_list) {
> @@ -123,119 +98,9 @@ static int amf_load_library(AVCodecContext *avctx)
> }
> ctx->dts_delay = 0;
>
> -
> -ctx->library = dlopen(AMF_DLL_NAMEA, RTLD_NOW | RTLD_LOCAL);
> -AMF_RETURN_IF_FALSE(ctx, ctx->library != NULL,
> -AVERROR_UNKNOWN, "DLL %s failed to open\n", AMF_DLL_NAMEA);
> -
> -init_fun = (AMFInit_Fn)dlsym(ctx->library, AMF_INIT_FUNCTION_NAME);
> -AMF_RETURN_IF_FALSE(ctx, init_fun != NULL, AVERROR_UNKNOWN, "DLL %s
> failed to find function %s\n", AMF_DLL_NAMEA, AMF_INIT_FUNCTION_NAME);
> -
> -version_fun = (AMFQueryVersion_Fn)dlsym(ctx->library,
> AMF_QUERY_VERSION_FUNCTION_NAME);
> -AMF_RETURN_IF_FALSE(ctx, version_fun != NULL, AVERROR_UNKNOWN, "DLL %s
> failed to find function %s\n", AMF_DLL_NAMEA,
> AMF_QUERY_VERSION_FUNCTION_NAME);
> -
> -res = version_fun(>version);
> -AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "%s failed with
> error %d\n", AMF_QUERY_VERSION_FUNCTION_NAME, res);
> -res = init_fun(AMF_FULL_VERSION, >factory);
> -AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "%s failed with
> error %d\n", AMF_INIT_FUNCTION_NAME, res);
> -res = ctx->factory->pVtbl->GetTrace(ctx->factory, >trace);
> -AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "GetTrace()
> failed with error %d\n", res);
> -res = ctx->factory->pVtbl->GetDebug(ctx->factory, >debug);
> -AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "GetDebug()
> failed with error %d\n", res);
> -return 0;
> -}
> -
> -#if CONFIG_D3D11VA
> -static int amf_init_from_d3d11_device(AVCodecContext *avctx,
> AVD3D11VADeviceContext *hwctx)
> -{
> -AmfContext *ctx = avctx->priv_data;
> -AMF_RESULT res;
>
[FFmpeg-devel] [PATCH] lavc/amfenc: moving amf common code (library and context) to lavu/hwcontext_amf from amfenc to be reused in other amf components
This patch moves AMF common parts from amfenc to hwcontext_amf.
Now av_hwdevice_ctx API is used for AMF context creation/destroying.
This patch does not change component behaviour.
it contains only restructurization for further patches with new amf components
---
sending patch one more time in May, since April's one wasn't commented/pushed.
previous April's link:
http://ffmpeg.org/pipermail/ffmpeg-devel/2018-April/229051.html
libavcodec/amfenc.c| 250 ---
libavcodec/amfenc.h| 27 +---
libavutil/Makefile | 2 +
libavutil/hwcontext.c | 4 +
libavutil/hwcontext.h | 1 +
libavutil/hwcontext_amf.c | 329 +
libavutil/hwcontext_amf.h | 43 ++
libavutil/hwcontext_internal.h | 1 +
8 files changed, 418 insertions(+), 239 deletions(-)
create mode 100644 libavutil/hwcontext_amf.c
create mode 100644 libavutil/hwcontext_amf.h
diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c
index 384d8efc92..a8186c7b86 100644
--- a/libavcodec/amfenc.c
+++ b/libavcodec/amfenc.c
@@ -21,13 +21,7 @@
#include "libavutil/avassert.h"
#include "libavutil/imgutils.h"
#include "libavutil/hwcontext.h"
-#if CONFIG_D3D11VA
-#include "libavutil/hwcontext_d3d11va.h"
-#endif
-#if CONFIG_DXVA2
-#define COBJMACROS
-#include "libavutil/hwcontext_dxva2.h"
-#endif
+
#include "libavutil/mem.h"
#include "libavutil/pixdesc.h"
#include "libavutil/time.h"
@@ -35,14 +29,12 @@
#include "amfenc.h"
#include "internal.h"
-#if CONFIG_D3D11VA
-#include
+#if CONFIG_DXVA2
+#include
#endif
-#ifdef _WIN32
-#include "compat/w32dlfcn.h"
-#else
-#include
+#if CONFIG_D3D11VA
+#include
#endif
#define FFMPEG_AMF_WRITER_ID L"ffmpeg_amf"
@@ -88,34 +80,17 @@ static enum AMF_SURFACE_FORMAT amf_av_to_amf_format(enum
AVPixelFormat fmt)
return AMF_SURFACE_UNKNOWN;
}
-static void AMF_CDECL_CALL AMFTraceWriter_Write(AMFTraceWriter *pThis,
-const wchar_t *scope, const wchar_t *message)
-{
-AmfTraceWriter *tracer = (AmfTraceWriter*)pThis;
-av_log(tracer->avctx, AV_LOG_DEBUG, "%ls: %ls", scope, message); // \n is
provided from AMF
-}
-static void AMF_CDECL_CALL AMFTraceWriter_Flush(AMFTraceWriter *pThis)
-{
-}
-
-static AMFTraceWriterVtbl tracer_vtbl =
-{
-.Write = AMFTraceWriter_Write,
-.Flush = AMFTraceWriter_Flush,
-};
-
-static int amf_load_library(AVCodecContext *avctx)
+static int amf_init_context(AVCodecContext *avctx)
{
-AmfContext*ctx = avctx->priv_data;
-AMFInit_Fn init_fun;
-AMFQueryVersion_Fn version_fun;
-AMF_RESULT res;
+AmfContext *ctx = avctx->priv_data;
+av_unused int ret;
ctx->delayed_frame = av_frame_alloc();
if (!ctx->delayed_frame) {
return AVERROR(ENOMEM);
}
+
// hardcoded to current HW queue size - will realloc in
timestamp_queue_enqueue() if too small
ctx->timestamp_list = av_fifo_alloc((avctx->max_b_frames + 16) *
sizeof(int64_t));
if (!ctx->timestamp_list) {
@@ -123,119 +98,9 @@ static int amf_load_library(AVCodecContext *avctx)
}
ctx->dts_delay = 0;
-
-ctx->library = dlopen(AMF_DLL_NAMEA, RTLD_NOW | RTLD_LOCAL);
-AMF_RETURN_IF_FALSE(ctx, ctx->library != NULL,
-AVERROR_UNKNOWN, "DLL %s failed to open\n", AMF_DLL_NAMEA);
-
-init_fun = (AMFInit_Fn)dlsym(ctx->library, AMF_INIT_FUNCTION_NAME);
-AMF_RETURN_IF_FALSE(ctx, init_fun != NULL, AVERROR_UNKNOWN, "DLL %s failed
to find function %s\n", AMF_DLL_NAMEA, AMF_INIT_FUNCTION_NAME);
-
-version_fun = (AMFQueryVersion_Fn)dlsym(ctx->library,
AMF_QUERY_VERSION_FUNCTION_NAME);
-AMF_RETURN_IF_FALSE(ctx, version_fun != NULL, AVERROR_UNKNOWN, "DLL %s
failed to find function %s\n", AMF_DLL_NAMEA, AMF_QUERY_VERSION_FUNCTION_NAME);
-
-res = version_fun(>version);
-AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "%s failed with
error %d\n", AMF_QUERY_VERSION_FUNCTION_NAME, res);
-res = init_fun(AMF_FULL_VERSION, >factory);
-AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "%s failed with
error %d\n", AMF_INIT_FUNCTION_NAME, res);
-res = ctx->factory->pVtbl->GetTrace(ctx->factory, >trace);
-AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "GetTrace()
failed with error %d\n", res);
-res = ctx->factory->pVtbl->GetDebug(ctx->factory, >debug);
-AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "GetDebug()
failed with error %d\n", res);
-return 0;
-}
-
-#if CONFIG_D3D11VA
-static int amf_init_from_d3d11_device(AVCodecContext *avctx,
AVD3D11VADeviceContext *hwctx)
-{
-AmfContext *ctx = avctx->priv_data;
-AMF_RESULT res;
-
-res = ctx->context->pVtbl->InitDX11(ctx->context, hwctx->device,
AMF_DX11_1);
-if (res != AMF_OK) {
-if (res == AMF_NOT_SUPPORTED)
-av_log(avctx, AV_LOG_ERROR, "AMF via D3D11 is not supported on the
given device.\n");
-else
-av_log(avctx,
