[FFmpeg-devel] [PATCH v2] avcodec/amfenc: increase precision of Sleep() on Windows

[Mark Thompson]

On 13/11/2023 14:37, Evgeny Pavlov wrote:
> This commit increase precision of Sleep() function on Windows.
> This fix reduces the sleep time on Windows to improve AMF encoding
> performance on low resolution input videos.
> 
> Fix for issue #10622
> 
> v2: use timeBeginPeriod/timeEndPeriod for increasing precision of Sleep()
> 
> Signed-off-by: Evgeny Pavlov <lucenticus at gmail.com>
> ---
>   libavcodec/amfenc.c | 31 +++++++++++++++++++++++++++++++
>   libavcodec/amfenc.h |  3 +++
>   2 files changed, 34 insertions(+)
> 
> diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c
> index 061859f85c..55e24856e8 100644
> --- a/libavcodec/amfenc.c
> +++ b/libavcodec/amfenc.c
> @@ -42,7 +42,12 @@
>   #endif
>   
>   #ifdef _WIN32
> +#include <timeapi.h>
>   #include "compat/w32dlfcn.h"
> +
> +typedef MMRESULT (*timeapi_fun)(UINT uPeriod);
> +#define WINMM_DLL "winmm.dll"
> +
>   #else
>   #include <dlfcn.h>
>   #endif
> @@ -113,6 +118,9 @@ static int amf_load_library(AVCodecContext *avctx)
>       AMFInit_Fn         init_fun;
>       AMFQueryVersion_Fn version_fun;
>       AMF_RESULT         res;
> +#ifdef _WIN32
> +    timeapi_fun time_begin_fun;
> +#endif
>   
>       ctx->delayed_frame = av_frame_alloc();
>       if (!ctx->delayed_frame) {
> @@ -145,6 +153,16 @@ static int amf_load_library(AVCodecContext *avctx)
>       AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "GetTrace() failed with error %d\n", res);
>       res = ctx->factory->pVtbl->GetDebug(ctx->factory, &ctx->debug);
>       AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "GetDebug() failed with error %d\n", res);
> +
> +#ifdef _WIN32
> +    // Increase precision of Sleep() function on Windows platform
> +    ctx->winmm_lib = dlopen(WINMM_DLL, RTLD_NOW | RTLD_LOCAL);
> +    AMF_RETURN_IF_FALSE(ctx, ctx->winmm_lib != NULL, 0, "DLL %s failed to open\n", WINMM_DLL);
> +    time_begin_fun = (timeapi_fun)dlsym(ctx->winmm_lib, "timeBeginPeriod");
> +    AMF_RETURN_IF_FALSE(ctx, time_begin_fun != NULL, 0, "DLL %s failed to find function %s\n", WINMM_DLL, "timeBeginPeriod");
> +    time_begin_fun(1);
> +#endif //_WIN32
> +
>       return 0;
>   }
>   
> @@ -375,6 +393,9 @@ static int amf_init_encoder(AVCodecContext *avctx)
>   int av_cold ff_amf_encode_close(AVCodecContext *avctx)
>   {
>       AmfContext *ctx = avctx->priv_data;
> +#ifdef _WIN32
> +    timeapi_fun time_end_fun;
> +#endif //_WIN32
>   
>       if (ctx->delayed_surface) {
>           ctx->delayed_surface->pVtbl->Release(ctx->delayed_surface);
> @@ -410,6 +431,16 @@ int av_cold ff_amf_encode_close(AVCodecContext *avctx)
>       av_frame_free(&ctx->delayed_frame);
>       av_fifo_freep2(&ctx->timestamp_list);
>   
> +#ifdef _WIN32
> +    if (ctx->winmm_lib) {
> +        time_end_fun = (timeapi_fun)dlsym(ctx->winmm_lib, "timeEndPeriod");
> +        AMF_RETURN_IF_FALSE(ctx, time_end_fun != NULL, 0, "DLL %s failed to find function %s\n", WINMM_DLL, "timeEndPeriod");
> +        time_end_fun(1);
> +        dlclose(ctx->winmm_lib);
> +        ctx->winmm_lib = NULL;
> +    }
> +#endif //_WIN32
> +
>       return 0;
>   }
>   
> diff --git a/libavcodec/amfenc.h b/libavcodec/amfenc.h
> index 2dbd378ef8..35bcf1dfe3 100644
> --- a/libavcodec/amfenc.h
> +++ b/libavcodec/amfenc.h
> @@ -50,6 +50,9 @@ typedef struct AmfContext {
>       AVClass            *avclass;
>       // access to AMF runtime
>       amf_handle          library; ///< handle to DLL library
> +#ifdef _WIN32
> +    amf_handle          winmm_lib; ///< handle to winmm DLL library
> +#endif //_WIN32
>       AMFFactory         *factory; ///< pointer to AMF factory
>       AMFDebug           *debug;   ///< pointer to AMF debug interface
>       AMFTrace           *trace;   ///< pointer to AMF trace interface

Is it reasonable to set this global state from a library without the parent program knowing?  We'd really prefer not to affect the global state unexpectedly.

It's also unclear to me what the effect of this tradeoff on power is, given that the whole reason why this happens is that Windows is trying to keep the CPU asleep for as long as possible to save power.

Thanks,

- Mark