diff mbox series

[FFmpeg-devel,1/3] nvenc: use runtime api version to support old drivers

Message ID 20200715143402.18582-1-wbsecg1@gmail.com
State New
Headers show
Series [FFmpeg-devel,1/3] nvenc: use runtime api version to support old drivers | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Wang Bin July 15, 2020, 2:34 p.m. UTC 
From: wang-bin <wbsecg1@gmail.com>

There are reserved bit fields in nvEncoderAPI.h structs, so driver's abi
is stable. Requesting runtime version of these structs should work. I've
compared 7.0~9.x headers, and confirmed on some devices.
---
 libavcodec/nvenc.c | 72 ++++++++++++++++++++++++++--------------------
 libavcodec/nvenc.h |  3 ++
 2 files changed, 44 insertions(+), 31 deletions(-)

Comments

Timo Rothenpieler July 15, 2020, 3:15 p.m. UTC | #1 
On 15.07.2020 16:34, wangbin wrote:
> From: wang-bin <wbsecg1@gmail.com>
> 
> There are reserved bit fields in nvEncoderAPI.h structs, so driver's abi
> is stable. Requesting runtime version of these structs should work. I've
> compared 7.0~9.x headers, and confirmed on some devices.
> ---
>   libavcodec/nvenc.c | 72 ++++++++++++++++++++++++++--------------------
>   libavcodec/nvenc.h |  3 ++
>   2 files changed, 44 insertions(+), 31 deletions(-)
> 
> diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
> index c6740c1842..ac35cb9f48 100644
> --- a/libavcodec/nvenc.c
> +++ b/libavcodec/nvenc.c
> @@ -193,14 +193,26 @@ static void nvenc_print_driver_requirement(AVCodecContext *avctx, int level)
>       av_log(avctx, level, "The minimum required Nvidia driver for nvenc is %s or newer\n", minver);
>   }
>   
> +static inline uint32_t struct_ver_rt(NvencContext* ctx, uint32_t struct_ver)
> +{
> +    return ((uint32_t)ctx->apiver_rt | ((struct_ver)<<16) | (0x7 << 28));
> +}

I'm really not a fan of hard-coding magic numbers, that can change in 
the future, into the ffmpeg implementation.

