[FFmpeg-devel] Added - HW accelerated H.264 and HEVC encoding for AMD GPUs based on AMF SDK
Mironov, Mikhail
Mikhail.Mironov at amd.com
Sun Oct 29 16:57:15 EET 2017
> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
> Of Mark Thompson
> Sent: October 28, 2017 5:29 PM
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] Added - HW accelerated H.264 and HEVC
> encoding for AMD GPUs based on AMF SDK
>
> On 27/10/17 19:09, mmironov wrote:
> > From b1b697aed459947cfa04bccdca0f7cfb5c8be72c Mon Sep 17 00:00:00
> 2001
> > From: mmironov <mikhail.mironov at amd.com>
> > Date: Fri, 27 Oct 2017 13:03:15 -0400
> > Subject: [PATCH] Added: HW accelerated H.264 and HEVC encoding for
> AMD GPUs
> > based on AMF SDK
>
> There isn't any sense in which this is "accelerated" is there? Just say it's a
> hardware encoder (as you already do in the changelog).
OK, will do next time.
Mikhail
> >
> > Signed-off-by: mmironov <mikhail.mironov at amd.com>
> > ---
> > Changelog | 3 +-
> > compat/amd/amfsdkenc.h | 1750
> ++++++++++++++++++++++++++++++++++++++++++++++
> > configure | 28 +
> > libavcodec/Makefile | 4 +
> > libavcodec/allcodecs.c | 4 +
> > libavcodec/amfenc.c | 463 ++++++++++++
> > libavcodec/amfenc.h | 131 ++++
> > libavcodec/amfenc_h264.c | 467 +++++++++++++
> > libavcodec/amfenc_hevc.c | 354 ++++++++++
> > libavcodec/version.h | 4 +-
> > 10 files changed, 3205 insertions(+), 3 deletions(-)
> > create mode 100644 compat/amd/amfsdkenc.h
> > create mode 100644 libavcodec/amfenc.c
> > create mode 100644 libavcodec/amfenc.h
> > create mode 100644 libavcodec/amfenc_h264.c
> > create mode 100644 libavcodec/amfenc_hevc.c
> >
> > diff --git a/Changelog b/Changelog
> > index 6592d86..f0d22fa 100644
> > --- a/Changelog
> > +++ b/Changelog
> > @@ -6,7 +6,8 @@ version <next>:
> > - Dropped support for OpenJPEG versions 2.0 and below. Using OpenJPEG
> now
> > requires 2.1 (or later) and pkg-config.
> > - VDA dropped (use VideoToolbox instead)
> > -
> > +- AMF H.264 encoder
> > +- AMF HEVC encoder
> >
> > version 3.4:
> > - deflicker video filter
> > diff --git a/compat/amd/amfsdkenc.h b/compat/amd/amfsdkenc.h
> > new file mode 100644
> > index 0000000..a640c17
> > --- /dev/null
> > +++ b/compat/amd/amfsdkenc.h
>
> (Ignoring the header, will consider this separately.)
>
> > diff --git a/configure b/configure
> > index 0e1ccaa..229443f 100755
> > --- a/configure
> > +++ b/configure
> > @@ -304,6 +304,7 @@ External library support:
> >
> > The following libraries provide various hardware acceleration features:
> > --disable-audiotoolbox disable Apple AudioToolbox code [autodetect]
> > + --disable-amf disable AMF video encoding code [autodetect]
> > --disable-cuda disable dynamically linked Nvidia CUDA code
> [autodetect]
> > --enable-cuda-sdk enable CUDA features that require the CUDA SDK
> [no]
> > --disable-cuvid disable Nvidia CUVID support [autodetect]
> > @@ -1643,6 +1644,7 @@ EXTERNAL_LIBRARY_LIST="
> > HWACCEL_AUTODETECT_LIBRARY_LIST="
> > audiotoolbox
> > crystalhd
> > + amf
> > cuda
> > cuvid
> > d3d11va
>
> Lists in configure should be kept in alphabetical order.
>
> > @@ -2785,12 +2787,16 @@ scale_npp_filter_deps="cuda libnpp"
> > scale_cuda_filter_deps="cuda_sdk"
> > thumbnail_cuda_filter_deps="cuda_sdk"
> >
> > +amf_deps_any="dlopen LoadLibrary"
> > +amf_encoder_deps="amf"
> > +
> > nvenc_deps="cuda"
> > nvenc_deps_any="libdl LoadLibrary"
> > nvenc_encoder_deps="nvenc"
> >
> > h263_v4l2m2m_decoder_deps="v4l2_m2m h263_v4l2_m2m"
> > h263_v4l2m2m_encoder_deps="v4l2_m2m h263_v4l2_m2m"
> > +h264_amf_encoder_deps="amf"
> > h264_crystalhd_decoder_select="crystalhd h264_mp4toannexb_bsf
> h264_parser"
> > h264_cuvid_decoder_deps="cuda cuvid"
> > h264_cuvid_decoder_select="h264_mp4toannexb_bsf"
> > @@ -2809,6 +2815,7 @@
> h264_vaapi_encoder_deps="VAEncPictureParameterBufferH264"
> > h264_vaapi_encoder_select="cbs_h264 vaapi_encode"
> > h264_v4l2m2m_decoder_deps="v4l2_m2m h264_v4l2_m2m"
> > h264_v4l2m2m_encoder_deps="v4l2_m2m h264_v4l2_m2m"
> > +hevc_amf_encoder_deps="amf"
> > hevc_cuvid_decoder_deps="cuda cuvid"
> > hevc_cuvid_decoder_select="hevc_mp4toannexb_bsf"
> > hevc_mediacodec_decoder_deps="mediacodec"
> > @@ -2830,6 +2837,8 @@ mjpeg_vaapi_encoder_select="vaapi_encode
> jpegtables"
> > mpeg1_cuvid_decoder_deps="cuda cuvid"
> > mpeg1_v4l2m2m_decoder_deps="v4l2_m2m mpeg1_v4l2_m2m"
> > mpeg2_crystalhd_decoder_select="crystalhd"
> > +amf_h264_encoder_select="h264_amf_encoder"
> > +amf_hevc_encoder_select="hevc_amf_encoder"
>
> These names aren't mentioned anywhere else.
>
> > mpeg2_cuvid_decoder_deps="cuda cuvid"
> > mpeg2_mmal_decoder_deps="mmal"
> > mpeg2_mediacodec_decoder_deps="mediacodec"
> > @@ -6305,6 +6314,18 @@ else
> > disable cuda cuvid nvenc
> > fi
> >
> > +if enabled x86; then
> > + case $target_os in
> > + mingw32*|mingw64*|win32|win64|cygwin*)
> > + ;;
> > + *)
> > + disable amf
> > + ;;
> > + esac
> > +else
> > + disable amf
> > +fi
>
> Why this OS test? It should just be going by whether the relevant build
> packages are present.
>
> > +
> > enabled nvenc &&
> > check_cc -I$source_path <<EOF || disable nvenc
> > #include "compat/nvenc/nvEncodeAPI.h"
> > @@ -6313,6 +6334,13 @@ void f(void) { struct { const GUID guid; } s[] = { {
> NV_ENC_PRESET_HQ_GUID } };
> > int main(void) { return 0; }
> > EOF
> >
> > +enabled amf &&
> > + check_cc -I$source_path <<EOF || disable amf
> > +#include "compat/amd/amfsdkenc.h"
> > +AMFFactory *factory;
> > +int main(void) { return 0; }
> > +EOF
> > +
> > # Funny iconv installations are not unusual, so check it after all flags have
> been set
> > if enabled libc_iconv; then
> > check_func_headers iconv.h iconv
> > diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> > index bc4d7da..cbf45ac 100644
> > --- a/libavcodec/Makefile
> > +++ b/libavcodec/Makefile
> > @@ -50,6 +50,7 @@ OBJS = allcodecs.o \
> > # subsystems
> > OBJS-$(CONFIG_AANDCTTABLES) += aandcttab.o
> > OBJS-$(CONFIG_AC3DSP) += ac3dsp.o ac3.o ac3tab.o
> > +OBJS-$(CONFIG_AMF) += amfenc.o
> > OBJS-$(CONFIG_AUDIO_FRAME_QUEUE) += audio_frame_queue.o
> > OBJS-$(CONFIG_AUDIODSP) += audiodsp.o
> > OBJS-$(CONFIG_BLOCKDSP) += blockdsp.o
> > @@ -334,6 +335,7 @@ OBJS-$(CONFIG_H264_DECODER) +=
> h264dec.o h264_cabac.o h264_cavlc.o \
> > OBJS-$(CONFIG_H264_CUVID_DECODER) += cuvid.o
> > OBJS-$(CONFIG_H264_MEDIACODEC_DECODER) += mediacodecdec.o
> > OBJS-$(CONFIG_H264_MMAL_DECODER) += mmaldec.o
> > +OBJS-$(CONFIG_H264_AMF_ENCODER) += amfenc_h264.o
> > OBJS-$(CONFIG_H264_NVENC_ENCODER) += nvenc_h264.o
> > OBJS-$(CONFIG_NVENC_ENCODER) += nvenc_h264.o
> > OBJS-$(CONFIG_NVENC_H264_ENCODER) += nvenc_h264.o
> > @@ -352,6 +354,7 @@ OBJS-$(CONFIG_HEVC_DECODER) +=
> hevcdec.o hevc_mvs.o \
> > hevcdsp.o hevc_filter.o hevc_data.o
> > OBJS-$(CONFIG_HEVC_CUVID_DECODER) += cuvid.o
> > OBJS-$(CONFIG_HEVC_MEDIACODEC_DECODER) += mediacodecdec.o
> > +OBJS-$(CONFIG_HEVC_AMF_ENCODER) += amfenc_hevc.o
> > OBJS-$(CONFIG_HEVC_NVENC_ENCODER) += nvenc_hevc.o
> > OBJS-$(CONFIG_NVENC_HEVC_ENCODER) += nvenc_hevc.o
> > OBJS-$(CONFIG_HEVC_QSV_DECODER) += qsvdec_h2645.o
> > @@ -1056,6 +1059,7 @@ SKIPHEADERS-$(CONFIG_JNI) += ffjni.h
> > SKIPHEADERS-$(CONFIG_LIBVPX) += libvpx.h
> > SKIPHEADERS-$(CONFIG_LIBWEBP_ENCODER) += libwebpenc_common.h
> > SKIPHEADERS-$(CONFIG_MEDIACODEC) += mediacodecdec_common.h
> mediacodec_surface.h mediacodec_wrapper.h mediacodec_sw_buffer.h
> > +SKIPHEADERS-$(CONFIG_AMF) += amfenc.h
> > SKIPHEADERS-$(CONFIG_NVENC) += nvenc.h
> > SKIPHEADERS-$(CONFIG_QSV) += qsv.h qsv_internal.h
> > SKIPHEADERS-$(CONFIG_QSVDEC) += qsvdec.h
> > diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> > index 8369126..d597540 100644
> > --- a/libavcodec/allcodecs.c
> > +++ b/libavcodec/allcodecs.c
> > @@ -649,6 +649,8 @@ static void register_all(void)
> > * above is available */
> > REGISTER_ENCODER(H263_V4L2M2M, h263_v4l2m2m);
> > REGISTER_ENCDEC (LIBOPENH264, libopenh264);
> > + REGISTER_ENCODER(H264_AMF, h264_amf);
> > + REGISTER_ENCODER(H264_AMF, h264_amf_d3d11va);
>
> No tabs. Why is the d3d11 version separate? The encoder should be able to
> accept multiple pixfmts.
>
> > REGISTER_DECODER(H264_CUVID, h264_cuvid);
> > REGISTER_ENCODER(H264_NVENC, h264_nvenc);
> > REGISTER_ENCODER(H264_OMX, h264_omx);
> > @@ -661,6 +663,8 @@ static void register_all(void)
> > REGISTER_ENCODER(NVENC_H264, nvenc_h264);
> > REGISTER_ENCODER(NVENC_HEVC, nvenc_hevc);
> > #endif
> > + REGISTER_ENCODER(HEVC_AMF, hevc_amf);
> > + REGISTER_ENCODER(HEVC_AMF, hevc_amf_d3d11va);
>
> Tab.
>
> > REGISTER_DECODER(HEVC_CUVID, hevc_cuvid);
> > REGISTER_DECODER(HEVC_MEDIACODEC, hevc_mediacodec);
> > REGISTER_ENCODER(HEVC_NVENC, hevc_nvenc);
> > diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c
> > new file mode 100644
> > index 0000000..8717928
> > --- /dev/null
> > +++ b/libavcodec/amfenc.c
> > @@ -0,0 +1,463 @@
> > +/*
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA
> > + */
> > +#include "amfenc.h"
> > +
> > +//#include
> "compat/amd/amf/public/include/components/VideoEncoderVCE.h"
> > +//#include
> "compat/amd/amf/public/include/components/VideoEncoderHEVC.h"
>
> Why are these commented out?
>
> > +
> > +#include "libavutil/time.h"
> > +#include "libavutil/imgutils.h"
> > +#include "libavutil/hwcontext.h"
> > +#include "libavutil/hwcontext_d3d11va.h"
> > +#include "libavutil/avassert.h"
> > +#include "libavutil/mem.h"
> > +#include "libavutil/pixdesc.h"
> > +#include "libavutil/hwcontext.h"
>
> Headers should be in alphabetical order, and not include the same thing
> multiple times.
>
> > +#include "internal.h"
> > +
> > +#include <d3d11.h>
> > +
> > +#ifdef _WIN32
> > +#include "compat/w32dlfcn.h"
> > +#else
> > +#include <dlfcn.h>
> > +#endif
> > +
> > +#define FFMPEG_AMF_WRITER_ID L"ffmpeg_amf"
> > +#define AMF_DEBUG_TRACE 0
> > +
> > +const enum AVPixelFormat ff_amf_pix_fmts[] = {
> > + AV_PIX_FMT_NV12,
> > + AV_PIX_FMT_BGRA,
> > + AV_PIX_FMT_ARGB,
> > + AV_PIX_FMT_RGBA,
> > + AV_PIX_FMT_YUV420P,
> > + AV_PIX_FMT_YUYV422,
> > + AV_PIX_FMT_D3D11,
> > + AV_PIX_FMT_NONE
> > +};
> > +
> > +typedef struct FormatMap {
> > + enum AVPixelFormat av_format;
> > + enum AMF_SURFACE_FORMAT amf_format;
> > +} FormatMap;
> > +
> > +static const FormatMap format_map[] =
> > +{
> > + { AV_PIX_FMT_NONE, AMF_SURFACE_UNKNOWN },
>
> Seems a bit pointless to include NONE in this list explicitly.
>
> > + { AV_PIX_FMT_NV12, AMF_SURFACE_NV12 },
> > + { AV_PIX_FMT_BGRA, AMF_SURFACE_BGRA },
> > + { AV_PIX_FMT_ARGB, AMF_SURFACE_ARGB },
> > + { AV_PIX_FMT_RGBA, AMF_SURFACE_RGBA },
> > + { AV_PIX_FMT_GRAY8, AMF_SURFACE_GRAY8 },
> > + { AV_PIX_FMT_YUV420P, AMF_SURFACE_YUV420P },
> > + { AV_PIX_FMT_BGR0, AMF_SURFACE_BGRA },
> > + { AV_PIX_FMT_YUV420P, AMF_SURFACE_YV12 },
> > + { AV_PIX_FMT_YUYV422, AMF_SURFACE_YUY2 },
>
> Do all of these formats actually work?
>
> > + { AV_PIX_FMT_D3D11, AMF_SURFACE_NV12 },
>
> D3D11 surfaces need not be NV12. The actual format is in
> AVHWFramesContext.sw_format - if you only support 8-bit then something
> nasty probably happens if you give it P010 surfaces.
>
> > +};
> > +
> > +static enum AMF_SURFACE_FORMAT amf_av_to_amf_format(enum
> AVPixelFormat fmt)
> > +{
> > + for (int i = 0; i < amf_countof(format_map); i++) {
> > + if (format_map[i].av_format == fmt) {
> > + return format_map[i].amf_format;
> > + }
> > + }
> > + return AMF_SURFACE_UNKNOWN;
> > +}
> > +
> > +// virtual functions decalred
>
> What does this comment mean?
>
> > +static void AMF_CDECL_CALL AMFTraceWriter_Write(AMFTraceWriter*
> pThis,
> > + const wchar_t* scope, const wchar_t* message)
>
> "type *variable" is preferred to "type* variable".
>
> Also, trailing space.
>
> > +{
> > + AmfTraceWriter *tracer = (AmfTraceWriter*)pThis;
> > +#if AMF_DEBUG_TRACE
> > + av_log(tracer->avctx, AV_LOG_INFO, "%ls: %ls", scope, message);
> > +#else
> > + av_log(tracer->avctx, AV_LOG_TRACE, "%ls: %ls", scope, message);
> > +#endif
>
> What sort of messages actually come out of this trace function?
>
> If this is intended for debugging (and should never be seen by the user), just
> make them AV_LOG_DEBUG and drop the AMF_DEBUG_TRACE define.
>
> > +}
> > +
> > +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)
> > +{
> > + AmfContext *ctx = avctx->priv_data;
> > + AMFInit_Fn init_fun = 0;
>
> NULL
>
> > + AMFQueryVersion_Fn version_fun = 0;
>
> NULL
>
> > +
> > + ctx->library = dlopen(AMF_DLL_NAMEA, RTLD_NOW | RTLD_LOCAL);
> > + AMF_RETURN_IF_FALSE(ctx, ctx->library != 0,
>
> Just ctx->library.
>
> Also trailing space.
>
> > + 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 != 0, 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, init_fun != 0, AVERROR_UNKNOWN, "DLL
> %s failed to find function %s. \n", AMF_DLL_NAMEA,
> AMF_QUERY_VERSION_FUNCTION_NAME);
> > +
> > + version_fun(&ctx->version);
> > + init_fun(AMF_FULL_VERSION, &ctx->factory);
> > + ctx->factory->pVtbl->GetTrace(ctx->factory, &ctx->trace);
> > + ctx->factory->pVtbl->GetDebug(ctx->factory, &ctx->debug);
>
> Do all of these functions necessarily succeed?
>
> > + return 0;
> > +}
> > +
> > +
> > +static int amf_init_context(AVCodecContext *avctx)
> > +{
> > + AmfContext *ctx = avctx->priv_data;
> > + AMF_RESULT res = AMF_OK;
> > +
> > + ctx->trace->pVtbl->EnableWriter(ctx->trace,
> AMF_TRACE_WRITER_CONSOLE, false);
>
> Using false probably wants <stdbool.h> and checking compiler support for it?
> Since compilers without stdbool are supported, we generally use 0 and 1
> instead.
>
> > +
> > +#if AMF_DEBUG_TRACE
> > + ctx->trace->pVtbl->EnableWriter(ctx->trace,
> AMF_TRACE_WRITER_DEBUG_OUTPUT, true);
> > + ctx->trace->pVtbl->SetWriterLevel(ctx->trace,
> AMF_TRACE_WRITER_DEBUG_OUTPUT, AMF_TRACE_TRACE);
> > + ctx->trace->pVtbl->SetGlobalLevel(ctx->trace, AMF_TRACE_TRACE);
> > +#else
> > + ctx->trace->pVtbl->EnableWriter(ctx->trace,
> AMF_TRACE_WRITER_DEBUG_OUTPUT, false);
> > +#endif
> > + ctx->tracer.vtbl = &tracer_vtbl;
> > + ctx->tracer.avctx = avctx;
> > + ctx->trace->pVtbl->RegisterWriter(ctx->trace,
> FFMPEG_AMF_WRITER_ID,
> > + (AMFTraceWriter*)&ctx->tracer, true);
>
> Can any of these functions fail?
>
> > +
> > + res = ctx->factory->pVtbl->CreateContext(ctx->factory, &ctx->context);
> > + AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN,
> "CreateContext() failed with error %d", res);
>
> Newline.
>
> > +
> > + // try to reuse existing DX device
> > +
> > + if (avctx->hw_frames_ctx) {
> > + AVHWFramesContext *device_ctx = (AVHWFramesContext*)avctx-
> >hw_frames_ctx->data;
> > + if (device_ctx->device_ctx->type == AV_HWDEVICE_TYPE_D3D11VA){
> > + if (device_ctx->device_ctx->hwctx) {
> > + AVD3D11VADeviceContext *device_d3d11 =
> (AVD3D11VADeviceContext *)device_ctx->device_ctx->hwctx;
> > + res = ctx->context->pVtbl->InitDX11(ctx->context, device_d3d11-
> >device, AMF_DX11_1);
> > + if (res == AMF_OK) {
> > + ctx->hw_frames_ctx = av_buffer_ref(avctx->hw_frames_ctx);
> > + } else {
> > + av_log(avctx, AV_LOG_INFO, "amf_shared: avctx-
> >hw_frames_ctx has non-AMD device, switching to default");
>
> I'm not sure this is going to act sensibly - if the user has D3D11 frames input
> on another device, does it work?
>
> Also newline.
>
> > + }
> > +
> > + }
> > + }
> > + } else if (avctx->hw_device_ctx) {
> > + AVHWDeviceContext *device_ctx = (AVHWDeviceContext*)(avctx-
> >hw_device_ctx->data);
> > + if (device_ctx->type == AV_HWDEVICE_TYPE_D3D11VA) {
> > + if (device_ctx->hwctx) {
> > + AVD3D11VADeviceContext *device_d3d11 =
> (AVD3D11VADeviceContext *)device_ctx->hwctx;
> > + res = ctx->context->pVtbl->InitDX11(ctx->context, device_d3d11-
> >device, AMF_DX11_1);
> > + if (res == AMF_OK) {
> > + ctx->hw_device_ctx = av_buffer_ref(avctx->hw_device_ctx);
> > + } else {
> > + av_log(avctx, AV_LOG_INFO, "amf_shared: avctx-
> >hw_device_ctx has non-AMD device, switching to default");
>
> Newline.
>
> > + }
> > + }
> > + }
> > + }
> > +
> > + if (!ctx->hw_frames_ctx) {
> > + res = ctx->context->pVtbl->InitDX11(ctx->context, 0, AMF_DX11_1);
> > + if (res != AMF_OK) {
> > + res = ctx->context->pVtbl->InitDX9(ctx->context, 0);
> > + AMF_RETURN_IF_FALSE(ctx, res == AMF_OK,
> AVERROR_UNKNOWN, "InitDX9() failed with error %d", res);
>
> Newline, and also in more messages below which I won't point out
> individually.
>
> > + }
> > + }
> > + return 0;
> > +}
> > +
> > +static int amf_init_encoder(AVCodecContext *avctx)
> > +{
> > + AmfContext *ctx = avctx->priv_data;
> > + const wchar_t *codec_id = 0;
>
> NULL.
>
> > + AMF_RESULT res = AMF_OK;
> > +
> > + switch (avctx->codec->id) {
> > + case AV_CODEC_ID_H264:
> > + codec_id = AMFVideoEncoderVCE_AVC;
> > + break;
> > + case AV_CODEC_ID_HEVC:
> > + codec_id = AMFVideoEncoder_HEVC;
> > + break;
> > + default:
> > + break;
> > + }
> > + AMF_RETURN_IF_FALSE(ctx, codec_id != 0, AVERROR(EINVAL), "Codec
> %d is not supported", avctx->codec->id);
> > +
> > + ctx->format = amf_av_to_amf_format(avctx->pix_fmt);
> > + AMF_RETURN_IF_FALSE(ctx, ctx->format != AMF_SURFACE_UNKNOWN,
> AVERROR(EINVAL), "Format %d is not supported", avctx->pix_fmt);
> > +
> > + res = ctx->factory->pVtbl->CreateComponent(ctx->factory, ctx->context,
> codec_id, &ctx->encoder);
> > + AMF_RETURN_IF_FALSE(ctx, res == AMF_OK,
> AVERROR_ENCODER_NOT_FOUND, "CreateComponent(%S) failed with error
> %d", codec_id, res);
>
> "%S" is not standard, please use %ls.
>
> > +
> > + ctx->eof = false;
> > + return 0;
> > +}
> > +
> > +static int amf_terminate(AVCodecContext *avctx)
> > +{
> > + AmfContext* ctx = avctx->priv_data;
>
> * next to variable.
>
> > +
> > + if (ctx->encoder) {
> > + ctx->encoder->pVtbl->Terminate(ctx->encoder);
> > + ctx->encoder->pVtbl->Release(ctx->encoder);
> > + ctx->encoder = 0;
>
> NULL.
>
> > + }
> > +
> > + if (ctx->context) {
> > + ctx->context->pVtbl->Terminate(ctx->context);
> > + ctx->context->pVtbl->Release(ctx->context);
> > + ctx->context = 0;
>
> NULL.
>
> > + }
> > + if (ctx->hw_device_ctx){
> > + av_buffer_unref(&ctx->hw_device_ctx);
> > + ctx->hw_device_ctx = 0;
> > + }
> > + if (ctx->hw_frames_ctx) {
> > + av_buffer_unref(&ctx->hw_frames_ctx);
> > + ctx->hw_frames_ctx = 0;
> > + }
>
> Just use av_buffer_unref() without the checks or setting to null.
>
> > +
> > + if (ctx->trace) {
> > + ctx->trace->pVtbl->UnregisterWriter(ctx->trace,
> FFMPEG_AMF_WRITER_ID);
> > + }
> > +
> > + if (ctx->library) {
> > + dlclose(ctx->library);
> > + ctx->library = 0;
> > + }
> > + ctx->trace = 0;
> > + ctx->debug = 0;
> > + ctx->factory = 0;
> > + ctx->version = 0;
> > +
> > + return 0;
> > +}
> > +
> > +static int amf_copy_surface(AVCodecContext *avctx, const AVFrame
> *frame,
> > + AMFSurface* surface)
> > +{
> > + AmfContext *ctx = avctx->priv_data;
> > + AMFPlane *plane = 0;
> > + uint8_t *dst_data[4];
> > + int dst_linesize[4];
> > +
> > + int planes = (int)surface->pVtbl->GetPlanesCount(surface);
> > + AMF_RETURN_IF_FALSE(ctx, planes <= amf_countof(dst_data),
> AVERROR(EINVAL), "Invalid number of planes %d in surface", planes);
> > +
> > + for (int i = 0; i < planes; i++) {
>
> Declare variables at the start of the block.
>
> > + plane = surface->pVtbl->GetPlaneAt(surface, i);
> > + dst_data[i] = plane->pVtbl->GetNative(plane);
> > + dst_linesize[i] = plane->pVtbl->GetHPitch(plane);
> > + }
> > + av_image_copy(dst_data, dst_linesize,
> > + (const uint8_t**)frame->data, frame->linesize, frame->format,
> > + avctx->width, avctx->height);
> > +
> > + surface->pVtbl->SetPts(surface, frame->pts);
>
> Does this accept the same range as frame->pts, including AV_NOPTS_VALUE?
>
> > +
> > + return 0;
> > +}
> > +
> > +static int amf_copy_buffer(AVCodecContext *avctx, AVPacket *pkt,
> AMFBuffer *buffer)
> > +{
> > + int ret = 0;
>
> Pointless initialisation?
>
> > + AMFVariantStruct var;
> > + int size = (int)buffer->pVtbl->GetSize(buffer);
> > +
> > + if (ret = ff_alloc_packet2(avctx, pkt, size, 0)) {
>
> Check for ret negative.
>
> > + return ret;
> > + }
> > + memcpy(pkt->data, buffer->pVtbl->GetNative(buffer), size);
> > +
> > + switch (avctx->codec->id) {
> > + case AV_CODEC_ID_H264:
> > + buffer->pVtbl->GetProperty(buffer,
> AMF_VIDEO_ENCODER_OUTPUT_DATA_TYPE, &var);
> > + switch (var.int64Value) {
> > + case AMF_VIDEO_ENCODER_OUTPUT_DATA_TYPE_IDR:
> > + pkt->flags = AV_PICTURE_TYPE_I | AV_PKT_FLAG_KEY;
>
> The AV_PICTURE_TYPE does not go in AVPacket.flags.
>
> > + break;
> > + case AMF_VIDEO_ENCODER_OUTPUT_DATA_TYPE_I:
> > + pkt->flags = AV_PICTURE_TYPE_I;
> > + break;
> > + case AMF_VIDEO_ENCODER_OUTPUT_DATA_TYPE_P:
> > + pkt->flags = AV_PICTURE_TYPE_P;
> > + break;
> > + case AMF_VIDEO_ENCODER_OUTPUT_DATA_TYPE_B:
> > + pkt->flags = AV_PICTURE_TYPE_B;
> > + break;
> > + default:
> > + av_log(avctx, AV_LOG_ERROR, "Unknown picture type
> encountered, expect the output to be broken.\n");
>
> When can this happen?
>
> > + break;
> > + }
> > + break;
> > + case AV_CODEC_ID_HEVC:
> > + buffer->pVtbl->GetProperty(buffer,
> AMF_VIDEO_ENCODER_HEVC_OUTPUT_DATA_TYPE, &var);
> > + switch (var.int64Value) {
> > + case AMF_VIDEO_ENCODER_HEVC_OUTPUT_DATA_TYPE_IDR:
> > + pkt->flags = AV_PICTURE_TYPE_I | AV_PKT_FLAG_KEY;
> > + break;
> > + case AMF_VIDEO_ENCODER_HEVC_OUTPUT_DATA_TYPE_I:
> > + pkt->flags = AV_PICTURE_TYPE_I | AV_PKT_FLAG_KEY;
>
> All intra picture generated by this encoder are necessarily IRAP?
>
> > + break;
> > + case AMF_VIDEO_ENCODER_HEVC_OUTPUT_DATA_TYPE_P:
> > + pkt->flags = AV_PICTURE_TYPE_P;
> > + break;
> > + default:
> > + av_log(avctx, AV_LOG_ERROR, "Unknown picture type
> encountered, expect the output to be broken.\n");
> > + break;
> > + }
> > + break;
> > + default:
> > + break;
> > + }
> > + pkt->pts = buffer->pVtbl->GetPts(buffer);
> > + pkt->dts = pkt->pts;
> > + return 0;
> > +}
> > +
> > +// amfenc API implmentation
> > +int ff_amf_encode_init(AVCodecContext *avctx)
> > +{
> > + AmfContext *ctx = avctx->priv_data;
> > + int ret = 0;
>
> Pointless initialisation.
>
> > +
> > + ctx->factory = 0;
> > + ctx->debug = 0;
> > + ctx->trace = 0;
> > + ctx->context = 0;
> > + ctx->encoder = 0;
> > + ctx->library = 0;
> > + ctx->version = 0;
> > + ctx->eof = 0;
> > + ctx->format = 0;
> > + ctx->tracer.vtbl = 0;
> > + ctx->tracer.avctx = 0;
>
> Some of these should probably be NULL.
>
> > +
> > + if ((ret = amf_load_library(avctx)) == 0) {
> > + if ((ret = amf_init_context(avctx)) == 0) {
> > + if ((ret = amf_init_encoder(avctx)) == 0) {
> > + return 0;
> > + }
> > + }
> > + }
> > + amf_terminate(avctx);
> > + return ret;
> > +}
> > +
> > +int av_cold ff_amf_encode_close(AVCodecContext *avctx)
> > +{
> > + int ret = 0;
>
> Pointless initialisation.
>
> > + ret = amf_terminate(avctx);
> > + return ret;
> > +}
> > +
> > +static GUID AMFTextureArrayIndexGUID =
> > +{ 0x28115527, 0xe7c3, 0x4b66, { 0x99, 0xd3, 0x4f, 0x2a, 0xe6, 0xb4, 0x7f,
> 0xaf } };
>
> This seems like something that really shouldn't be hardcoded like this here.
>
> > +
> > +int ff_amf_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
> > + const AVFrame *frame, int *got_packet)
> > +{
> > + int ret = 0;
> > + AMF_RESULT res = AMF_OK;
> > + AmfContext *ctx = avctx->priv_data;
> > + AMFSurface *surface = 0;
> > + AMFData *data = 0;
> > + amf_bool submitted = false;
> > +
> > + while (!submitted) {
> > + if (!frame) { // submit drain
> > + if (!ctx->eof) { // submit drain onre time only
> > + res = ctx->encoder->pVtbl->Drain(ctx->encoder);
> > + if (res == AMF_INPUT_FULL) {
> > + av_usleep(1000); // input queue is full: wait, poll and submit
> Drain again
> > + // need to get some output and try again
> > + } else if (res == AMF_OK) {
> > + ctx->eof = true; // drain started
> > + submitted = true;
> > + }
> > + }
> > + } else { // submit frame
> > + if (surface == 0) { // prepare surface from frame one time only
> > + if (frame->hw_frames_ctx && ( // HW frame detected
> > + // check if the same hw_frames_ctx as used in
> initialization
> > + (ctx->hw_frames_ctx && frame->hw_frames_ctx->data == ctx-
> >hw_frames_ctx->data) ||
> > + // check if the same hw_device_ctx as used in initialization
> > + (ctx->hw_device_ctx && ((AVHWFramesContext*)frame-
> >hw_frames_ctx->data)->device_ctx ==
> > + (AVHWDeviceContext*)ctx->hw_device_ctx->data)
> > + )) {
> > + ID3D11Texture2D* texture = (ID3D11Texture2D*)frame-
> >data[0]; // actual texture
> > + int index = (int)(size_t)frame->data[1]; // index is a slice in
> texture array is - set to tell AMF which slice to use
> > + texture->lpVtbl->SetPrivateData(texture,
> &AMFTextureArrayIndexGUID, sizeof(index), &index);
> > +
> > + res = ctx->context->pVtbl->CreateSurfaceFromDX11Native(ctx-
> >context, texture, &surface, NULL); // wrap to AMF surface
> > + surface->pVtbl->SetCrop(surface, 0, 0, frame->width, frame-
> >height); // decode surfaces are vertically aligned by 16 tell AMF real size
> > + surface->pVtbl->SetPts(surface, frame->pts);
> > + } else {
> > + res = ctx->context->pVtbl->AllocSurface(ctx->context,
> AMF_MEMORY_HOST, ctx->format, avctx->width, avctx->height, &surface);
> > + AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_BUG,
> "AllocSurface() failed with error %d", res);
> > + amf_copy_surface(avctx, frame, surface);
> > + }
> > + }
> > + // encode
> > + res = ctx->encoder->pVtbl->SubmitInput(ctx->encoder,
> (AMFData*)surface);
> > + if (res == AMF_INPUT_FULL) { // handle full queue
> > + av_usleep(1000); // input queue is full: wait, poll and submit
> surface again
>
> Is there really no way in the API to wait for this properly?
>
> > + } else {
> > + surface->pVtbl->Release(surface);
> > + surface = NULL;
> > + AMF_RETURN_IF_FALSE(ctx, res == AMF_OK,
> AVERROR_UNKNOWN, "SubmitInput() failed with error %d", res);
> > + submitted = 1;
> > + }
> > + }
> > + // poll results
> > + if (!data) {
> > + res = ctx->encoder->pVtbl->QueryOutput(ctx->encoder, &data);
> > + if (data) {
> > + AMFBuffer* buffer;
> > + AMFGuid guid = IID_AMFBuffer();
> > + data->pVtbl->QueryInterface(data, &guid, (void**)&buffer); //
> query for buffer interface
> > + ret = amf_copy_buffer(avctx, pkt, buffer);
> > + if (!ret)
> > + *got_packet = 1;
> > + buffer->pVtbl->Release(buffer);
> > + data->pVtbl->Release(data);
> > + if (ctx->eof) {
> > + submitted = true; // we are in the drain state - no submissions
> > + }
> > + } else if (res == AMF_EOF) {
> > + submitted = true; // drain complete
> > + } else {
> > + if (!submitted) {
> > + av_usleep(1000); // wait and poll again
> > + }
> > + }
> > + }
> > + }
>
> I suspect this setup is not actually going to follow the constraints of the
> deprecated encode2(). Given the API here, I think you would be much better
> off writing with send_frame()/receive_packet().
>
> > + return ret;
> > +}
> > diff --git a/libavcodec/amfenc.h b/libavcodec/amfenc.h
> > new file mode 100644
> > index 0000000..6b0135a
> > --- /dev/null
> > +++ b/libavcodec/amfenc.h
> > @@ -0,0 +1,131 @@
> > +/*
> > +* This file is part of FFmpeg.
> > +*
> > +* FFmpeg is free software; you can redistribute it and/or
> > +* modify it under the terms of the GNU Lesser General Public
> > +* License as published by the Free Software Foundation; either
> > +* version 2.1 of the License, or (at your option) any later version.
> > +*
> > +* FFmpeg is distributed in the hope that it will be useful,
> > +* but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> GNU
> > +* Lesser General Public License for more details.
> > +*
> > +* You should have received a copy of the GNU Lesser General Public
> > +* License along with FFmpeg; if not, write to the Free Software
> > +* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA
> > +*/
> > +
> > +#ifndef AVCODEC_AMFENC_H
> > +#define AVCODEC_AMFENC_H
> > +
> > +#include "config.h"
> > +#include "avcodec.h"
> > +//#include "compat/amd/amf/public/include/core/Factory.h"
>
> Commented out?
>
> > +#include "compat/amd/amfsdkenc.h"
> > +
> > +
> > +/**
> > +* AMF trace writer callback class
> > +* Used to capture all AMF logging
> > +*/
> > +
> > +typedef struct AmfTraceWriter {
> > + AMFTraceWriterVtbl* vtbl;
> > + AVCodecContext *avctx;
> > +} AmfTraceWriter;
> > +
> > +/**
> > +* AMF encoder context
> > +*/
> > +
> > +typedef struct AmfContext {
> > + AVClass* avclass;
> > + /** access to AMF runtime */
> > + amf_handle library; ///< handle to DLL library
> > + AMFFactory* factory; ///< pointer to AMF factory
> > + AMFDebug* debug; ///< pointer to AMF debug interface
> > + AMFTrace* trace; ///< pointer to AMF trace interface
> > +
> > + amf_uint64 version; ///< version of AMF runtime
> > + AmfTraceWriter tracer; ///< AMF writer registered with AMF
> > + AMFContext* context; ///< AMF context
> > + //encoder
> > + AMFComponent* encoder; ///< AMF encoder object
> > + amf_bool eof; ///< flag indicating EOF happened
> > + AMF_SURFACE_FORMAT format; ///< AMF surface format
> > +
> > + AVBufferRef *hw_device_ctx; ///< pointer to HW accelerator
> (decoder)
> > + AVBufferRef *hw_frames_ctx; ///< pointer to HW accelerator
> (frame allocator)
> > +
> > + /** common encoder option options */
> > +
> > + /** Static options, have to be set before Init() call */
> > + int usage;
> > + int profile;
> > + int level;
> > + int preanalysis;
> > + int quality;
> > + int b_frame_delta_qp;
> > + int ref_b_frame_delta_qp;
> > +
> > + /** Dynamic options, can be set after Init() call */
> > +
> > + int rate_control_mode;
> > + int enforce_hrd;
> > + int filler_data;
> > + int enable_vbaq;
> > + int skip_frame;
> > + int qp_i;
> > + int qp_p;
> > + int qp_b;
> > + int max_au_size;
> > + int header_spacing;
> > + int b_frame_ref;
> > + int intra_refresh_mb;
> > + int slices;
> > + int coding_mode;
> > + int me_half_pel;
> > + int me_quater_pel;
> > +
> > + /** HEVC - specific options */
> > +
> > + int gops_per_idr;
> > + int header_insertion_mode;
> > + int min_qp_i;
> > + int max_qp_i;
> > + int min_qp_p;
> > + int max_qp_p;
> > + int tier;
>
> There are lots of tabs and strange indentation here.
>
> > +} AmfContext;
> > +
> > +/**
> > +* Common encoder initization code
> > +*/
> > +int ff_amf_encode_init(AVCodecContext *avctx);
> > +/**
> > +* Common encoder termination code
> > +*/
> > +int ff_amf_encode_close(AVCodecContext *avctx);
> > +
> > +/**
> > +* Ecoding one frame - common for all AMF encoders
> > +*/
> > +int ff_amf_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
> > + const AVFrame *frame, int *got_packet);
> > +
> > +/**
> > +* Supported formats
> > +*/
> > +extern const enum AVPixelFormat ff_amf_pix_fmts[];
> > +
> > +/**
> > +* Error handling helper
> > +*/
> > +#define AMF_RETURN_IF_FALSE(avctx, exp, ret_value, /*optional
> message,*/ ...) \
> > + if (!(exp)) { \
> > + av_log(avctx, AV_LOG_ERROR, __VA_ARGS__); \
> > + return AVERROR(ret_value); \
> > + }
> > +
> > +#endif //AVCODEC_AMFENC_H
> > \ No newline at end of file
>
> git has review comment for you too.
>
> > diff --git a/libavcodec/amfenc_h264.c b/libavcodec/amfenc_h264.c
> > new file mode 100644
> > index 0000000..a6e0f3c
> > --- /dev/null
> > +++ b/libavcodec/amfenc_h264.c
> > @@ -0,0 +1,467 @@
> > +/*
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA
> > + */
> > +
> > +#include "amfenc.h"
> > +//#include
> "compat/amd/amf/public/include/components/VideoEncoderVCE.h"
> > +#include "libavutil/opt.h"
> > +#include "libavutil/internal.h"
> > +#include "internal.h"
> > +
> > +#define OFFSET(x) offsetof(AmfContext, x)
> > +#define VE AV_OPT_FLAG_VIDEO_PARAM |
> AV_OPT_FLAG_ENCODING_PARAM
> > +
> > +static const AVOption options[] = {
> > + // Static
> > + /// Usage
> > + { "usage", "Encoder Usage", OFFSET(usage),
> AV_OPT_TYPE_INT, { .i64 = AMF_VIDEO_ENCODER_USAGE_TRANSCONDING
> }, AMF_VIDEO_ENCODER_USAGE_TRANSCONDING,
> AMF_VIDEO_ENCODER_USAGE_WEBCAM, VE, "usage" },
> > + { "transcoding", "Generic Transcoding", 0,
> AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_USAGE_TRANSCONDING }, 0, 0, VE, "usage" },
> > + { "ultralowlatency","", 0, AV_OPT_TYPE_CONST, { .i64
> = AMF_VIDEO_ENCODER_USAGE_ULTRA_LOW_LATENCY }, 0, 0, VE, "usage"
> },
> > + { "lowlatency", "", 0, AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_USAGE_LOW_LATENCY }, 0, 0, VE, "usage" },
> > + { "webcam", "Webcam", 0, AV_OPT_TYPE_CONST, {
> .i64 = AMF_VIDEO_ENCODER_USAGE_WEBCAM }, 0, 0, VE, "usage" },
> > +
> > + /// Profile,
> > + { "profile", "Profile", OFFSET(profile),AV_OPT_TYPE_INT, {
> .i64 = AMF_VIDEO_ENCODER_PROFILE_MAIN },
> AMF_VIDEO_ENCODER_PROFILE_BASELINE,
> AMF_VIDEO_ENCODER_PROFILE_HIGH, VE, "profile" },
> > + { "baseline", "", 0, AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_PROFILE_BASELINE }, 0, 0, VE, "profile" },
>
> Do you really support baseline profile H.264? You probably mean
> constrained baseline.
>
> > + { "main", "", 0, AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_PROFILE_MAIN }, 0, 0, VE, "profile" },
> > + { "high", "", 0, AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_PROFILE_HIGH }, 0, 0, VE, "profile" },
> > +
> > + /// Profile Level
> > + { "level", "Profile Level", OFFSET(level), AV_OPT_TYPE_INT, {
> .i64 = 0 }, 0, 62, VE, "level" },
> > + { "auto", "", 0, AV_OPT_TYPE_CONST, { .i64 = 0
> }, 0, 0, VE, "level" },
> > + { "1.0", "", 0, AV_OPT_TYPE_CONST, { .i64 = 10
> }, 0, 0, VE, "level" },
> > + { "1.1", "", 0, AV_OPT_TYPE_CONST, { .i64 = 11
> }, 0, 0, VE, "level" },
> > + { "1.2", "", 0, AV_OPT_TYPE_CONST, { .i64 = 12
> }, 0, 0, VE, "level" },
> > + { "1.3", "", 0, AV_OPT_TYPE_CONST, { .i64 = 13
> }, 0, 0, VE, "level" },
> > + { "2.0", "", 0, AV_OPT_TYPE_CONST, { .i64 = 20
> }, 0, 0, VE, "level" },
> > + { "2.1", "", 0, AV_OPT_TYPE_CONST, { .i64 = 21
> }, 0, 0, VE, "level" },
> > + { "2.2", "", 0, AV_OPT_TYPE_CONST, { .i64 = 22
> }, 0, 0, VE, "level" },
> > + { "3.0", "", 0, AV_OPT_TYPE_CONST, { .i64 = 30
> }, 0, 0, VE, "level" },
> > + { "3.1", "", 0, AV_OPT_TYPE_CONST, { .i64 = 31
> }, 0, 0, VE, "level" },
> > + { "3.2", "", 0, AV_OPT_TYPE_CONST, { .i64 = 32
> }, 0, 0, VE, "level" },
> > + { "4.0", "", 0, AV_OPT_TYPE_CONST, { .i64 = 40
> }, 0, 0, VE, "level" },
> > + { "4.1", "", 0, AV_OPT_TYPE_CONST, { .i64 = 41
> }, 0, 0, VE, "level" },
> > + { "4.2", "", 0, AV_OPT_TYPE_CONST, { .i64 = 42
> }, 0, 0, VE, "level" },
> > + { "5.0", "", 0, AV_OPT_TYPE_CONST, { .i64 = 50
> }, 0, 0, VE, "level" },
> > + { "5.1", "", 0, AV_OPT_TYPE_CONST, { .i64 = 51
> }, 0, 0, VE, "level" },
> > + { "5.2", "", 0, AV_OPT_TYPE_CONST, { .i64 = 52
> }, 0, 0, VE, "level" },
> > + { "6.0", "", 0, AV_OPT_TYPE_CONST, { .i64 = 60
> }, 0, 0, VE, "level" },
> > + { "6.1", "", 0, AV_OPT_TYPE_CONST, { .i64 = 61
> }, 0, 0, VE, "level" },
> > + { "6.2", "", 0, AV_OPT_TYPE_CONST, { .i64 = 62
> }, 0, 0, VE, "level" },
>
> These private options for profile and level are fine, but you should read
> AVCodecContext.(profile|level) first.
>
> > +
> > + /// Quality Preset
> > + { "quality", "Quality Preference", OFFSET(quality),
> AV_OPT_TYPE_INT, { .i64 = AMF_VIDEO_ENCODER_QUALITY_PRESET_SPEED
> }, AMF_VIDEO_ENCODER_QUALITY_PRESET_BALANCED,
> AMF_VIDEO_ENCODER_QUALITY_PRESET_QUALITY, VE, "quality" },
> > + { "speed", "Prefer Speed", 0,
> AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_QUALITY_PRESET_SPEED }, 0, 0, VE, "quality" },
> > + { "balanced", "Balanced", 0,
> AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_QUALITY_PRESET_BALANCED }, 0, 0, VE, "quality" },
> > + { "quality", "Prefer Quality", 0,
> AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_QUALITY_PRESET_QUALITY }, 0, 0, VE, "quality" },
> > +
> > + // Dynamic
> > + /// Rate Control Method
> > + { "rc", "Rate Control Method",
> OFFSET(rate_control_mode), AV_OPT_TYPE_INT, { .i64 =
> AMF_VIDEO_ENCODER_RATE_CONTROL_METHOD_PEAK_CONSTRAINED_VB
> R }, AMF_VIDEO_ENCODER_RATE_CONTROL_METHOD_CONSTANT_QP,
> AMF_VIDEO_ENCODER_RATE_CONTROL_METHOD_LATENCY_CONSTRAINED
> _VBR, VE, "rc" },
> > + { "cqp", "Constant Quantization Parameter", 0,
> AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_RATE_CONTROL_METHOD_CONSTANT_QP },
> 0, 0, VE, "rc" },
> > + { "cbr", "Constant Bitrate", 0,
> AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_RATE_CONTROL_METHOD_CBR }, 0, 0,
> VE, "rc" },
> > + { "vbr_peak", "Peak Contrained Variable Bitrate", 0,
> AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_RATE_CONTROL_METHOD_PEAK_CONSTRAINED_VB
> R }, 0, 0, VE, "rc" },
> > + { "vbr_latency", "Latency Constrained Variable Bitrate", 0,
> AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_RATE_CONTROL_METHOD_LATENCY_CONSTRAINED
> _VBR }, 0, 0, VE, "rc" },
> > +
> > + /// Enforce HRD, Filler Data, VBAQ, Frame Skipping
> > + { "enforce_hrd", "Enforce HRD", OFFSET(enforce_hrd),
> AV_OPT_TYPE_BOOL,{ .i64 = 0 }, 0, 1, VE, NULL },
> > + { "filler_data", "Filler Data Enable", OFFSET(filler_data),
> AV_OPT_TYPE_BOOL,{ .i64 = 0 }, 0, 1, VE, NULL },
> > + { "vbaq", "Enable VBAQ", OFFSET(enable_vbaq),
> AV_OPT_TYPE_BOOL,{ .i64 = 0 }, 0, 1, VE, NULL },
> > + { "frame_skipping", "Rate Control Based Frame Skip",
> OFFSET(skip_frame), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE, NULL },
> > +
> > + /// QP Values
> > + { "qp_i", "Quantization Parameter for I-Frame", OFFSET(qp_i),
> AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 51, VE },
> > + { "qp_p", "Quantization Parameter for P-Frame", OFFSET(qp_p),
> AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 51, VE },
> > + { "qp_b", "Quantization Parameter for B-Frame", OFFSET(qp_b),
> AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 51, VE },
> > +
> > + /// Pre-Pass, Pre-Analysis, Two-Pass
> > + { "preanalysis", "Pre-Analysis Mode", OFFSET(preanalysis),
> AV_OPT_TYPE_BOOL,{ .i64 = 0 }, 0, 1, VE, NULL },
> > +
> > + /// Maximum Access Unit Size
> > + { "max_au_size", "Maximum Access Unit Size (in bits)",
> OFFSET(max_au_size), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, VE,
> NULL },
>
> Maximum access unit size seems like a slightly strange thing to set - the HRD
> parameters and rate control should define how this works.
>
> Is it actually maximum NAL unit size? (For RFC 6184 single NAL unit packets.)
>
> > +
> > + /// Header Insertion Spacing
> > + { "header_spacing", "Header Insertion Spacing",
> OFFSET(header_spacing), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 1000, VE },
> > +
> > + /// B-Frames
> > + // BPicturesPattern=bf
> > + { "bf_delta_qp", "B-Picture Delta QP",
> OFFSET(b_frame_delta_qp), AV_OPT_TYPE_INT, { .i64 = 4 }, -10, 10, VE,
> NULL },
> > + { "bf_ref", "Enable Reference to B-Frames",
> OFFSET(b_frame_ref), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, VE, NULL },
> > + { "bf_ref_delta_qp","Reference B-Picture Delta QP",
> OFFSET(ref_b_frame_delta_qp), AV_OPT_TYPE_INT, { .i64 = 4 }, -10, 10, VE,
> NULL },
> > +
> > + /// Intra-Refresh
> > + { "intra_refresh_mb","Intra Refresh MBs Number Per Slot in
> Macroblocks", OFFSET(intra_refresh_mb), AV_OPT_TYPE_INT, { .i64 = 0
> }, 0, INT_MAX, VE },
> > + { "slices", "Number of Slices per Frame", OFFSET(slices),
> AV_OPT_TYPE_INT, { .i64 = 1 } , 1, 8160, VE, NULL },
>
> Use AVCodecContext.slices rather than a private option.
>
> > +
> > + /// coder
> > + { "coding", "Coding Type", OFFSET(coding_mode),
> AV_OPT_TYPE_INT, { .i64 = AMF_VIDEO_ENCODER_UNDEFINED },
> AMF_VIDEO_ENCODER_UNDEFINED, AMF_VIDEO_ENCODER_CALV, VE,
> "coding" },
> > + { "auto", "Automatic", 0,
> AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_UNDEFINED }, 0, 0,
> VE, "coding" },
> > + { "cavlc", "Context Adaptive Variable-Length Coding", 0,
> AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_CALV }, 0, 0, VE,
> "coding" },
> > + { "cabac", "Context Adaptive Binary Arithmetic Coding", 0,
> AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_CABAC }, 0, 0, VE,
> "coding" },
>
> Other encoders have the same option named "coder", it might be nice to be
> consistent.
>
> > +
> > + { "me_half_pel", "Enable ME Half Pixel",
> OFFSET(me_half_pel), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, VE, NULL },
> > + { "me_quater_pel", "Enable ME Quarter Pixel ",
> OFFSET(me_quater_pel), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, VE, NULL },
> > +
> > + { NULL }
> > +};
>
> I think these options could do with some better documentation (maybe in
> the texinfo, though, and it can be a later patch).
>
> > +
> > +static av_cold int amf_encode_init_h264(AVCodecContext *avctx)
> > +{
> > + int ret = 0;
> > + AMF_RESULT res = AMF_OK;
> > + AmfContext *ctx = avctx->priv_data;
> > + AMFVariantStruct var = {0};
> > + amf_int64 profile_level = 0;
> > +
> > + AMFSize framesize = AMFConstructSize(avctx->width, avctx-
> >height);
> > + AMFRate framerate = AMFConstructRate(avctx->time_base.den,
> avctx->time_base.num * avctx->ticks_per_frame);
>
> Is VFR encoding not supported?
>
> > +
> > + int deblocking_filter = (avctx->flags &
> AV_CODEC_FLAG_LOOP_FILTER) ? 1 : 0;
> > +
> > + if ((ret = ff_amf_encode_init(avctx)) != 0)
> > + return ret;
> > +
> > + // Static parameters
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_USAGE, ctx->usage);
> > +
> > + AMF_ASSIGN_PROPERTY_SIZE(res, ctx->encoder,
> AMF_VIDEO_ENCODER_FRAMESIZE, framesize);
> > +
> > + AMF_ASSIGN_PROPERTY_RATE(res, ctx->encoder,
> AMF_VIDEO_ENCODER_FRAMERATE, framerate);
> > +
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_PROFILE, ctx->profile);
>
> Check avctx->profile as well.
>
> > +
> > + profile_level = ctx->level;
>
> Check avctx->level as well.
>
> > + if (profile_level == 0) {
> > + // Automatic detection of correct profile level.
> > + struct {
> > + uint64_t max_samples;
> > + uint64_t max_samples_per_sec;
> > + int level;
> > + } restrictions[] = {
> > + { 25344, 380160, 10 },
> > + { 101376, 768000, 11 },
> > + { 101376, 1536000, 12 },
> > + //{ 101376, 3041280, 13 }, // Backwards compatible 2.0
> > + { 101376, 3041280, 20 },
> > + { 202752, 5068800, 21 },
> > + { 414720, 5184000, 22 },
> > + { 414720, 10368000, 30 },
> > + { 921600, 27648000, 31 },
> > + { 1310720, 55296000, 32 },
> > + //{ 2097152, 62914560, 40 }, // Backwards compatible 4.1
> > + { 2097152, 62914560, 41 },
> > + { 2228224, 133693440, 42 },
> > + { 5652480, 150994994, 50 },
> > + { 9437184, 251658240, 51 },
> > + { 9437184, 530841600, 52 },
> > + { 35651584, 1069547520, 60 },
> > + { 35651584, 2139095040, 61 },
> > + { 35651584, 4278190080, 62 },
> > + { 0, 0, -1 }
> > + };
> > + uint64_t samples = framesize.width * framesize.height;
> > + uint64_t samples_per_sec = (samples * framerate.num) /
> framerate.den;
> > + profile_level = 52; // Default to 5.2 for now.
> > + for (unsigned int index = 0; restrictions[index].level != -1; index++) {
> > + if ((samples < restrictions[index].max_samples)
> > + && (samples_per_sec <
> restrictions[index].max_samples_per_sec)) {
> > + profile_level = restrictions[index].level;
> > + break;
> > + }
> > + }
> > + }
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_PROFILE_LEVEL, profile_level);
>
> Um, does this really have to be done outside the encoder?
>
> > +
> > + // Maximum Reference Frames
> > + if (avctx->refs != -1) {
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_MAX_NUM_REFRAMES, avctx->refs);
> > + }
> > + if (avctx->sample_aspect_ratio.den && avctx-
> >sample_aspect_ratio.num) {
> > + AMFRatio ratio = AMFConstructRatio(avctx-
> >sample_aspect_ratio.num, avctx->sample_aspect_ratio.den);
> > + AMF_ASSIGN_PROPERTY_RATIO(res, ctx->encoder,
> AMF_VIDEO_ENCODER_ASPECT_RATIO, ratio);
> > + }
> > +
> > + /// Color Range (Partial/TV/MPEG or Full/PC/JPEG)
> > + if (avctx->color_range == AVCOL_RANGE_JPEG) {
> > + AMF_ASSIGN_PROPERTY_BOOL(res, ctx->encoder,
> AMF_VIDEO_ENCODER_FULL_RANGE_COLOR, 1);
> > + }
> > +
> > + if (ctx->rate_control_mode ==
> AMF_VIDEO_ENCODER_RATE_CONTROL_METHOD_CONSTANT_QP) {
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_RATE_CONTROL_PREANALYSIS_ENABLE, false);
> > + if (ctx->preanalysis)
> > + av_log(ctx, AV_LOG_WARNING, "Pre-Analysis is not supported by
> cqp Rate Control Method, automatically disabled.");
>
> (More newlines missing.)
>
> > + } else {
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_RATE_CONTROL_PREANALYSIS_ENABLE, ctx-
> >preanalysis);
> > + }
> > +
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_QUALITY_PRESET, ctx->quality);
> > +
> > + // Initialize Encoder
> > + res = ctx->encoder->pVtbl->Init(ctx->encoder, ctx->format, avctx-
> >width, avctx->height);
> > + AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_BUG, "encoder-
> >Init() failed with error %d", res);
> > +
> > + // Dynamic parmaters
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_RATE_CONTROL_METHOD, ctx-
> >rate_control_mode);
> > +
> > + /// VBV Buffer
> > + if (avctx->rc_buffer_size != 0)
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_VBV_BUFFER_SIZE, avctx->rc_buffer_size);
> > + if (avctx->rc_initial_buffer_occupancy != 0) {
> > + int percent = avctx->rc_buffer_size * 64 / avctx-
> >rc_initial_buffer_occupancy;
> > + if (percent > 64)
> > + percent = 64;
>
> ???
>
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_INITIAL_VBV_BUFFER_FULLNESS, percent);
> > + }
> > + /// Maximum Access Unit Size
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_MAX_AU_SIZE, ctx->max_au_size);
> > +
> > +
> > + // QP Minimum / Maximum
> > + if (ctx->rate_control_mode ==
> AMF_VIDEO_ENCODER_RATE_CONTROL_METHOD_CONSTANT_QP) {
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_MIN_QP, 0);
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_MAX_QP, 51);
> > + } else {
> > + if (avctx->qmin != -1) {
> > + int qval = avctx->qmin > 51 ? 51 : avctx->qmin;
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_MIN_QP, qval);
> > + }
>
> Trailing spaces.
>
> > + if (avctx->qmax != -1) {
> > + int qval = avctx->qmax > 51 ? 51 : avctx->qmax;
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_MAX_QP, qval);
> > + }
> > + }
> > + // QP Values
> > + if (ctx->qp_i != -1)
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_QP_I, ctx->qp_i);
> > + if (ctx->qp_p != -1)
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_QP_P, ctx->qp_p);
> > + if (ctx->qp_b != -1)
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_QP_B, ctx->qp_b);
> > +
> > + // Bitrate
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_TARGET_BITRATE, avctx->bit_rate);
> > + if (ctx->rate_control_mode ==
> AMF_VIDEO_ENCODER_RATE_CONTROL_METHOD_CBR) {
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_PEAK_BITRATE, avctx->bit_rate);
> > + } else {
> > + int rc_max_rate = avctx->rc_max_rate >= avctx->bit_rate ? avctx-
> >rc_max_rate : avctx->bit_rate * 13 / 10;
>
> Where does 13/10 come from?
>
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_PEAK_BITRATE, rc_max_rate);
> > + }
> > + // Enforce HRD, Filler Data, VBAQ, Frame Skipping, Deblocking Filter
> > + AMF_ASSIGN_PROPERTY_BOOL(res, ctx->encoder,
> AMF_VIDEO_ENCODER_ENFORCE_HRD, !!ctx->enforce_hrd);
> > + AMF_ASSIGN_PROPERTY_BOOL(res, ctx->encoder,
> AMF_VIDEO_ENCODER_FILLER_DATA_ENABLE, !!ctx->filler_data);
> > + AMF_ASSIGN_PROPERTY_BOOL(res, ctx->encoder,
> AMF_VIDEO_ENCODER_RATE_CONTROL_SKIP_FRAME_ENABLE, !!ctx-
> >skip_frame);
> > + if (ctx->rate_control_mode ==
> AMF_VIDEO_ENCODER_RATE_CONTROL_METHOD_CONSTANT_QP) {
> > + AMF_ASSIGN_PROPERTY_BOOL(res, ctx->encoder,
> AMF_VIDEO_ENCODER_ENABLE_VBAQ, false);
> > + if (ctx->enable_vbaq)
> > + av_log(ctx, AV_LOG_WARNING, "VBAQ is not supported by cqp
> Rate Control Method, automatically disabled.");
> > + } else {
> > + AMF_ASSIGN_PROPERTY_BOOL(res, ctx->encoder,
> AMF_VIDEO_ENCODER_ENABLE_VBAQ, !!ctx->enable_vbaq);
> > + }
> > + AMF_ASSIGN_PROPERTY_BOOL(res, ctx->encoder,
> AMF_VIDEO_ENCODER_DE_BLOCKING_FILTER, !!deblocking_filter);
> > +
> > + // B-Frames
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_B_PIC_PATTERN, avctx->max_b_frames);
> > + if (avctx->max_b_frames && res == AMF_OK) {
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_B_PIC_DELTA_QP, ctx->b_frame_delta_qp);
> > + AMF_ASSIGN_PROPERTY_BOOL(res, ctx->encoder,
> AMF_VIDEO_ENCODER_B_REFERENCE_ENABLE, !!ctx->b_frame_ref);
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_REF_B_PIC_DELTA_QP, ctx->ref_b_frame_delta_qp);
> > + }
> > +
> > + // Keyframe Interval
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_IDR_PERIOD, avctx->gop_size);
> > +
> > + // Header Insertion Spacing
> > + if (ctx->header_spacing >= 0)
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEADER_INSERTION_SPACING, ctx-
> >header_spacing);
> > +
> > + // Intra-Refresh, Slicing
> > + if (ctx->intra_refresh_mb > 0)
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_INTRA_REFRESH_NUM_MBS_PER_SLOT, ctx-
> >intra_refresh_mb);
> > + if (ctx->slices > 1)
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_SLICES_PER_FRAME, ctx->slices);> +
> > + // Coding
> > + if (ctx->coding_mode != 0)
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_CABAC_ENABLE, ctx->coding_mode);
> > +
> > + // Motion Estimation
> > + AMF_ASSIGN_PROPERTY_BOOL(res, ctx->encoder,
> AMF_VIDEO_ENCODER_MOTION_HALF_PIXEL, !!ctx->me_half_pel);
> > + AMF_ASSIGN_PROPERTY_BOOL(res, ctx->encoder,
> AMF_VIDEO_ENCODER_MOTION_QUARTERPIXEL, !!ctx->me_quater_pel);
> > +
> > + // fill extradata
> > + AMFVariantInit(&var);
>
> Can this fail?
>
> > + res = ctx->encoder->pVtbl->GetProperty(ctx->encoder,
> AMF_VIDEO_ENCODER_EXTRADATA, &var);
> > + if (res == AMF_OK && var.pInterface) {
> > + AMFBuffer* buffer;
> > + AMFGuid guid = IID_AMFBuffer();
> > +
> > + var.pInterface->pVtbl->QueryInterface(var.pInterface, &guid,
> (void**)&buffer); // query for buffer interface
> > +
> > + avctx->extradata_size = (int)buffer->pVtbl->GetSize(buffer);
> > + avctx->extradata = av_mallocz(avctx->extradata_size +
> AV_INPUT_BUFFER_PADDING_SIZE);
> > + if (!avctx->extradata) {
> > + buffer->pVtbl->Release(buffer);
> > + var.pInterface->pVtbl->Release(var.pInterface);
> > + return AVERROR(ENOMEM);
> > + }
> > + memcpy(avctx->extradata, buffer->pVtbl->GetNative(buffer), avctx-
> >extradata_size);
> > +
> > + buffer->pVtbl->Release(buffer);
> > + var.pInterface->pVtbl->Release(var.pInterface);
> > + }
> > + return 0;
> > +}
> > +
> > +
> > +
> > +static const AVCodecDefault defaults[] = {
> > + { "refs", "-1" },
> > + { "aspect", "0" },
> > + { "sar", "0" },
> > + { "qmin", "-1" },
> > + { "qmax", "-1" },
> > + { "b", "2M" },
> > + { "maxrate", "3M" },
> > + { "g", "250" },
> > + { "keyint_min", "0" },
> > + { "bf", "0" },
> > + { "slices", "1" },
> > + { NULL },
> > +};
> > +
> > +static const AVClass h264_amf_class = {
> > + .class_name = "h264_amf",
> > + .item_name = av_default_item_name,
> > + .option = options,
> > + .version = LIBAVUTIL_VERSION_INT,
> > +};
> > +static const AVClass h264_amf_d3d11va_class = {
> > + .class_name = "h264_amf_d3d11va",
> > + .item_name = av_default_item_name,
> > + .option = options,
> > + .version = LIBAVUTIL_VERSION_INT,
> > +};
> > +// regular encoder
> > +AVCodec ff_h264_amf_encoder = {
> > + .name = "h264_amf",
> > + .long_name = NULL_IF_CONFIG_SMALL("AMD AMF H.264 Encoder"),
> > + .type = AVMEDIA_TYPE_VIDEO,
> > + .id = AV_CODEC_ID_H264,
> > + .init = amf_encode_init_h264,
> > + .encode2 = ff_amf_encode_frame,
> > + .close = ff_amf_encode_close,
> > + .priv_data_size = sizeof(AmfContext),
> > + .priv_class = &h264_amf_class,
> > + .defaults = defaults,
> > + .capabilities = AV_CODEC_CAP_DELAY,
> > + .caps_internal = FF_CODEC_CAP_INIT_CLEANUP,
> > + .pix_fmts = ff_amf_pix_fmts,
> > +};
> > +// encoder connected with D3D11 HW accelerator
> > +AVCodec ff_h264_amf_d3d11va_encoder = {
> > + .name = "h264_amf_d3d11va",
> > + .long_name = NULL_IF_CONFIG_SMALL("AMD AMF H.264 Encoder with
> d3d11va"),
> > + .type = AVMEDIA_TYPE_VIDEO,
> > + .id = AV_CODEC_ID_H264,
> > + .init = amf_encode_init_h264,
> > + .encode2 = ff_amf_encode_frame,
> > + .close = ff_amf_encode_close,
> > + .priv_data_size = sizeof(AmfContext),
> > + .priv_class = &h264_amf_d3d11va_class,
> > + .defaults = defaults,
> > + .capabilities = AV_CODEC_CAP_DELAY,
> > + .caps_internal = FF_CODEC_CAP_INIT_CLEANUP,
> > + .pix_fmts = ff_amf_pix_fmts,
> > +};
>
> As above, why does this separate (identical) instance exist?
>
> > +
> > +/**
> > +* Basic test BAT file:
> > +echo off
> > +if "%~1"=="" (
> > +echo input file name is empty. Use basic_transcode_amf_h264.bat
> video.mp4
> > +goto error
> > +)
> > +
> > +SET "CWD=%~dp0"
> > +SET bitrate=5M
> > +SET maxbitrate=6M
> > +SET bufsize=2M
> > +SET x264_preset=veryfast
> > +SET amf_quality=speed
> > +
> > +
> > +rem veryfast and zerolatency options make x264 comparable with VCE
> > +
> > +rem change path to ffmpeg.exe if needed
> > +
> > +"%CWD%\..\bin\ffmpeg.exe" -y -t 100 -threads 0 -i "%~1" -c:v h264_amf -
> b:v %bitrate% -maxrate %maxbitrate% -bufsize %bufsize% -rc vbr_peak -
> quality %amf_quality% out_amf_h264.mp4
> > +"%CWD%\..\bin\ffmpeg.exe" -y -t 100 -threads 0 -i "%~1" -c:v libx264 -
> b:v %bitrate% -maxrate %maxbitrate% -bufsize %bufsize% -preset
> %x264_preset% -nal-hrd vbr -tune zerolatency out_x264_h264.mp4
> > +
> > +echo PSNR > result.txt
> > +
> > +"%CWD%\..\bin\ffmpeg.exe" -y -hide_banner -loglevel info -t 100 -
> threads 0 -i "%~1" -i out_amf_h264.mp4 -lavfi
> psnr="stats_file='amf_h264.psnr.log'" -f null - > "trace.txt" 2>&1
> > +for /f "tokens=*" %%A in ('type "trace.txt"') do @echo amf_h264 : %%A
> >end.txt
> > +type end.txt >> result.txt
> > +
> > +
> > +"%CWD%\..\bin\ffmpeg.exe" -y -hide_banner -loglevel info -t 100 -
> threads 0 -i "%~1" -i out_x264_h264.mp4 -lavfi
> psnr="stats_file='x264_h264.psnr.log'" -f null - > "trace.txt" 2>&1
> > +for /f "tokens=*" %%A in ('type "trace.txt"') do @echo x264_h264: %%A
> >end.txt
> > +type end.txt >> result.txt
> > +
> > +
> > +echo SSIM >> result.txt
> > +
> > +"%CWD%\..\bin\ffmpeg.exe" -y -hide_banner -loglevel info -t 100 -
> threads 0 -i "%~1" -i out_amf_h264.mp4 -lavfi
> ssim="stats_file='amf_h264_ssim.log'" -f null - > "trace.txt" 2>&1
> > +for /f "tokens=*" %%A in ('type "trace.txt"') do @echo amf_h264 : %%A
> >end.txt
> > +type end.txt >> result.txt
> > +
> > +"%CWD%\..\bin\ffmpeg.exe" -y -hide_banner -loglevel info -t 100 -
> threads 0 -i "%~1" -i out_x264_h264.mp4 -lavfi
> ssim="stats_file=x264_h264_ssim.log'" -f null - > "trace.txt" 2>&1
> > +for /f "tokens=*" %%A in ('type "trace.txt"') do @echo x264_h264: %%A
> >end.txt
> > +type end.txt >> result.txt
> > +
> > +del trace.txt
> > +del end.txt
> > +
> > +echo
> > +type result.txt
> > +
> > +
> > +:error
> > +*/
> > +
> > +
> > +/**
> > +* d3d11va integration test bat file
> > +rem echo off
> > +if "%~1"=="" (
> > +echo input file name is empty. Use dx11_transcode_amf_h264.bat
> video.mp4
> > +goto error
> > +)
> > +
> > +SET "CWD=%~dp0"
> > +SET bitrate=5M
> > +SET maxbitrate=6M
> > +SET bufsize=2M
> > +
> > +rem change path to ffmpeg.exe if needed
> > +
> > +"%CWD%\..\bin\ffmpeg.exe" -y -t 100 -hwaccel d3d11va -
> hwaccel_output_format d3d11 -threads 1 -i "%~1" -c:v h264_amf -b:v
> %bitrate% -maxrate %maxbitrate% -bufsize %bufsize% -rc vbr_peak
> shared_dx11_amf_h264.mp4
> > +"%CWD%\..\bin\ffmpeg.exe" -y -t 100 -hwaccel d3d11va -
> threads 1 -i "%~1" -c:v h264_amf_d3d11va -b:v %bitrate% -maxrate
> %maxbitrate% -bufsize %bufsize% -rc vbr_peak
> custom_dx11_amf_h264.mp4
> > +
> > +*/
>
> Don't include any of this stuff.
>
> > +
> > diff --git a/libavcodec/amfenc_hevc.c b/libavcodec/amfenc_hevc.c
> > new file mode 100644
> > index 0000000..4d3c7d4
> > --- /dev/null
> > +++ b/libavcodec/amfenc_hevc.c
> > @@ -0,0 +1,354 @@
> > +/*
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA
> > + */
> > +
> > +#include "amfenc.h"
> > +//#include
> "compat/amd/amf/public/include/components/VideoEncoderHEVC.h"
> > +#include "libavutil/opt.h"
> > +#include "libavutil/internal.h"
> > +#include "internal.h"
> > +
> > +#define OFFSET(x) offsetof(AmfContext, x)
> > +#define VE AV_OPT_FLAG_VIDEO_PARAM |
> AV_OPT_FLAG_ENCODING_PARAM
> > +static const AVOption options[] = {
> > + { "usage", "Set the encoding usage", OFFSET(usage),
> AV_OPT_TYPE_INT, { .i64 =
> AMF_VIDEO_ENCODER_HEVC_USAGE_TRANSCONDING },
> AMF_VIDEO_ENCODER_HEVC_USAGE_TRANSCONDING,
> AMF_VIDEO_ENCODER_HEVC_USAGE_WEBCAM, VE, "usage" },
> > + { "transcoding", "", 0, AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_HEVC_USAGE_TRANSCONDING }, 0, 0, VE,
> "usage" },
> > + { "ultralowlatency","", 0, AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_HEVC_USAGE_ULTRA_LOW_LATENCY }, 0, 0, VE,
> "usage" },
> > + { "lowlatency", "", 0, AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_HEVC_USAGE_LOW_LATENCY }, 0, 0, VE,
> "usage" },
> > + { "webcam", "", 0, AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_HEVC_USAGE_WEBCAM }, 0, 0, VE, "usage" },
>
> Could some of this be in common with the H.264 encoder? (Maybe in the
> header?)
>
> > +
> > + { "profile", "Set the profile (default main)", OFFSET(profile),
> AV_OPT_TYPE_INT,{ .i64 = AMF_VIDEO_ENCODER_HEVC_PROFILE_MAIN },
> AMF_VIDEO_ENCODER_HEVC_PROFILE_MAIN,
> AMF_VIDEO_ENCODER_HEVC_PROFILE_MAIN, VE, "profile" },
> > + { "main", "", 0, AV_OPT_TYPE_CONST,{ .i64 =
> AMF_VIDEO_ENCODER_HEVC_PROFILE_MAIN }, 0, 0, VE, "profile" },
> > +
> > + { "profile_tier", "Set the profile tier (default main)", OFFSET(tier),
> AV_OPT_TYPE_INT,{ .i64 = AMF_VIDEO_ENCODER_HEVC_TIER_MAIN },
> AMF_VIDEO_ENCODER_HEVC_TIER_MAIN,
> AMF_VIDEO_ENCODER_HEVC_TIER_HIGH, VE, "tier" },
> > + { "main", "", 0, AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_HEVC_TIER_MAIN }, 0, 0, VE, "tier" },
> > + { "high", "", 0, AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_HEVC_TIER_HIGH }, 0, 0, VE, "tier" },
> > +
> > + { "level", "Set the encoding level (default auto)", OFFSET(level),
> AV_OPT_TYPE_INT,{ .i64 = 0 }, 0, AMF_LEVEL_6_2, VE, "level" },
> > + { "auto", "", 0, AV_OPT_TYPE_CONST, { .i64 = 0 }, 0, 0, VE,
> "level" },
> > + { "1.0", "", 0, AV_OPT_TYPE_CONST, { .i64 = AMF_LEVEL_1 }, 0, 0,
> VE, "level" },
> > + { "2.0", "", 0, AV_OPT_TYPE_CONST, { .i64 = AMF_LEVEL_2 }, 0, 0,
> VE, "level" },
> > + { "2.1", "", 0, AV_OPT_TYPE_CONST, { .i64 = AMF_LEVEL_2_1 }, 0,
> 0, VE, "level" },
> > + { "3.0", "", 0, AV_OPT_TYPE_CONST, { .i64 = AMF_LEVEL_3 }, 0, 0,
> VE, "level" },
> > + { "3.1", "", 0, AV_OPT_TYPE_CONST, { .i64 = AMF_LEVEL_3_1 }, 0,
> 0, VE, "level" },
> > + { "4.0", "", 0, AV_OPT_TYPE_CONST, { .i64 = AMF_LEVEL_4 }, 0, 0,
> VE, "level" },
> > + { "4.1", "", 0, AV_OPT_TYPE_CONST, { .i64 = AMF_LEVEL_4_1 }, 0,
> 0, VE, "level" },
> > + { "5.0", "", 0, AV_OPT_TYPE_CONST, { .i64 = AMF_LEVEL_5 }, 0, 0,
> VE, "level" },
> > + { "5.1", "", 0, AV_OPT_TYPE_CONST, { .i64 = AMF_LEVEL_5_1 }, 0,
> 0, VE, "level" },
> > + { "5.2", "", 0, AV_OPT_TYPE_CONST, { .i64 = AMF_LEVEL_5_2 }, 0,
> 0, VE, "level" },
> > + { "6.0", "", 0, AV_OPT_TYPE_CONST, { .i64 = AMF_LEVEL_6 }, 0, 0,
> VE, "level" },
> > + { "6.1", "", 0, AV_OPT_TYPE_CONST, { .i64 = AMF_LEVEL_6_1 }, 0,
> 0, VE, "level" },
> > + { "6.2", "", 0, AV_OPT_TYPE_CONST, { .i64 = AMF_LEVEL_6_2 }, 0,
> 0, VE, "level" },
> > +
> > + { "quality", "Set the encoding quality", OFFSET(quality),
> AV_OPT_TYPE_INT, { .i64 =
> AMF_VIDEO_ENCODER_HEVC_QUALITY_PRESET_SPEED },
> AMF_VIDEO_ENCODER_HEVC_QUALITY_PRESET_QUALITY,
> AMF_VIDEO_ENCODER_HEVC_QUALITY_PRESET_SPEED, VE, "quality" },
> > + { "balanced", "", 0, AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_HEVC_QUALITY_PRESET_BALANCED }, 0, 0, VE,
> "quality" },
> > + { "speed", "", 0, AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_HEVC_QUALITY_PRESET_SPEED }, 0, 0, VE, "quality"
> },
> > + { "quality", "", 0, AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_HEVC_QUALITY_PRESET_QUALITY }, 0, 0, VE,
> "quality" },
> > +
> > + { "rc", "Set the rate control mode",
> OFFSET(rate_control_mode), AV_OPT_TYPE_INT, { .i64 =
> AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_METHOD_PEAK_CONSTRAIN
> ED_VBR },
> AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_METHOD_CONSTANT_QP,
> AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_METHOD_CBR, VE, "rc" },
> > + { "cqp", "", 0, AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_METHOD_CONSTANT_QP
> }, 0, 0, VE, "rc" },
> > + { "cbr", "", 0, AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_METHOD_CBR },
> 0, 0, VE, "rc" },
> > + { "vbr_peak", "", 0, AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_METHOD_PEAK_CONSTRAIN
> ED_VBR }, 0, 0, VE, "rc" },
> > + { "vbr_latency", "", 0, AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_METHOD_LATENCY_CONSTR
> AINED_VBR }, 0, 0, VE, "rc" },
> > +
>
> Trailing spaces.
>
> > +
> > + { "header_insertion_mode", "Set header insertion mode",
> OFFSET(header_insertion_mode), AV_OPT_TYPE_INT,{ .i64 =
> AMF_VIDEO_ENCODER_HEVC_HEADER_INSERTION_MODE_NONE },
> AMF_VIDEO_ENCODER_HEVC_HEADER_INSERTION_MODE_NONE,
> AMF_VIDEO_ENCODER_HEVC_HEADER_INSERTION_MODE_IDR_ALIGNED,
> VE, "hdrmode" },
> > + { "balanced", "", 0, AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_HEVC_HEADER_INSERTION_MODE_NONE }, 0, 0,
> VE, "hdrmode" },
> > + { "speed", "", 0, AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_HEVC_HEADER_INSERTION_MODE_GOP_ALIGNED },
> 0, 0, VE, "hdrmode" },
> > + { "quality", "", 0, AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_HEVC_HEADER_INSERTION_MODE_IDR_ALIGNED },
> 0, 0, VE, "hdrmode" },
>
> Names look suspicious...
>
> > +
> > + { "gops_per_idr", "GOPs per IDR 0-no IDR will be inserted",
> OFFSET(gops_per_idr), AV_OPT_TYPE_INT,{ .i64 = 60 }, 0, INT_MAX, VE },
> > + { "preanalysis", "Enable preanalysis", OFFSET(preanalysis),
> AV_OPT_TYPE_BOOL,{ .i64 = 0 }, 0, 1, VE, NULL },
> > + { "vbaq", "Enable VBAQ", OFFSET(enable_vbaq),
> AV_OPT_TYPE_BOOL,{ .i64 = 0 }, 0, 1, VE, NULL },
> > + { "enforce_hrd", "Enforce HRD", OFFSET(enforce_hrd),
> AV_OPT_TYPE_BOOL,{ .i64 = 0 }, 0, 1, VE, NULL },
> > + { "filler_data", "Filler Data Enable", OFFSET(filler_data),
> AV_OPT_TYPE_BOOL,{ .i64 = 0 }, 0, 1, VE, NULL },
> > + { "max_au_size", "Max AU Size in bits",
> OFFSET(max_au_size), AV_OPT_TYPE_INT,{ .i64 = 0 }, 0, INT_MAX, VE, NULL
> },
> > + { "min_qp_i", "min quantization parameter for I-frame",
> OFFSET(min_qp_i), AV_OPT_TYPE_INT,{ .i64 = -1 }, -1, 51, VE },
> > + { "max_qp_i", "max quantization parameter for I-frame",
> OFFSET(max_qp_i), AV_OPT_TYPE_INT,{ .i64 = -1 }, -1, 51, VE },
> > + { "min_qp_p", "min quantization parameter for P-frame",
> OFFSET(min_qp_p), AV_OPT_TYPE_INT,{ .i64 = -1 }, -1, 51, VE },
> > + { "max_qp_p", "max quantization parameter for P-frame",
> OFFSET(max_qp_p), AV_OPT_TYPE_INT,{ .i64 = -1 }, -1, 51, VE },
> > + { "qp_p", "quantization parameter for P-frame", OFFSET(qp_p),
> AV_OPT_TYPE_INT,{ .i64 = -1 }, -1, 51, VE },
> > + { "qp_i", "quantization parameter for I-frame", OFFSET(qp_i),
> AV_OPT_TYPE_INT,{ .i64 = -1 }, -1, 51, VE },
> > + { "skip_frame", "Rate Control Based Frame Skip",
> OFFSET(skip_frame), AV_OPT_TYPE_BOOL,{ .i64 = 0 }, 0, 1, VE, NULL },
> > + { "me_half_pel", "Enable ME Half Pixel",
> OFFSET(me_half_pel), AV_OPT_TYPE_BOOL,{ .i64 = 1 }, 0, 1, VE, NULL },
> > + { "me_quater_pel", "Enable ME Quarter Pixel ",
> OFFSET(me_quater_pel), AV_OPT_TYPE_BOOL,{ .i64 = 1 }, 0, 1, VE, NULL },
> > +
> > + { NULL }
> > +};
> > +
> > +static av_cold int amf_encode_init_hevc(AVCodecContext *avctx)
> > +{
> > + int ret = 0;
> > + AMF_RESULT res = AMF_OK;
> > + AmfContext *ctx = avctx->priv_data;
> > + AMFVariantStruct var = {0};
> > +
> > + AMFSize framesize = AMFConstructSize(avctx->width, avctx-
> >height);
> > + AMFRate framerate = AMFConstructRate(avctx->time_base.den,
> avctx->time_base.num * avctx->ticks_per_frame);
> > +
> > + int deblocking_filter = (avctx->flags &
> AV_CODEC_FLAG_LOOP_FILTER) ? 1 : 0;
> > +
> > + if ((ret = ff_amf_encode_init(avctx)) < 0)
> > + return ret;
> > +
>
> Trailing spaces.
>
> > + // init static parameters
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_USAGE, ctx->usage);
> > +
> > + AMF_ASSIGN_PROPERTY_SIZE(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_FRAMESIZE, framesize);
> > +
> > + AMF_ASSIGN_PROPERTY_RATE(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_FRAMERATE, framerate);
> > +
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_PROFILE, ctx->profile);
> > +
> > + switch (ctx->profile) {
> > + case AMF_VIDEO_ENCODER_HEVC_TIER_HIGH:
> > + avctx->profile = FF_PROFILE_HEVC_REXT;
> > + break;
> > + case AMF_VIDEO_ENCODER_HEVC_TIER_MAIN:
> > + avctx->profile = FF_PROFILE_HEVC_MAIN;
> > + break;
>
> It's set by the user - you should be using it, not writing it.
>
> > + default:
> > + break;
> > + }
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_TIER, ctx->tier);
> > +
> > + if (ctx->level != 0) {
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_PROFILE_LEVEL, ctx->level);
> > + avctx->level = ctx->level;
>
> As profile, you should be using it, not writing it.
>
> > + }
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_QUALITY_PRESET, ctx->quality);
> > + // Maximum Reference Frames
> > + if (avctx->refs != 0) {
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_MAX_NUM_REFRAMES, avctx->refs);
> > + }
> > + // Aspect Ratio
> > + if (avctx->sample_aspect_ratio.den && avctx-
> >sample_aspect_ratio.num) {
> > + AMFRatio ratio = AMFConstructRatio(avctx-
> >sample_aspect_ratio.num, avctx->sample_aspect_ratio.den);
> > + AMF_ASSIGN_PROPERTY_RATIO(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_ASPECT_RATIO, ratio);
> > + }
> > +
> > + // Picture control properties
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_NUM_GOPS_PER_IDR, ctx->gops_per_idr);
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_GOP_SIZE, avctx->gop_size);
> > + if (avctx->slices > 1) {
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_SLICES_PER_FRAME, avctx->slices);
> > + }
> > + AMF_ASSIGN_PROPERTY_BOOL(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_DE_BLOCKING_FILTER_DISABLE,
> deblocking_filter);
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_HEADER_INSERTION_MODE, ctx-
> >header_insertion_mode);
> > +
> > + // Rate control
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_METHOD, ctx-
> >rate_control_mode);
> > + if (avctx->rc_buffer_size)
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_VBV_BUFFER_SIZE, avctx->rc_buffer_size);
> > +
> > + if (avctx->rc_initial_buffer_occupancy != 0) {
> > + int percent = avctx->rc_buffer_size * 64 / avctx-
> >rc_initial_buffer_occupancy;
> > + if (percent > 64)
> > + percent = 64;
>
> As H.264; what is this trying to do?
>
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_INITIAL_VBV_BUFFER_FULLNESS, percent);
> > + }
> > + // Pre-Pass, Pre-Analysis, Two-Pass
> > + AMF_ASSIGN_PROPERTY_BOOL(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_PREANALYSIS_ENABLE, ctx-
> >preanalysis);
> > +
> > + if (ctx->rate_control_mode ==
> AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_METHOD_CONSTANT_QP) {
> > + AMF_ASSIGN_PROPERTY_BOOL(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_ENABLE_VBAQ, false);
> > + if (ctx->enable_vbaq)
> > + av_log(ctx, AV_LOG_WARNING, "VBAQ is not supported by cqp
> Rate Control Method, automatically disabled.");
> > + } else {
> > + AMF_ASSIGN_PROPERTY_BOOL(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_ENABLE_VBAQ, !!ctx->enable_vbaq);
> > + }
> > + AMF_ASSIGN_PROPERTY_BOOL(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_MOTION_HALF_PIXEL, ctx->me_half_pel);
> > + AMF_ASSIGN_PROPERTY_BOOL(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_MOTION_QUARTERPIXEL, ctx-
> >me_quater_pel);
> > +
> > + // init encoder
> > + res = ctx->encoder->pVtbl->Init(ctx->encoder, ctx->format, avctx-
> >width, avctx->height);
> > + AMF_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_BUG, "encoder-
> >Init() failed with error %d", res);
> > +
> > + // init dynamic rate control params
> > + AMF_ASSIGN_PROPERTY_BOOL(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_ENFORCE_HRD, ctx->enforce_hrd);
> > + AMF_ASSIGN_PROPERTY_BOOL(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_FILLER_DATA_ENABLE, ctx->filler_data);
> > +
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_TARGET_BITRATE, avctx->bit_rate);
> > + if (ctx->rate_control_mode ==
> AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_METHOD_CBR) {
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_PEAK_BITRATE, avctx->bit_rate);
> > + } else {
> > + int rc_max_rate = avctx->rc_max_rate >= avctx->bit_rate ? avctx-
> >rc_max_rate : avctx->bit_rate * 13 / 10;
>
> Why 13/10?
>
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_PEAK_BITRATE, rc_max_rate);
> > + }
> > +
> > + // init dynamic picture control params
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_MAX_AU_SIZE, ctx->max_au_size);
> > +
> > +
> > + if (ctx->min_qp_i != -1) {
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_MIN_QP_I, ctx->min_qp_i);
> > + }
> > + if (ctx->max_qp_i != -1) {
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_MAX_QP_I, ctx->max_qp_i);
> > + }
> > + if (ctx->min_qp_p != -1) {
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_MIN_QP_P, ctx->min_qp_p);
> > + }
> > + if (ctx->max_qp_p != -1) {
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_MAX_QP_P, ctx->max_qp_p);
> > + }
> > +
> > + if (ctx->qp_p != -1) {
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_QP_I, ctx->qp_p);
> > + }
> > + if (ctx->qp_i != -1) {
> > + AMF_ASSIGN_PROPERTY_INT64(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_QP_P, ctx->qp_i);
> > + }
> > + AMF_ASSIGN_PROPERTY_BOOL(res, ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_SKIP_FRAME_ENABLE, ctx-
> >skip_frame);
> > +
> > +
> > + // fill extradata
> > + AMFVariantInit(&var);
>
> Check return value?
>
> > + res = ctx->encoder->pVtbl->GetProperty(ctx->encoder,
> AMF_VIDEO_ENCODER_HEVC_EXTRADATA, &var);
> > + if(res == AMF_OK && var.pInterface){
>
> Formatting.
>
> > + AMFBuffer* buffer;
> > + AMFGuid guid = IID_AMFBuffer();
> > +
> > + var.pInterface->pVtbl->QueryInterface(var.pInterface, &guid,
> (void**)&buffer); // query for buffer interface
> > +
> > + avctx->extradata_size = (int)buffer->pVtbl->GetSize(buffer);
> > + avctx->extradata = av_mallocz(avctx->extradata_size +
> AV_INPUT_BUFFER_PADDING_SIZE);
> > + if (!avctx->extradata) {
> > + buffer->pVtbl->Release(buffer);
> > + var.pInterface->pVtbl->Release(var.pInterface);
> > + return AVERROR(ENOMEM);
> > + }
> > + memcpy(avctx->extradata, buffer->pVtbl->GetNative(buffer), avctx-
> >extradata_size);
> > +
> > + buffer->pVtbl->Release(buffer);
> > + var.pInterface->pVtbl->Release(var.pInterface);
> > + }
>
> Should that fail if res is not OK?
>
> > + return 0;
> > +}
> > +static const AVCodecDefault defaults[] = {
> > + { "b", "2M" },
> > + { "maxrate", "3M" },
> > + { "qmin", "-1" },
> > + { "qmax", "-1" },
> > + { "qdiff", "-1" },
> > + { "qblur", "-1" },
> > + { "qcomp", "-1" },
>
> Why set these? You don't appear to be using them.
>
> > + { "g", "250" },
> > + { "bf", "0" },
> > + { NULL },
> > +};
> > +static const AVClass hevc_amf_class = {
> > + .class_name = "hevc_amf",
> > + .item_name = av_default_item_name,
> > + .option = options,
> > + .version = LIBAVUTIL_VERSION_INT,
> > +};
> > +static const AVClass hevc_amf_amf_d3d11va_class = {
> > + .class_name = "hevc_amf_amf_d3d11va",
> > + .item_name = av_default_item_name,
> > + .option = options,
> > + .version = LIBAVUTIL_VERSION_INT,
> > +};
> > +// regular encoder
> > +AVCodec ff_hevc_amf_encoder = {
> > + .name = "hevc_amf",
> > + .long_name = NULL_IF_CONFIG_SMALL("AMD AMF HEVC encoder"),
> > + .type = AVMEDIA_TYPE_VIDEO,
> > + .id = AV_CODEC_ID_HEVC,
> > + .init = amf_encode_init_hevc,
> > + .encode2 = ff_amf_encode_frame,
> > + .close = ff_amf_encode_close,
> > + .priv_data_size = sizeof(AmfContext),
> > + .priv_class = &hevc_amf_class,
> > + .defaults = defaults,
> > + .capabilities = AV_CODEC_CAP_DELAY,
> > + .caps_internal = FF_CODEC_CAP_INIT_CLEANUP,
> > + .pix_fmts = ff_amf_pix_fmts,
> > +};
> > +// encoder connected with D3D11 HW accelerator
> > +AVCodec ff_hevc_amf_d3d11va_encoder = {
> > +.name = "hevc_amf_d3d11va",
> > +.long_name = NULL_IF_CONFIG_SMALL("AMD AMF HEVC encoder with
> d3d11va"),
> > +.type = AVMEDIA_TYPE_VIDEO,
> > +.id = AV_CODEC_ID_HEVC,
> > +.init = amf_encode_init_hevc,
> > +.encode2 = ff_amf_encode_frame,
> > +.close = ff_amf_encode_close,
> > +.priv_data_size = sizeof(AmfContext),
> > +.priv_class = &hevc_amf_amf_d3d11va_class,
> > +.defaults = defaults,
> > +.capabilities = AV_CODEC_CAP_DELAY,
> > +.caps_internal = FF_CODEC_CAP_INIT_CLEANUP,
> > +.pix_fmts = ff_amf_pix_fmts,
> > +};
> > +
> > +/**
> > +* Basic test BAT file:
> > +echo off
> > +if "%~1"=="" (
> > +echo input file name is empty. Use basic_transcode_amf_hevc.bat
> video.mp4
> > +goto error
> > +)
> > +
> > +SET "CWD=%~dp0"
> > +SET bitrate=5M
> > +SET maxbitrate=6M
> > +SET bufsize=2M
> > +SET x265_preset=veryfast
> > +SET amf_quality=speed
> > +
> > +
> > +rem veryfast and zerolatency options make x265 comparable with VCE
> > +
> > +rem change path to ffmpeg.exe if needed
> > +
> > +"%CWD%\..\bin\ffmpeg.exe" -y -t 100 -threads 0 -i "%~1" -c:v hevc_amf -
> b:v %bitrate% -maxrate %maxbitrate% -bufsize %bufsize% -rc vbr_peak -
> quality %amf_quality% out_amf_hevc.mp4
> > +"%CWD%\..\bin\ffmpeg.exe" -y -t 100 -threads 0 -i "%~1" -c:v libx265 -
> b:v %bitrate% -maxrate %maxbitrate% -bufsize %bufsize% -preset
> %x265_preset% -x265-params vbv-maxrate=6000:vbv-bufsize=2000 -tune
> zerolatency out_x265_hevc.mp4
> > +
> > +echo PSNR > result.txt
> > +
> > +"%CWD%\..\bin\ffmpeg.exe" -y -hide_banner -loglevel info -t 100 -
> threads 0 -i "%~1" -i out_amf_hevc.mp4 -lavfi
> psnr="stats_file='amf_hevc.psnr.log'" -f null - > "trace.txt" 2>&1
> > +for /f "tokens=*" %%A in ('type "trace.txt"') do @echo amf_hevc : %%A
> >end.txt
> > +type end.txt >> result.txt
> > +
> > +
> > +"%CWD%\..\bin\ffmpeg.exe" -y -hide_banner -loglevel info -t 100 -
> threads 0 -i "%~1" -i out_x265_hevc.mp4 -lavfi
> psnr="stats_file='x265_hevc.psnr.log'" -f null - > "trace.txt" 2>&1
> > +for /f "tokens=*" %%A in ('type "trace.txt"') do @echo x265_hevc: %%A
> >end.txt
> > +type end.txt >> result.txt
> > +
> > +
> > +echo SSIM >> result.txt
> > +
> > +"%CWD%\..\bin\ffmpeg.exe" -y -hide_banner -loglevel info -t 100 -
> threads 0 -i "%~1" -i out_amf_hevc.mp4 -lavfi
> ssim="stats_file='amf_hevc_ssim.log'" -f null - > "trace.txt" 2>&1
> > +for /f "tokens=*" %%A in ('type "trace.txt"') do @echo amf_hevc : %%A
> >end.txt
> > +type end.txt >> result.txt
> > +
> > +"%CWD%\..\bin\ffmpeg.exe" -y -hide_banner -loglevel info -t 100 -
> threads 0 -i "%~1" -i out_x265_hevc.mp4 -lavfi
> ssim="stats_file=x265_hevc_ssim.log'" -f null - > "trace.txt" 2>&1
> > +for /f "tokens=*" %%A in ('type "trace.txt"') do @echo x265_hevc: %%A
> >end.txt
> > +type end.txt >> result.txt
> > +
> > +del trace.txt
> > +del end.txt
> > +
> > +echo
> > +type result.txt
> > +
> > +
> > +:error
> > +
> > +
> > +*/
>
> Don't include this stuff.
>
> > \ No newline at end of file
>
> git has another a review comment for you.
>
> > diff --git a/libavcodec/version.h b/libavcodec/version.h
> > index 226da19..6c0d7a8 100644
> > --- a/libavcodec/version.h
> > +++ b/libavcodec/version.h
> > @@ -28,8 +28,8 @@
> > #include "libavutil/version.h"
> >
> > #define LIBAVCODEC_VERSION_MAJOR 58
> > -#define LIBAVCODEC_VERSION_MINOR 0
> > -#define LIBAVCODEC_VERSION_MICRO 101
> > +#define LIBAVCODEC_VERSION_MINOR 1
> > +#define LIBAVCODEC_VERSION_MICRO 100
> >
> > #define LIBAVCODEC_VERSION_INT
> AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
> > LIBAVCODEC_VERSION_MINOR, \
> >
>
> Thanks,
>
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
More information about the ffmpeg-devel
mailing list