> +static inline uint32_t api_ver(uint32_t major_ver, uint32_t minor_ver)
> +{
> +    return major_ver | (minor_ver << 24);
> +}
> +
>   static av_cold int nvenc_load_libraries(AVCodecContext *avctx)
>   {
>       NvencContext *ctx            = avctx->priv_data;
>       NvencDynLoadFunctions *dl_fn = &ctx->nvenc_dload_funcs;
>       NVENCSTATUS err;
>       uint32_t nvenc_max_ver;
> +    uint32_t nvenc_max_major;
> +    uint32_t nvenc_max_minor;
> +    uint32_t func_ver = NV_ENCODE_API_FUNCTION_LIST_VER;
>       int ret;
> -
>       ret = cuda_load_functions(&dl_fn->cuda_dl, avctx);
>       if (ret < 0)
>           return ret;
> @@ -214,19 +226,17 @@ static av_cold int nvenc_load_libraries(AVCodecContext *avctx)
>       err = dl_fn->nvenc_dl->NvEncodeAPIGetMaxSupportedVersion(&nvenc_max_ver);
>       if (err != NV_ENC_SUCCESS)
>           return nvenc_print_error(avctx, err, "Failed to query nvenc max version");
> +    nvenc_max_major = nvenc_max_ver >> 4;
> +    nvenc_max_minor = nvenc_max_ver & 0xf;
> +    av_log(avctx, AV_LOG_VERBOSE, "nvenc build version: %d.%d, runtime version: %d.%d\n", NVENCAPI_MAJOR_VERSION, NVENCAPI_MINOR_VERSION, nvenc_max_major, nvenc_max_minor);
>   
> -    av_log(avctx, AV_LOG_VERBOSE, "Loaded Nvenc version %d.%d\n", nvenc_max_ver >> 4, nvenc_max_ver & 0xf);
> -
> -    if ((NVENCAPI_MAJOR_VERSION << 4 | NVENCAPI_MINOR_VERSION) > nvenc_max_ver) {
> -        av_log(avctx, AV_LOG_ERROR, "Driver does not support the required nvenc API version. "
> -               "Required: %d.%d Found: %d.%d\n",
> -               NVENCAPI_MAJOR_VERSION, NVENCAPI_MINOR_VERSION,
> -               nvenc_max_ver >> 4, nvenc_max_ver & 0xf);
> -        nvenc_print_driver_requirement(avctx, AV_LOG_ERROR);
> -        return AVERROR(ENOSYS);
> -    }
> +    ctx->apiver_rt = api_ver(nvenc_max_major, nvenc_max_minor); /* NVENCAPI_VERSION */
> +    ctx->config_ver_rt = struct_ver_rt(ctx, 7) | (1<<31); /* NV_ENC_CONFIG_VER */
> +    if (ctx->apiver_rt < api_ver(8, 1))
> +        ctx->config_ver_rt = struct_ver_rt(ctx, 6) | (1<<31);
> +    func_ver = struct_ver_rt(ctx, 2);
>   
> -    dl_fn->nvenc_funcs.version = NV_ENCODE_API_FUNCTION_LIST_VER;
> +    dl_fn->nvenc_funcs.version = func_ver;
>   
>       err = dl_fn->nvenc_dl->NvEncodeAPICreateInstance(&dl_fn->nvenc_funcs);
>       if (err != NV_ENC_SUCCESS)
> @@ -267,8 +277,8 @@ static av_cold int nvenc_open_session(AVCodecContext *avctx)
>       NV_ENCODE_API_FUNCTION_LIST *p_nvenc = &ctx->nvenc_dload_funcs.nvenc_funcs;
>       NVENCSTATUS ret;
>   
> -    params.version    = NV_ENC_OPEN_ENCODE_SESSION_EX_PARAMS_VER;
> -    params.apiVersion = NVENCAPI_VERSION;
> +    params.version    = struct_ver_rt(ctx, 1); // NV_ENC_OPEN_ENCODE_SESSION_EX_PARAMS_VER
> +    params.apiVersion = ctx->apiver_rt;
>       if (ctx->d3d11_device) {
>           params.device     = ctx->d3d11_device;
>           params.deviceType = NV_ENC_DEVICE_TYPE_DIRECTX;
> @@ -329,7 +339,7 @@ static int nvenc_check_cap(AVCodecContext *avctx, NV_ENC_CAPS cap)
>       NV_ENC_CAPS_PARAM params        = { 0 };
>       int ret, val = 0;
>   
> -    params.version     = NV_ENC_CAPS_PARAM_VER;
> +    params.version     = struct_ver_rt(ctx, 1); // NV_ENC_CAPS_PARAM_VER;
>       params.capsToQuery = cap;
>   
>       ret = p_nvenc->nvEncGetEncodeCaps(ctx->nvencoder, ctx->init_encode_params.encodeGUID, &params, &val);
> @@ -688,7 +698,7 @@ static av_cold void set_constqp(AVCodecContext *avctx)
>       NV_ENC_RC_PARAMS *rc = &ctx->encode_config.rcParams;
>   
>       rc->rateControlMode = NV_ENC_PARAMS_RC_CONSTQP;
> -
> +    /*rc->reservedBitField1 = 0;*/
>       if (ctx->init_qp_p >= 0) {
>           rc->constQP.qpInterP = ctx->init_qp_p;
>           if (ctx->init_qp_i >= 0 && ctx->init_qp_b >= 0) {
> @@ -1221,8 +1231,8 @@ static av_cold int nvenc_setup_encoder(AVCodecContext *avctx)
>       int res = 0;
>       int dw, dh;
>   
> -    ctx->encode_config.version = NV_ENC_CONFIG_VER;
> -    ctx->init_encode_params.version = NV_ENC_INITIALIZE_PARAMS_VER;
> +    ctx->encode_config.version = ctx->config_ver_rt;//NV_ENC_CONFIG_VER;
> +    ctx->init_encode_params.version = struct_ver_rt(ctx, 5) | (1<<31);//NV_ENC_INITIALIZE_PARAMS_VER;

More magic numbers.

>   
>       ctx->init_encode_params.encodeHeight = avctx->height;
>       ctx->init_encode_params.encodeWidth = avctx->width;
> @@ -1231,8 +1241,8 @@ static av_cold int nvenc_setup_encoder(AVCodecContext *avctx)
>   
>       nvenc_map_preset(ctx);
>   
> -    preset_config.version = NV_ENC_PRESET_CONFIG_VER;
> -    preset_config.presetCfg.version = NV_ENC_CONFIG_VER;
> +    preset_config.version = struct_ver_rt(ctx, 4) | (1<<31);// NV_ENC_PRESET_CONFIG_VER;

Yet more of them.

> +    preset_config.presetCfg.version = ctx->config_ver_rt;//NV_ENC_CONFIG_VER;
>   
>       if (IS_SDK10_PRESET(ctx->preset)) {
>   #ifdef NVENC_HAVE_NEW_PRESETS
> @@ -1260,7 +1270,7 @@ static av_cold int nvenc_setup_encoder(AVCodecContext *avctx)
>   
>       memcpy(&ctx->encode_config, &preset_config.presetCfg, sizeof(ctx->encode_config));
>   
> -    ctx->encode_config.version = NV_ENC_CONFIG_VER;
> +    ctx->encode_config.version = ctx->config_ver_rt;//NV_ENC_CONFIG_VER;
>   
>       compute_dar(avctx, &dw, &dh);
>       ctx->init_encode_params.darHeight = dh;
> @@ -1404,7 +1414,7 @@ static av_cold int nvenc_alloc_surface(AVCodecContext *avctx, int idx)
>   
>       NVENCSTATUS nv_status;
>       NV_ENC_CREATE_BITSTREAM_BUFFER allocOut = { 0 };
> -    allocOut.version = NV_ENC_CREATE_BITSTREAM_BUFFER_VER;
> +    allocOut.version = struct_ver_rt(ctx, 1);//NV_ENC_CREATE_BITSTREAM_BUFFER_VER;
>   
>       if (avctx->pix_fmt == AV_PIX_FMT_CUDA || avctx->pix_fmt == AV_PIX_FMT_D3D11) {
>           ctx->surfaces[idx].in_ref = av_frame_alloc();
> @@ -1420,7 +1430,7 @@ static av_cold int nvenc_alloc_surface(AVCodecContext *avctx, int idx)
>               return AVERROR(EINVAL);
>           }
>   
> -        allocSurf.version = NV_ENC_CREATE_INPUT_BUFFER_VER;
> +        allocSurf.version = struct_ver_rt(ctx, 1);//NV_ENC_CREATE_INPUT_BUFFER_VER;
>           allocSurf.width = avctx->width;
>           allocSurf.height = avctx->height;
>           allocSurf.bufferFmt = ctx->surfaces[idx].format;
> @@ -1503,7 +1513,7 @@ static av_cold int nvenc_setup_extradata(AVCodecContext *avctx)
>       uint32_t outSize = 0;
>       char tmpHeader[256];
>       NV_ENC_SEQUENCE_PARAM_PAYLOAD payload = { 0 };
> -    payload.version = NV_ENC_SEQUENCE_PARAM_PAYLOAD_VER;
> +    payload.version = struct_ver_rt(ctx, 1);//NV_ENC_SEQUENCE_PARAM_PAYLOAD_VER;
>   
>       payload.spsppsBuffer = tmpHeader;
>       payload.inBufferSize = sizeof(tmpHeader);
> @@ -1535,7 +1545,7 @@ av_cold int ff_nvenc_encode_close(AVCodecContext *avctx)
>   
>       /* the encoder has to be flushed before it can be closed */
>       if (ctx->nvencoder) {
> -        NV_ENC_PIC_PARAMS params        = { .version        = NV_ENC_PIC_PARAMS_VER,
> +        NV_ENC_PIC_PARAMS params        = { .version        = struct_ver_rt(ctx, 4) | (1<<31),
>                                               .encodePicFlags = NV_ENC_PIC_FLAG_EOS };

Same here

>           res = nvenc_push_context(avctx);
> @@ -1747,7 +1757,7 @@ static int nvenc_register_frame(AVCodecContext *avctx, const AVFrame *frame)
>       if (idx < 0)
>           return idx;
>   
> -    reg.version            = NV_ENC_REGISTER_RESOURCE_VER;
> +    reg.version            = struct_ver_rt(ctx, 3);// NV_ENC_REGISTER_RESOURCE_VER;
>       reg.width              = frames_ctx->width;
>       reg.height             = frames_ctx->height;
>       reg.pitch              = frame->linesize[0];
> @@ -1802,7 +1812,7 @@ static int nvenc_upload_frame(AVCodecContext *avctx, const AVFrame *frame,
>               return res;
>   
>           if (!ctx->registered_frames[reg_idx].mapped) {
> -            ctx->registered_frames[reg_idx].in_map.version = NV_ENC_MAP_INPUT_RESOURCE_VER;
> +            ctx->registered_frames[reg_idx].in_map.version = struct_ver_rt(ctx, 4);// NV_ENC_MAP_INPUT_RESOURCE_VER;
>               ctx->registered_frames[reg_idx].in_map.registeredResource = ctx->registered_frames[reg_idx].regptr;
>               nv_status = p_nvenc->nvEncMapInputResource(ctx->nvencoder, &ctx->registered_frames[reg_idx].in_map);
>               if (nv_status != NV_ENC_SUCCESS) {
> @@ -1822,7 +1832,7 @@ static int nvenc_upload_frame(AVCodecContext *avctx, const AVFrame *frame,
>       } else {
>           NV_ENC_LOCK_INPUT_BUFFER lockBufferParams = { 0 };
>   
> -        lockBufferParams.version = NV_ENC_LOCK_INPUT_BUFFER_VER;
> +        lockBufferParams.version = struct_ver_rt(ctx, 1);//NV_ENC_LOCK_INPUT_BUFFER_VER;
>           lockBufferParams.inputBuffer = nvenc_frame->input_surface;
>   
>           nv_status = p_nvenc->nvEncLockInputBuffer(ctx->nvencoder, &lockBufferParams);
> @@ -1936,7 +1946,7 @@ static int process_output_surface(AVCodecContext *avctx, AVPacket *pkt, NvencSur
>           goto error;
>       }
>   
> -    lock_params.version = NV_ENC_LOCK_BITSTREAM_VER;
> +    lock_params.version = struct_ver_rt(ctx, 1);//NV_ENC_LOCK_BITSTREAM_VER;
>   
>       lock_params.doNotWait = 0;
>       lock_params.outputBitstream = tmpoutsurf->output_surface;
> @@ -2054,7 +2064,7 @@ static void reconfig_encoder(AVCodecContext *avctx, const AVFrame *frame)
>       int reconfig_bitrate = 0, reconfig_dar = 0;
>       int dw, dh;
>   
> -    params.version = NV_ENC_RECONFIGURE_PARAMS_VER;
> +    params.version = struct_ver_rt(ctx, 1) | (1<<31);//NV_ENC_RECONFIGURE_PARAMS_VER;
>       params.reInitEncodeParams = ctx->init_encode_params;
>   
>       compute_dar(avctx, &dw, &dh);
> @@ -2148,7 +2158,7 @@ static int nvenc_send_frame(AVCodecContext *avctx, const AVFrame *frame)
>       NV_ENCODE_API_FUNCTION_LIST *p_nvenc = &dl_fn->nvenc_funcs;
>   
>       NV_ENC_PIC_PARAMS pic_params = { 0 };
> -    pic_params.version = NV_ENC_PIC_PARAMS_VER;
> +    pic_params.version = struct_ver_rt(ctx, 4) | (1<<31);//NV_ENC_PIC_PARAMS_VER;
>   
>       if ((!ctx->cu_context && !ctx->d3d11_device) || !ctx->nvencoder)
>           return AVERROR(EINVAL);
> diff --git a/libavcodec/nvenc.h b/libavcodec/nvenc.h
> index fb3820f7cf..6a39391276 100644
> --- a/libavcodec/nvenc.h
> +++ b/libavcodec/nvenc.h
> @@ -146,6 +146,9 @@ typedef struct NvencContext
>   {
>       AVClass *avclass;
>   
> +    uint32_t apiver_rt;
> +    uint32_t config_ver_rt;
> +
>       NvencDynLoadFunctions nvenc_dload_funcs;
>   
>       NV_ENC_INITIALIZE_PARAMS init_encode_params;
> 

What is the logic behind picking those numbers? What happens if a struct 
gets updated, and ffmpeg wants to use the new fields when available, 
like happened plenty of times?
Wang Bin July 16, 2020, 2:16 a.m. UTC | #2 
Timo Rothenpieler <timo@rothenpieler.org> 于2020年7月15日周三 下午11:16写道：

> On 15.07.2020 16:34, wangbin wrote:
> > From: wang-bin <wbsecg1@gmail.com>
> >
> > There are reserved bit fields in nvEncoderAPI.h structs, so driver's abi
> > is stable. Requesting runtime version of these structs should work. I've
> > compared 7.0~9.x headers, and confirmed on some devices.
> > ---
> >   libavcodec/nvenc.c | 72 ++++++++++++++++++++++++++--------------------
> >   libavcodec/nvenc.h |  3 ++
> >   2 files changed, 44 insertions(+), 31 deletions(-)
> >
> > diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
> > index c6740c1842..ac35cb9f48 100644
> > --- a/libavcodec/nvenc.c
> > +++ b/libavcodec/nvenc.c
> > @@ -193,14 +193,26 @@ static void
> nvenc_print_driver_requirement(AVCodecContext *avctx, int level)
> >       av_log(avctx, level, "The minimum required Nvidia driver for nvenc
> is %s or newer\n", minver);
> >   }
> >
> > +static inline uint32_t struct_ver_rt(NvencContext* ctx, uint32_t
> struct_ver)
> > +{
> > +    return ((uint32_t)ctx->apiver_rt | ((struct_ver)<<16) | (0x7 <<
> 28));
> > +}
>
> I'm really not a fan of hard-coding magic numbers, that can change in
> the future, into the ffmpeg implementation.
>
> > +static inline uint32_t api_ver(uint32_t major_ver, uint32_t minor_ver)
> > +{
> > +    return major_ver | (minor_ver << 24);
> > +}
> > +
> >   static av_cold int nvenc_load_libraries(AVCodecContext *avctx)
> >   {
> >       NvencContext *ctx            = avctx->priv_data;
> >       NvencDynLoadFunctions *dl_fn = &ctx->nvenc_dload_funcs;
> >       NVENCSTATUS err;
> >       uint32_t nvenc_max_ver;
> > +    uint32_t nvenc_max_major;
> > +    uint32_t nvenc_max_minor;
> > +    uint32_t func_ver = NV_ENCODE_API_FUNCTION_LIST_VER;
> >       int ret;
> > -
> >       ret = cuda_load_functions(&dl_fn->cuda_dl, avctx);
> >       if (ret < 0)
> >           return ret;
> > @@ -214,19 +226,17 @@ static av_cold int
> nvenc_load_libraries(AVCodecContext *avctx)
> >       err =
> dl_fn->nvenc_dl->NvEncodeAPIGetMaxSupportedVersion(&nvenc_max_ver);
> >       if (err != NV_ENC_SUCCESS)
> >           return nvenc_print_error(avctx, err, "Failed to query nvenc
> max version");
> > +    nvenc_max_major = nvenc_max_ver >> 4;
> > +    nvenc_max_minor = nvenc_max_ver & 0xf;
> > +    av_log(avctx, AV_LOG_VERBOSE, "nvenc build version: %d.%d, runtime
> version: %d.%d\n", NVENCAPI_MAJOR_VERSION, NVENCAPI_MINOR_VERSION,
> nvenc_max_major, nvenc_max_minor);
> >
> > -    av_log(avctx, AV_LOG_VERBOSE, "Loaded Nvenc version %d.%d\n",
> nvenc_max_ver >> 4, nvenc_max_ver & 0xf);
> > -
> > -    if ((NVENCAPI_MAJOR_VERSION << 4 | NVENCAPI_MINOR_VERSION) >
> nvenc_max_ver) {
> > -        av_log(avctx, AV_LOG_ERROR, "Driver does not support the
> required nvenc API version. "
> > -               "Required: %d.%d Found: %d.%d\n",
> > -               NVENCAPI_MAJOR_VERSION, NVENCAPI_MINOR_VERSION,
> > -               nvenc_max_ver >> 4, nvenc_max_ver & 0xf);
> > -        nvenc_print_driver_requirement(avctx, AV_LOG_ERROR);
> > -        return AVERROR(ENOSYS);
> > -    }
> > +    ctx->apiver_rt = api_ver(nvenc_max_major, nvenc_max_minor); /*
> NVENCAPI_VERSION */
> > +    ctx->config_ver_rt = struct_ver_rt(ctx, 7) | (1<<31); /*
> NV_ENC_CONFIG_VER */
> > +    if (ctx->apiver_rt < api_ver(8, 1))
> > +        ctx->config_ver_rt = struct_ver_rt(ctx, 6) | (1<<31);
> > +    func_ver = struct_ver_rt(ctx, 2);
> >
> > -    dl_fn->nvenc_funcs.version = NV_ENCODE_API_FUNCTION_LIST_VER;
> > +    dl_fn->nvenc_funcs.version = func_ver;
> >
> >       err =
> dl_fn->nvenc_dl->NvEncodeAPICreateInstance(&dl_fn->nvenc_funcs);
> >       if (err != NV_ENC_SUCCESS)
> > @@ -267,8 +277,8 @@ static av_cold int nvenc_open_session(AVCodecContext
> *avctx)
> >       NV_ENCODE_API_FUNCTION_LIST *p_nvenc =
> &ctx->nvenc_dload_funcs.nvenc_funcs;
> >       NVENCSTATUS ret;
> >
> > -    params.version    = NV_ENC_OPEN_ENCODE_SESSION_EX_PARAMS_VER;
> > -    params.apiVersion = NVENCAPI_VERSION;
> > +    params.version    = struct_ver_rt(ctx, 1); //
> NV_ENC_OPEN_ENCODE_SESSION_EX_PARAMS_VER
> > +    params.apiVersion = ctx->apiver_rt;
> >       if (ctx->d3d11_device) {
> >           params.device     = ctx->d3d11_device;
> >           params.deviceType = NV_ENC_DEVICE_TYPE_DIRECTX;
> > @@ -329,7 +339,7 @@ static int nvenc_check_cap(AVCodecContext *avctx,
> NV_ENC_CAPS cap)
> >       NV_ENC_CAPS_PARAM params        = { 0 };
> >       int ret, val = 0;
> >
> > -    params.version     = NV_ENC_CAPS_PARAM_VER;
> > +    params.version     = struct_ver_rt(ctx, 1); //
> NV_ENC_CAPS_PARAM_VER;
> >       params.capsToQuery = cap;
> >
> >       ret = p_nvenc->nvEncGetEncodeCaps(ctx->nvencoder,
> ctx->init_encode_params.encodeGUID, &params, &val);
> > @@ -688,7 +698,7 @@ static av_cold void set_constqp(AVCodecContext
> *avctx)
> >       NV_ENC_RC_PARAMS *rc = &ctx->encode_config.rcParams;
> >
> >       rc->rateControlMode = NV_ENC_PARAMS_RC_CONSTQP;
> > -
> > +    /*rc->reservedBitField1 = 0;*/
> >       if (ctx->init_qp_p >= 0) {
> >           rc->constQP.qpInterP = ctx->init_qp_p;
> >           if (ctx->init_qp_i >= 0 && ctx->init_qp_b >= 0) {
> > @@ -1221,8 +1231,8 @@ static av_cold int
> nvenc_setup_encoder(AVCodecContext *avctx)
> >       int res = 0;
> >       int dw, dh;
> >
> > -    ctx->encode_config.version = NV_ENC_CONFIG_VER;
> > -    ctx->init_encode_params.version = NV_ENC_INITIALIZE_PARAMS_VER;
> > +    ctx->encode_config.version = ctx->config_ver_rt;//NV_ENC_CONFIG_VER;
> > +    ctx->init_encode_params.version = struct_ver_rt(ctx, 5) |
> (1<<31);//NV_ENC_INITIALIZE_PARAMS_VER;
>
> More magic numbers.
>
> >
> >       ctx->init_encode_params.encodeHeight = avctx->height;
> >       ctx->init_encode_params.encodeWidth = avctx->width;
> > @@ -1231,8 +1241,8 @@ static av_cold int
> nvenc_setup_encoder(AVCodecContext *avctx)
> >
> >       nvenc_map_preset(ctx);
> >
> > -    preset_config.version = NV_ENC_PRESET_CONFIG_VER;
> > -    preset_config.presetCfg.version = NV_ENC_CONFIG_VER;
> > +    preset_config.version = struct_ver_rt(ctx, 4) | (1<<31);//
> NV_ENC_PRESET_CONFIG_VER;
>
> Yet more of them.
>
> > +    preset_config.presetCfg.version =
> ctx->config_ver_rt;//NV_ENC_CONFIG_VER;
> >
> >       if (IS_SDK10_PRESET(ctx->preset)) {
> >   #ifdef NVENC_HAVE_NEW_PRESETS
> > @@ -1260,7 +1270,7 @@ static av_cold int
> nvenc_setup_encoder(AVCodecContext *avctx)
> >
> >       memcpy(&ctx->encode_config, &preset_config.presetCfg,
> sizeof(ctx->encode_config));
> >
> > -    ctx->encode_config.version = NV_ENC_CONFIG_VER;
> > +    ctx->encode_config.version = ctx->config_ver_rt;//NV_ENC_CONFIG_VER;
> >
> >       compute_dar(avctx, &dw, &dh);
> >       ctx->init_encode_params.darHeight = dh;
> > @@ -1404,7 +1414,7 @@ static av_cold int
> nvenc_alloc_surface(AVCodecContext *avctx, int idx)
> >
> >       NVENCSTATUS nv_status;
> >       NV_ENC_CREATE_BITSTREAM_BUFFER allocOut = { 0 };
> > -    allocOut.version = NV_ENC_CREATE_BITSTREAM_BUFFER_VER;
> > +    allocOut.version = struct_ver_rt(ctx,
> 1);//NV_ENC_CREATE_BITSTREAM_BUFFER_VER;
> >
> >       if (avctx->pix_fmt == AV_PIX_FMT_CUDA || avctx->pix_fmt ==
> AV_PIX_FMT_D3D11) {
> >           ctx->surfaces[idx].in_ref = av_frame_alloc();
> > @@ -1420,7 +1430,7 @@ static av_cold int
> nvenc_alloc_surface(AVCodecContext *avctx, int idx)
> >               return AVERROR(EINVAL);
> >           }
> >
> > -        allocSurf.version = NV_ENC_CREATE_INPUT_BUFFER_VER;
> > +        allocSurf.version = struct_ver_rt(ctx,
> 1);//NV_ENC_CREATE_INPUT_BUFFER_VER;
> >           allocSurf.width = avctx->width;
> >           allocSurf.height = avctx->height;
> >           allocSurf.bufferFmt = ctx->surfaces[idx].format;
> > @@ -1503,7 +1513,7 @@ static av_cold int
> nvenc_setup_extradata(AVCodecContext *avctx)
> >       uint32_t outSize = 0;
> >       char tmpHeader[256];
> >       NV_ENC_SEQUENCE_PARAM_PAYLOAD payload = { 0 };
> > -    payload.version = NV_ENC_SEQUENCE_PARAM_PAYLOAD_VER;
> > +    payload.version = struct_ver_rt(ctx,
> 1);//NV_ENC_SEQUENCE_PARAM_PAYLOAD_VER;
> >
> >       payload.spsppsBuffer = tmpHeader;
> >       payload.inBufferSize = sizeof(tmpHeader);
> > @@ -1535,7 +1545,7 @@ av_cold int ff_nvenc_encode_close(AVCodecContext
> *avctx)
> >
> >       /* the encoder has to be flushed before it can be closed */
> >       if (ctx->nvencoder) {
> > -        NV_ENC_PIC_PARAMS params        = { .version        =
> NV_ENC_PIC_PARAMS_VER,
> > +        NV_ENC_PIC_PARAMS params        = { .version        =
> struct_ver_rt(ctx, 4) | (1<<31),
> >                                               .encodePicFlags =
> NV_ENC_PIC_FLAG_EOS };
>
> Same here
>
> >           res = nvenc_push_context(avctx);
> > @@ -1747,7 +1757,7 @@ static int nvenc_register_frame(AVCodecContext
> *avctx, const AVFrame *frame)
> >       if (idx < 0)
> >           return idx;
> >
> > -    reg.version            = NV_ENC_REGISTER_RESOURCE_VER;
> > +    reg.version            = struct_ver_rt(ctx, 3);//
> NV_ENC_REGISTER_RESOURCE_VER;
> >       reg.width              = frames_ctx->width;
> >       reg.height             = frames_ctx->height;
> >       reg.pitch              = frame->linesize[0];
> > @@ -1802,7 +1812,7 @@ static int nvenc_upload_frame(AVCodecContext
> *avctx, const AVFrame *frame,
> >               return res;
> >
> >           if (!ctx->registered_frames[reg_idx].mapped) {
> > -            ctx->registered_frames[reg_idx].in_map.version =
> NV_ENC_MAP_INPUT_RESOURCE_VER;
> > +            ctx->registered_frames[reg_idx].in_map.version =
> struct_ver_rt(ctx, 4);// NV_ENC_MAP_INPUT_RESOURCE_VER;
> >               ctx->registered_frames[reg_idx].in_map.registeredResource
> = ctx->registered_frames[reg_idx].regptr;
> >               nv_status = p_nvenc->nvEncMapInputResource(ctx->nvencoder,
> &ctx->registered_frames[reg_idx].in_map);
> >               if (nv_status != NV_ENC_SUCCESS) {
> > @@ -1822,7 +1832,7 @@ static int nvenc_upload_frame(AVCodecContext
> *avctx, const AVFrame *frame,
> >       } else {
> >           NV_ENC_LOCK_INPUT_BUFFER lockBufferParams = { 0 };
> >
> > -        lockBufferParams.version = NV_ENC_LOCK_INPUT_BUFFER_VER;
> > +        lockBufferParams.version = struct_ver_rt(ctx,
> 1);//NV_ENC_LOCK_INPUT_BUFFER_VER;
> >           lockBufferParams.inputBuffer = nvenc_frame->input_surface;
> >
> >           nv_status = p_nvenc->nvEncLockInputBuffer(ctx->nvencoder,
> &lockBufferParams);
> > @@ -1936,7 +1946,7 @@ static int process_output_surface(AVCodecContext
> *avctx, AVPacket *pkt, NvencSur
> >           goto error;
> >       }
> >
> > -    lock_params.version = NV_ENC_LOCK_BITSTREAM_VER;
> > +    lock_params.version = struct_ver_rt(ctx,
> 1);//NV_ENC_LOCK_BITSTREAM_VER;
> >
> >       lock_params.doNotWait = 0;
> >       lock_params.outputBitstream = tmpoutsurf->output_surface;
> > @@ -2054,7 +2064,7 @@ static void reconfig_encoder(AVCodecContext
> *avctx, const AVFrame *frame)
> >       int reconfig_bitrate = 0, reconfig_dar = 0;
> >       int dw, dh;
> >
> > -    params.version = NV_ENC_RECONFIGURE_PARAMS_VER;
> > +    params.version = struct_ver_rt(ctx, 1) |
> (1<<31);//NV_ENC_RECONFIGURE_PARAMS_VER;
> >       params.reInitEncodeParams = ctx->init_encode_params;
> >
> >       compute_dar(avctx, &dw, &dh);
> > @@ -2148,7 +2158,7 @@ static int nvenc_send_frame(AVCodecContext *avctx,
> const AVFrame *frame)
> >       NV_ENCODE_API_FUNCTION_LIST *p_nvenc = &dl_fn->nvenc_funcs;
> >
> >       NV_ENC_PIC_PARAMS pic_params = { 0 };
> > -    pic_params.version = NV_ENC_PIC_PARAMS_VER;
> > +    pic_params.version = struct_ver_rt(ctx, 4) |
> (1<<31);//NV_ENC_PIC_PARAMS_VER;
> >
> >       if ((!ctx->cu_context && !ctx->d3d11_device) || !ctx->nvencoder)
> >           return AVERROR(EINVAL);
> > diff --git a/libavcodec/nvenc.h b/libavcodec/nvenc.h
> > index fb3820f7cf..6a39391276 100644
> > --- a/libavcodec/nvenc.h
> > +++ b/libavcodec/nvenc.h
> > @@ -146,6 +146,9 @@ typedef struct NvencContext
> >   {
> >       AVClass *avclass;
> >
> > +    uint32_t apiver_rt;
> > +    uint32_t config_ver_rt;
> > +
> >       NvencDynLoadFunctions nvenc_dload_funcs;
> >
> >       NV_ENC_INITIALIZE_PARAMS init_encode_params;
> >
>
> What is the logic behind picking those numbers?


These numbers are copied from nvEncodeAPI.h. For example,
#define NVENCAPI_VERSION (NVENCAPI_MAJOR_VERSION | (NVENCAPI_MINOR_VERSION
<< 24))
It's hardcoded to sdk version, and was used to create an encode session
via  NV_ENC_OPEN_ENCODE_SESSION_EX_PARAMS.apiVersion =  NVENCAPI_VERSION.
sdk version is usually higher than runtime driver version because FFmpeg
upgrade sdk header frequently. So to create driver supported encode
session, we need a version number. Unfortunately no convenience macro is
provide, so I define uint32_t api_ver(uint32_t major_ver, uint32_t
minor_ver). The same reason for  struct_ver_rt().
I compared 7.0~10.0 headers, struct version changes rarely. An exception
is NV_ENC_CONFIG.version, it changed in 8.1. This explains

> > +    ctx->config_ver_rt = struct_ver_rt(ctx, 7) | (1<<31); /*
> NV_ENC_CONFIG_VER */
> > +    if (ctx->apiver_rt < api_ver(8, 1))
> > +        ctx->config_ver_rt = struct_ver_rt(ctx, 6) | (1<<31);




> What happens if a struct
> gets updated, and ffmpeg wants to use the new fields when available,
> like happened plenty of times?
>
Upgrade nvEncodeAPI.h like we've already done. Part of reserved bits will
become a struct member and have a name. These bits will be recognized on
new drivers but ignored on old ones.
Timo Rothenpieler July 16, 2020, 9:36 a.m. UTC | #3 
On 16.07.2020 04:16, Wang Bin wrote:
> 
>> What happens if a struct
>> gets updated, and ffmpeg wants to use the new fields when available,
>> like happened plenty of times?
>>
> Upgrade nvEncodeAPI.h like we've already done. Part of reserved bits will
> become a struct member and have a name. These bits will be recognized on
> new drivers but ignored on old ones.

So if someone relies on one of the new fields, be it for quality 
settings or something, it will then silently be ignored?
This needs some kind of guard for every single field, with an 
appropriate error message if a feature newer than what's supported has 
been used.

I'm really not sure if this is all worth it. It will massively blow up 
the code and make it harder and harder to maintain with every version.
diff mbox series

Patch

diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
index c6740c1842..ac35cb9f48 100644
--- a/libavcodec/nvenc.c
+++ b/libavcodec/nvenc.c
@@ -193,14 +193,26 @@  static void nvenc_print_driver_requirement(AVCodecContext *avctx, int level)
     av_log(avctx, level, "The minimum required Nvidia driver for nvenc is %s or newer\n", minver);
 }
 
+static inline uint32_t struct_ver_rt(NvencContext* ctx, uint32_t struct_ver)
+{
+    return ((uint32_t)ctx->apiver_rt | ((struct_ver)<<16) | (0x7 << 28));
+}
+
+static inline uint32_t api_ver(uint32_t major_ver, uint32_t minor_ver)
+{
+    return major_ver | (minor_ver << 24);
+}
+
 static av_cold int nvenc_load_libraries(AVCodecContext *avctx)
 {
     NvencContext *ctx            = avctx->priv_data;
     NvencDynLoadFunctions *dl_fn = &ctx->nvenc_dload_funcs;
     NVENCSTATUS err;
     uint32_t nvenc_max_ver;
+    uint32_t nvenc_max_major;
+    uint32_t nvenc_max_minor;
+    uint32_t func_ver = NV_ENCODE_API_FUNCTION_LIST_VER;
     int ret;
-
     ret = cuda_load_functions(&dl_fn->cuda_dl, avctx);
     if (ret < 0)
         return ret;
@@ -214,19 +226,17 @@  static av_cold int nvenc_load_libraries(AVCodecContext *avctx)
     err = dl_fn->nvenc_dl->NvEncodeAPIGetMaxSupportedVersion(&nvenc_max_ver);
     if (err != NV_ENC_SUCCESS)
         return nvenc_print_error(avctx, err, "Failed to query nvenc max version");
+    nvenc_max_major = nvenc_max_ver >> 4;
+    nvenc_max_minor = nvenc_max_ver & 0xf;
+    av_log(avctx, AV_LOG_VERBOSE, "nvenc build version: %d.%d, runtime version: %d.%d\n", NVENCAPI_MAJOR_VERSION, NVENCAPI_MINOR_VERSION, nvenc_max_major, nvenc_max_minor);
 
-    av_log(avctx, AV_LOG_VERBOSE, "Loaded Nvenc version %d.%d\n", nvenc_max_ver >> 4, nvenc_max_ver & 0xf);
-
-    if ((NVENCAPI_MAJOR_VERSION << 4 | NVENCAPI_MINOR_VERSION) > nvenc_max_ver) {
-        av_log(avctx, AV_LOG_ERROR, "Driver does not support the required nvenc API version. "
-               "Required: %d.%d Found: %d.%d\n",
-               NVENCAPI_MAJOR_VERSION, NVENCAPI_MINOR_VERSION,
-               nvenc_max_ver >> 4, nvenc_max_ver & 0xf);
-        nvenc_print_driver_requirement(avctx, AV_LOG_ERROR);
-        return AVERROR(ENOSYS);
-    }
+    ctx->apiver_rt = api_ver(nvenc_max_major, nvenc_max_minor); /* NVENCAPI_VERSION */
+    ctx->config_ver_rt = struct_ver_rt(ctx, 7) | (1<<31); /* NV_ENC_CONFIG_VER */
+    if (ctx->apiver_rt < api_ver(8, 1))
+        ctx->config_ver_rt = struct_ver_rt(ctx, 6) | (1<<31);
+    func_ver = struct_ver_rt(ctx, 2);
 
-    dl_fn->nvenc_funcs.version = NV_ENCODE_API_FUNCTION_LIST_VER;
+    dl_fn->nvenc_funcs.version = func_ver;
 
     err = dl_fn->nvenc_dl->NvEncodeAPICreateInstance(&dl_fn->nvenc_funcs);
     if (err != NV_ENC_SUCCESS)
@@ -267,8 +277,8 @@  static av_cold int nvenc_open_session(AVCodecContext *avctx)
     NV_ENCODE_API_FUNCTION_LIST *p_nvenc = &ctx->nvenc_dload_funcs.nvenc_funcs;
     NVENCSTATUS ret;
 
-    params.version    = NV_ENC_OPEN_ENCODE_SESSION_EX_PARAMS_VER;
-    params.apiVersion = NVENCAPI_VERSION;
+    params.version    = struct_ver_rt(ctx, 1); // NV_ENC_OPEN_ENCODE_SESSION_EX_PARAMS_VER
+    params.apiVersion = ctx->apiver_rt;
     if (ctx->d3d11_device) {
         params.device     = ctx->d3d11_device;
         params.deviceType = NV_ENC_DEVICE_TYPE_DIRECTX;
@@ -329,7 +339,7 @@  static int nvenc_check_cap(AVCodecContext *avctx, NV_ENC_CAPS cap)
     NV_ENC_CAPS_PARAM params        = { 0 };
     int ret, val = 0;
 
-    params.version     = NV_ENC_CAPS_PARAM_VER;
+    params.version     = struct_ver_rt(ctx, 1); // NV_ENC_CAPS_PARAM_VER;
     params.capsToQuery = cap;
 
     ret = p_nvenc->nvEncGetEncodeCaps(ctx->nvencoder, ctx->init_encode_params.encodeGUID, &params, &val);
@@ -688,7 +698,7 @@  static av_cold void set_constqp(AVCodecContext *avctx)
     NV_ENC_RC_PARAMS *rc = &ctx->encode_config.rcParams;
 
     rc->rateControlMode = NV_ENC_PARAMS_RC_CONSTQP;
-
+    /*rc->reservedBitField1 = 0;*/
     if (ctx->init_qp_p >= 0) {
         rc->constQP.qpInterP = ctx->init_qp_p;
         if (ctx->init_qp_i >= 0 && ctx->init_qp_b >= 0) {
@@ -1221,8 +1231,8 @@  static av_cold int nvenc_setup_encoder(AVCodecContext *avctx)
     int res = 0;
     int dw, dh;
 
-    ctx->encode_config.version = NV_ENC_CONFIG_VER;
-    ctx->init_encode_params.version = NV_ENC_INITIALIZE_PARAMS_VER;
+    ctx->encode_config.version = ctx->config_ver_rt;//NV_ENC_CONFIG_VER;
+    ctx->init_encode_params.version = struct_ver_rt(ctx, 5) | (1<<31);//NV_ENC_INITIALIZE_PARAMS_VER;
 
     ctx->init_encode_params.encodeHeight = avctx->height;
     ctx->init_encode_params.encodeWidth = avctx->width;
@@ -1231,8 +1241,8 @@  static av_cold int nvenc_setup_encoder(AVCodecContext *avctx)
 
     nvenc_map_preset(ctx);
 
-    preset_config.version = NV_ENC_PRESET_CONFIG_VER;
-    preset_config.presetCfg.version = NV_ENC_CONFIG_VER;
+    preset_config.version = struct_ver_rt(ctx, 4) | (1<<31);// NV_ENC_PRESET_CONFIG_VER;
+    preset_config.presetCfg.version = ctx->config_ver_rt;//NV_ENC_CONFIG_VER;
 
     if (IS_SDK10_PRESET(ctx->preset)) {
 #ifdef NVENC_HAVE_NEW_PRESETS
@@ -1260,7 +1270,7 @@  static av_cold int nvenc_setup_encoder(AVCodecContext *avctx)
 
     memcpy(&ctx->encode_config, &preset_config.presetCfg, sizeof(ctx->encode_config));
 
-    ctx->encode_config.version = NV_ENC_CONFIG_VER;
+    ctx->encode_config.version = ctx->config_ver_rt;//NV_ENC_CONFIG_VER;
 
     compute_dar(avctx, &dw, &dh);
     ctx->init_encode_params.darHeight = dh;
@@ -1404,7 +1414,7 @@  static av_cold int nvenc_alloc_surface(AVCodecContext *avctx, int idx)
 
     NVENCSTATUS nv_status;
     NV_ENC_CREATE_BITSTREAM_BUFFER allocOut = { 0 };
-    allocOut.version = NV_ENC_CREATE_BITSTREAM_BUFFER_VER;
+    allocOut.version = struct_ver_rt(ctx, 1);//NV_ENC_CREATE_BITSTREAM_BUFFER_VER;
 
     if (avctx->pix_fmt == AV_PIX_FMT_CUDA || avctx->pix_fmt == AV_PIX_FMT_D3D11) {
         ctx->surfaces[idx].in_ref = av_frame_alloc();
@@ -1420,7 +1430,7 @@  static av_cold int nvenc_alloc_surface(AVCodecContext *avctx, int idx)
             return AVERROR(EINVAL);
         }
 
-        allocSurf.version = NV_ENC_CREATE_INPUT_BUFFER_VER;
+        allocSurf.version = struct_ver_rt(ctx, 1);//NV_ENC_CREATE_INPUT_BUFFER_VER;
         allocSurf.width = avctx->width;
         allocSurf.height = avctx->height;
         allocSurf.bufferFmt = ctx->surfaces[idx].format;
@@ -1503,7 +1513,7 @@  static av_cold int nvenc_setup_extradata(AVCodecContext *avctx)
     uint32_t outSize = 0;
     char tmpHeader[256];
     NV_ENC_SEQUENCE_PARAM_PAYLOAD payload = { 0 };
-    payload.version = NV_ENC_SEQUENCE_PARAM_PAYLOAD_VER;
+    payload.version = struct_ver_rt(ctx, 1);//NV_ENC_SEQUENCE_PARAM_PAYLOAD_VER;
 
     payload.spsppsBuffer = tmpHeader;
     payload.inBufferSize = sizeof(tmpHeader);
@@ -1535,7 +1545,7 @@  av_cold int ff_nvenc_encode_close(AVCodecContext *avctx)
 
     /* the encoder has to be flushed before it can be closed */
     if (ctx->nvencoder) {
-        NV_ENC_PIC_PARAMS params        = { .version        = NV_ENC_PIC_PARAMS_VER,
+        NV_ENC_PIC_PARAMS params        = { .version        = struct_ver_rt(ctx, 4) | (1<<31),
                                             .encodePicFlags = NV_ENC_PIC_FLAG_EOS };
 
         res = nvenc_push_context(avctx);
@@ -1747,7 +1757,7 @@  static int nvenc_register_frame(AVCodecContext *avctx, const AVFrame *frame)
     if (idx < 0)
         return idx;
 
-    reg.version            = NV_ENC_REGISTER_RESOURCE_VER;
+    reg.version            = struct_ver_rt(ctx, 3);// NV_ENC_REGISTER_RESOURCE_VER;
     reg.width              = frames_ctx->width;
     reg.height             = frames_ctx->height;
     reg.pitch              = frame->linesize[0];
@@ -1802,7 +1812,7 @@  static int nvenc_upload_frame(AVCodecContext *avctx, const AVFrame *frame,
             return res;
 
         if (!ctx->registered_frames[reg_idx].mapped) {
-            ctx->registered_frames[reg_idx].in_map.version = NV_ENC_MAP_INPUT_RESOURCE_VER;
+            ctx->registered_frames[reg_idx].in_map.version = struct_ver_rt(ctx, 4);// NV_ENC_MAP_INPUT_RESOURCE_VER;
             ctx->registered_frames[reg_idx].in_map.registeredResource = ctx->registered_frames[reg_idx].regptr;
             nv_status = p_nvenc->nvEncMapInputResource(ctx->nvencoder, &ctx->registered_frames[reg_idx].in_map);
             if (nv_status != NV_ENC_SUCCESS) {
@@ -1822,7 +1832,7 @@  static int nvenc_upload_frame(AVCodecContext *avctx, const AVFrame *frame,
     } else {
         NV_ENC_LOCK_INPUT_BUFFER lockBufferParams = { 0 };
 
-        lockBufferParams.version = NV_ENC_LOCK_INPUT_BUFFER_VER;
+        lockBufferParams.version = struct_ver_rt(ctx, 1);//NV_ENC_LOCK_INPUT_BUFFER_VER;
         lockBufferParams.inputBuffer = nvenc_frame->input_surface;
 
         nv_status = p_nvenc->nvEncLockInputBuffer(ctx->nvencoder, &lockBufferParams);
@@ -1936,7 +1946,7 @@  static int process_output_surface(AVCodecContext *avctx, AVPacket *pkt, NvencSur
         goto error;
     }
 
-    lock_params.version = NV_ENC_LOCK_BITSTREAM_VER;
+    lock_params.version = struct_ver_rt(ctx, 1);//NV_ENC_LOCK_BITSTREAM_VER;
 
     lock_params.doNotWait = 0;
     lock_params.outputBitstream = tmpoutsurf->output_surface;
@@ -2054,7 +2064,7 @@  static void reconfig_encoder(AVCodecContext *avctx, const AVFrame *frame)
     int reconfig_bitrate = 0, reconfig_dar = 0;
     int dw, dh;
 
-    params.version = NV_ENC_RECONFIGURE_PARAMS_VER;
+    params.version = struct_ver_rt(ctx, 1) | (1<<31);//NV_ENC_RECONFIGURE_PARAMS_VER;
     params.reInitEncodeParams = ctx->init_encode_params;
 
     compute_dar(avctx, &dw, &dh);
@@ -2148,7 +2158,7 @@  static int nvenc_send_frame(AVCodecContext *avctx, const AVFrame *frame)
     NV_ENCODE_API_FUNCTION_LIST *p_nvenc = &dl_fn->nvenc_funcs;
 
     NV_ENC_PIC_PARAMS pic_params = { 0 };
-    pic_params.version = NV_ENC_PIC_PARAMS_VER;
+    pic_params.version = struct_ver_rt(ctx, 4) | (1<<31);//NV_ENC_PIC_PARAMS_VER;
 
     if ((!ctx->cu_context && !ctx->d3d11_device) || !ctx->nvencoder)
         return AVERROR(EINVAL);
diff --git a/libavcodec/nvenc.h b/libavcodec/nvenc.h
index fb3820f7cf..6a39391276 100644
--- a/libavcodec/nvenc.h
+++ b/libavcodec/nvenc.h
@@ -146,6 +146,9 @@  typedef struct NvencContext
 {
     AVClass *avclass;
 
+    uint32_t apiver_rt;
+    uint32_t config_ver_rt;
+
     NvencDynLoadFunctions nvenc_dload_funcs;
 
     NV_ENC_INITIALIZE_PARAMS init_encode_params;