[FFmpeg-devel] [PATCH v3 1/2] avcodec: add external enc libvvenc for H266/VVC
Andreas Rheinhardt
andreas.rheinhardt at outlook.com
Tue May 14 19:49:13 EEST 2024
Christian Bartnik:
> From: Thomas Siedel <thomas.ff at spin-digital.com>
>
> Add external encoder VVenC for H266/VVC encoding.
> Register new encoder libvvenc.
> Add libvvenc to wrap the vvenc interface.
> libvvenc implements encoder option: preset,qp,period,subjopt,
> vvenc-params,levelidc,tier.
> Enable encoder by adding --enable-libvvenc in configure step.
>
> Co-authored-by: Christian Bartnik chris10317h5 at gmail.com
> Signed-off-by: Christian Bartnik <chris10317h5 at gmail.com>
> ---
> configure | 4 +
> doc/encoders.texi | 65 +++++
> libavcodec/Makefile | 1 +
> libavcodec/allcodecs.c | 1 +
> libavcodec/libvvenc.c | 566 +++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 637 insertions(+)
> create mode 100644 libavcodec/libvvenc.c
>
> diff --git a/configure b/configure
> index a909b0689c..5d9a14821b 100755
> --- a/configure
> +++ b/configure
> @@ -293,6 +293,7 @@ External library support:
> --enable-libvorbis enable Vorbis en/decoding via libvorbis,
> native implementation exists [no]
> --enable-libvpx enable VP8 and VP9 de/encoding via libvpx [no]
> + --enable-libvvenc enable H.266/VVC encoding via vvenc [no]
> --enable-libwebp enable WebP encoding via libwebp [no]
> --enable-libx264 enable H.264 encoding via x264 [no]
> --enable-libx265 enable HEVC encoding via x265 [no]
> @@ -1966,6 +1967,7 @@ EXTERNAL_LIBRARY_LIST="
> libvmaf
> libvorbis
> libvpx
> + libvvenc
> libwebp
> libxevd
> libxeve
> @@ -3558,6 +3560,7 @@ libvpx_vp8_decoder_deps="libvpx"
> libvpx_vp8_encoder_deps="libvpx"
> libvpx_vp9_decoder_deps="libvpx"
> libvpx_vp9_encoder_deps="libvpx"
> +libvvenc_encoder_deps="libvvenc"
> libwebp_encoder_deps="libwebp"
> libwebp_anim_encoder_deps="libwebp"
> libx262_encoder_deps="libx262"
> @@ -7025,6 +7028,7 @@ enabled libvpx && {
> die "libvpx enabled but no supported decoders found"
> fi
> }
> +enabled libvvenc && require_pkg_config libvvenc "libvvenc >= 1.6.1" "vvenc/vvenc.h" vvenc_get_version
>
> enabled libwebp && {
> enabled libwebp_encoder && require_pkg_config libwebp "libwebp >= 0.2.0" webp/encode.h WebPGetEncoderVersion
> diff --git a/doc/encoders.texi b/doc/encoders.texi
> index c82f316f94..92aab17c49 100644
> --- a/doc/encoders.texi
> +++ b/doc/encoders.texi
> @@ -2378,6 +2378,71 @@ Indicates frame duration
> For more information about libvpx see:
> @url{http://www.webmproject.org/}
>
> + at section libvvenc
> +
> +VVenC H.266/VVC encoder wrapper.
> +
> +This encoder requires the presence of the libvvenc headers and library
> +during configuration. You need to explicitly configure the build with
> + at option{--enable-libvvenc}.
> +
> +The VVenC project website is at
> + at url{https://github.com/fraunhoferhhi/vvenc}.
> +
> + at subsection Supported Pixel Formats
> +
> +VVenC supports only 10-bit color spaces as input. But the internal (encoded)
> +bit depth can be set to 8-bit or 10-bit at runtime.
> +
> + at subsection Options
> +
> + at table @option
> + at item b
> +Sets target video bitrate.
> +
> + at item g
> +Set the GOP size. Currently support for g=1 (Intra only) or default.
> +
> + at item preset
> +Set the VVenC preset.
> +
> + at item levelidc
> +Set level idc.
> +
> + at item tier
> +Set vvc tier.
> +
> + at item qp
> +Set constant quantization parameter.
> +
> + at item subopt @var{boolean}
> +Set subjective (perceptually motivated) optimization. Default is 1 (on).
> +
> + at item bitdepth8 @var{boolean}
> +Set 8bit coding mode instead of using 10bit. Default is 0 (off).
> +
> + at item period
> +set (intra) refresh period in seconds.
> +
> + at item vvenc-params
> +Set vvenc options using a list of @var{key}=@var{value} couples separated
> +by ":". See @command{vvencapp --fullhelp} or @command{vvencFFapp --fullhelp} for a list of options.
> +
> +For example, the options might be provided as:
> +
> + at example
> +intraperiod=64:decodingrefreshtype=idr:poc0idr=1:internalbitdepth=8
> + at end example
> +
> +For example the encoding options for 2-pass encoding might be provided with @option{-vvenc-params}:
> +
> + at example
> +ffmpeg -i input -c:v libvvenc -b 1M -vvenc-params passes=2:pass=1:rcstatsfile=stats.json output.mp4
> +ffmpeg -i input -c:v libvvenc -b 1M -vvenc-params passes=2:pass=2:rcstatsfile=stats.json output.mp4
> + at end example
> +
> + at end table
> +
> @section libwebp
>
> libwebp WebP Image encoder wrapper
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index 2443d2c6fd..5d7349090e 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -1153,6 +1153,7 @@ OBJS-$(CONFIG_LIBVPX_VP8_DECODER) += libvpxdec.o
> OBJS-$(CONFIG_LIBVPX_VP8_ENCODER) += libvpxenc.o
> OBJS-$(CONFIG_LIBVPX_VP9_DECODER) += libvpxdec.o
> OBJS-$(CONFIG_LIBVPX_VP9_ENCODER) += libvpxenc.o
> +OBJS-$(CONFIG_LIBVVENC_ENCODER) += libvvenc.o
> OBJS-$(CONFIG_LIBWEBP_ENCODER) += libwebpenc_common.o libwebpenc.o
> OBJS-$(CONFIG_LIBWEBP_ANIM_ENCODER) += libwebpenc_common.o libwebpenc_animencoder.o
> OBJS-$(CONFIG_LIBX262_ENCODER) += libx264.o
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index b102a8069e..59d36dbd56 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -800,6 +800,7 @@ extern const FFCodec ff_libvpx_vp8_encoder;
> extern const FFCodec ff_libvpx_vp8_decoder;
> extern FFCodec ff_libvpx_vp9_encoder;
> extern const FFCodec ff_libvpx_vp9_decoder;
> +extern const FFCodec ff_libvvenc_encoder;
> /* preferred over libwebp */
> extern const FFCodec ff_libwebp_anim_encoder;
> extern const FFCodec ff_libwebp_encoder;
> diff --git a/libavcodec/libvvenc.c b/libavcodec/libvvenc.c
> new file mode 100644
> index 0000000000..78d4f55a2a
> --- /dev/null
> +++ b/libavcodec/libvvenc.c
> @@ -0,0 +1,566 @@
> +/*
> + * H.266 encoding using the VVenC library
> + *
> + * Copyright (C) 2022, Thomas Siedel
> + *
> + * 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 "config_components.h"
Seems unneeded.
> +
> +#include <vvenc/vvenc.h>
> +#include <vvenc/vvencCfg.h>
> +#include <vvenc/version.h>
> +
> +#include "avcodec.h"
> +#include "codec_internal.h"
> +#include "encode.h"
> +#include "internal.h"
> +#include "packet_internal.h"
> +#include "profiles.h"
> +
> +#include "libavutil/avutil.h"
> +#include "libavutil/mem.h"
> +#include "libavutil/pixdesc.h"
> +#include "libavutil/opt.h"
> +#include "libavutil/common.h"
> +#include "libavutil/imgutils.h"
> +#include "libavutil/frame.h"
> +#include "libavutil/log.h"
> +
> +typedef struct VVenCOptions {
> + int preset; // preset 0: faster 4: slower
> + int qp; // quantization parameter 0-63
> + int subjectiveOptimization; // perceptually motivated QP adaptation, XPSNR based
> + int flag8bitCoding; // encode in 8bit instead of 10bit
> + int intraRefreshSec; // intra period/refresh in seconds
> + int levelIdc; // vvc level_idc
> + int tier; // vvc tier
> + AVDictionary *vvenc_opts;
> +} VVenCOptions;
> +
> +typedef struct VVenCContext {
> + AVClass *av_class;
> + VVenCOptions options; // encoder options
> + vvencEncoder *vvencEnc;
> + vvencAccessUnit *pAU;
> + bool encodeDone;
We do not use CamelCase for variable and struct member names.
> +} VVenCContext;
> +
> +
> +static av_cold void ff_vvenc_log_callback(void *ctx, int level,
> + const char *fmt, va_list args)
Remove the ff_ prefix from static functions (this is the prefix for
non-static functions).
> +{
> + vvenc_config params;
> + vvencEncoder *vvencEnc = (vvencEncoder *)ctx;
> + if (vvencEnc){
> + vvenc_config_default(¶ms);
> + vvenc_get_config(vvencEnc, ¶ms);
> + if ((int)params.m_verbosity >= level)
> + vfprintf(level == 1 ? stderr : stdout, fmt, args);
> + }
> +}
> +
> +static void ff_vvenc_set_verbository(vvenc_config* params )
What's the reason for the space before the closing parentheses?
Moreover, we prefer to put the * to the parameter, not the type.
> +{
> + params->m_verbosity = VVENC_VERBOSE;
> + if (av_log_get_level() >= AV_LOG_DEBUG)
av_log_get_level() can be changed at any time by someone else. So better
just call it once and cache the value for simplicity and consistency.
> + params->m_verbosity = VVENC_DETAILS;
> + else if (av_log_get_level() >= AV_LOG_VERBOSE)
> + params->m_verbosity = VVENC_NOTICE; // output per picture info
> + else if (av_log_get_level() >= AV_LOG_INFO)
> + params->m_verbosity = VVENC_WARNING; // ffmpeg default ffmpeg loglevel
> + else
> + params->m_verbosity = VVENC_SILENT;
> +}
> +
> +static int ff_vvenc_set_pic_format(AVCodecContext *avctx, vvenc_config* params )
> +{
> + VVenCContext *s =(VVenCContext *) avctx->priv_data;
This cast is unnecessary in C.
> +
> + params->m_internChromaFormat = VVENC_CHROMA_420;
> + params->m_inputBitDepth[0] = 10;
> +
> + if (avctx->pix_fmt != AV_PIX_FMT_YUV420P10LE){
> + av_log(avctx, AV_LOG_ERROR,
> + "unsupported pixel format %s, currently only support for yuv420p10le\n",
> + av_get_pix_fmt_name(avctx->pix_fmt));
> + return AVERROR(EINVAL);
1. This whole block of code is dead: For encoders that set
AVCodec.pix_fmts, it is checked generically that the pixel format set is
one of the pixel formats in AVCodec.pix_fmts.
2. Why only LE? Is this also true on BE systems?
> + }
> +
> + if (s->options.flag8bitCoding) {
> +#if VVENC_VERSION_MAJOR > 1 || (VVENC_VERSION_MAJOR == 1 && VVENC_VERSION_MINOR > 9) || (VVENC_VERSION_MAJOR == 1 && VVENC_VERSION_MINOR >= 9 && VVENC_VERSION_PATCH >= 1)
> + params->m_internalBitDepth[0] = 8;
> +#else
> + av_log(avctx, AV_LOG_ERROR,
> + "unsupported 8bit coding mode. 8bit coding needs at least vvenc version >= 1.9.1 "
> + "(current version %s)\n", vvenc_get_version() );
> + return AVERROR(EINVAL);
> +#endif
> + }
> + return 0;
> +}
> +
> +static void ff_vvenc_set_color_format(AVCodecContext *avctx, vvenc_config* params )
> +{
> + if (avctx->color_primaries != AVCOL_PRI_UNSPECIFIED)
> + params->m_colourPrimaries = (int) avctx->color_primaries;
> + if (avctx->colorspace != AVCOL_SPC_UNSPECIFIED)
> + params->m_matrixCoefficients = (int) avctx->colorspace;
> + if (avctx->color_trc != AVCOL_TRC_UNSPECIFIED) {
> + params->m_transferCharacteristics = (int) avctx->color_trc;
> +
> + if (avctx->color_trc == AVCOL_TRC_SMPTE2084)
> + params->m_HdrMode = (avctx->color_primaries == AVCOL_PRI_BT2020) ?
> + VVENC_HDR_PQ_BT2020 : VVENC_HDR_PQ;
> + else if (avctx->color_trc == AVCOL_TRC_BT2020_10
> + || avctx->color_trc == AVCOL_TRC_ARIB_STD_B67)
> + params->m_HdrMode = (avctx->color_trc == AVCOL_TRC_BT2020_10 ||
> + avctx->color_primaries == AVCOL_PRI_BT2020 ||
> + avctx->colorspace == AVCOL_SPC_BT2020_NCL ||
> + avctx->colorspace == AVCOL_SPC_BT2020_CL) ?
> + VVENC_HDR_HLG_BT2020 : VVENC_HDR_HLG;
> + }
> +
> + if (params->m_HdrMode == VVENC_HDR_OFF
> + && (avctx->color_primaries != AVCOL_PRI_UNSPECIFIED
> + || avctx->colorspace != AVCOL_SPC_UNSPECIFIED)) {
> + params->m_vuiParametersPresent = 1;
> + params->m_colourDescriptionPresent = true;
> + }
> +}
> +
> +static void ff_vvenc_set_framerate(AVCodecContext *avctx, vvenc_config* params )
> +{
> + params->m_FrameRate = avctx->time_base.den;
> + params->m_FrameScale = avctx->time_base.num;
> +
> +FF_DISABLE_DEPRECATION_WARNINGS
> +
> +#if FF_API_TICKS_PER_FRAME
> + if (avctx->ticks_per_frame == 1) {
> +#endif
> + params->m_TicksPerSecond = -1; // auto mode for ticks per frame = 1
> +#if FF_API_TICKS_PER_FRAME
> + } else {
> + params->m_TicksPerSecond =
> + ceil((avctx->time_base.den / (double) avctx->time_base.num) *
> + (double) avctx->ticks_per_frame);
> + }
> +#endif
> +FF_ENABLE_DEPRECATION_WARNINGS
> +}
> +
> +static int ff_vvenc_parse_vvenc_params(AVCodecContext *avctx, vvenc_config* params, char* statsfile )
> +{
> + int parse_ret, ret;
> + VVenCContext *s;
> + AVDictionaryEntry *en = NULL;
const
> + s =(VVenCContext *) avctx->priv_data;
Can be initialized directly and without the cast.
> + ret = 0;
> +
> + while ((en = av_dict_get(s->options.vvenc_opts, "", en,
> + AV_DICT_IGNORE_SUFFIX))) {
av_dict_iterate()
> + av_log(avctx, AV_LOG_DEBUG, "vvenc_set_param: '%s:%s'\n", en->key,
> + en->value);
> + parse_ret = vvenc_set_param(params, en->key, en->value);
> + switch (parse_ret) {
> + case VVENC_PARAM_BAD_NAME:
> + av_log(avctx, AV_LOG_ERROR, "Unknown vvenc option: %s.\n",
> + en->key);
> + ret = AVERROR(EINVAL);
> + break;
> + case VVENC_PARAM_BAD_VALUE:
> + av_log(avctx, AV_LOG_ERROR,
> + "Invalid vvenc value for %s: %s.\n", en->key, en->value);
> + ret = AVERROR(EINVAL);
> + break;
> + default:
> + break;
> + }
> +
> + if (memcmp(en->key, "rcstatsfile", 11) == 0 ||
> + memcmp(en->key, "RCStatsFile", 11) == 0) {
This presumes that en->key is at least 11 byte long (and it would accept
keys only prefixed by rcstatsfile); this need not be true at all. How
about using av_strcasecmp()?
> + strncpy(statsfile, en->value, strlen(statsfile));
> + statsfile[strlen(statsfile)] = '\0';
Did you test this at all?
1. Using strlen(statsfile) means that the filename of the statsfile is
bounded by strlen("vvenc-rcstats.json"); more precisely, every iteration
of this code block can grow strlen() of the buffer by one and there is
no real bounds check, so this could lead to a stack buffer overflow
(when using a dictionary that contains rcstatsfile entries multiple times).
2. We have av_strlcpy() for this.
3. But a better approach is to just not copy the string at all, avoiding
any length restriction on the statsfile: Use const char **statsfile
instead of the char *statsfile parameter; in encode_init below you just
initialize statsfile via 'const char *statsfile = "vvenc-rcstats.json";'.
The typical way to pass the statistics of a first pass to an encoder is
via an allocated buffer, not via a filename for the encoder to read
from/write to (see AVCodecContext.stats_out, stats_in). Is this possible
with libvvenc?
> + }
> + }
> + return ret;
> +}
> +
> +static int ff_vvenc_set_rc_mode(AVCodecContext *avctx, vvenc_config* params)
> +{
> + if (params->m_RCPass != -1 && params->m_RCNumPasses == 1)
> + params->m_RCNumPasses = 2; /* enable 2pass mode */
We actually have AV_CODEC_FLAG_PASS1 and AV_CODEC_FLAG_PASS2 that you
completely ignore.
> +
> + if(avctx->rc_max_rate) {
> +#if VVENC_VERSION_MAJOR > 1 || (VVENC_VERSION_MAJOR == 1 && VVENC_VERSION_MINOR > 8)
Maybe you should map all of the major, minor and patch versions to one
VVENC_VERSION?
> + params->m_RCMaxBitrate = avctx->rc_max_rate;
> +#endif
> +
> +#if VVENC_VERSION_MAJOR == 1 && VVENC_VERSION_MINOR < 11
> + /* rc_max_rate without a bit_rate enables capped CQF mode.
> + (QP + subj. optimization + max. bitrate) */
> + if(!avctx->bit_rate) {
> + av_log( avctx, AV_LOG_ERROR,
> + "Capped Constant Quality Factor mode (capped CQF) needs at "
> + "least vvenc version >= 1.11.0 (current version %s)\n",
> + vvenc_get_version());
> + return AVERROR(EINVAL);
> + }
> +#endif
> + }
> + return 0;
> +}
> +
> +static int ff_vvenc_init_extradata(AVCodecContext *avctx, VVenCContext *s)
> +{
> + int ret;
> + if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
> + ret = vvenc_get_headers(s->vvencEnc, s->pAU);
> + if (0 != ret) {
> + av_log(avctx, AV_LOG_ERROR,
> + "cannot get headers (SPS,PPS) from vvc encoder(vvenc): %s\n",
> + vvenc_get_last_error(s->vvencEnc));
> + vvenc_encoder_close(s->vvencEnc);
> + return AVERROR(EINVAL);
> + }
> +
> + if (s->pAU->payloadUsedSize <= 0) {
> + vvenc_encoder_close(s->vvencEnc);
> + return AVERROR_INVALIDDATA;
> + }
> +
> + avctx->extradata_size = s->pAU->payloadUsedSize;
> + avctx->extradata =
> + av_mallocz(avctx->extradata_size + AV_INPUT_BUFFER_PADDING_SIZE);
> + if (!avctx->extradata) {
> + av_log(avctx, AV_LOG_ERROR,
> + "Cannot allocate VVC header of size %d.\n",
> + avctx->extradata_size);
Pointless error message, like so many of your allocation errors.
> + vvenc_encoder_close(s->vvencEnc);
> + return AVERROR(ENOMEM);
> + }
> +
> + memcpy(avctx->extradata, s->pAU->payload, avctx->extradata_size);
> + memset(avctx->extradata + avctx->extradata_size, 0,
> + AV_INPUT_BUFFER_PADDING_SIZE);
Unnecessary given that you use av_mallocz().
> + }
> + return 0;
> +}
> +
> +static av_cold int ff_vvenc_encode_init(AVCodecContext *avctx)
> +{
> + int ret;
> + int framerate, qp;
> + VVenCContext *s;
> + vvenc_config params;
> + vvencPresetMode preset;
> + char statsfile[1024] = "vvenc-rcstats.json";
> +
> + s = (VVenCContext *) avctx->priv_data;
> + qp = s->options.qp;
> + preset = (vvencPresetMode) s->options.preset;
> +
> + if (avctx->flags & AV_CODEC_FLAG_INTERLACED_DCT) {
> + av_log(avctx, AV_LOG_ERROR,
> + "ff_vvenc_encode_init::init() interlaced encoding not supported yet\n");
> + return AVERROR_INVALIDDATA;
This is not invalid data; and there is no need to add the name of the
function to the error message.
(And what does "not supported yet" even mean? Does VVC even have
specific interlaced coding methods? Will this ever be supported?)
> + }
> +
> + vvenc_config_default(¶ms);
> +
> + framerate = avctx->time_base.den / avctx->time_base.num;
You are aware that the division performed here is an integer division?
And are you aware that non-cfr content does not really have a framerate?
And that for cfr content, AVCodecContext.framerate contains the framerate?
> + vvenc_init_default(¶ms, avctx->width, avctx->height, framerate,
> + (qp >= 0) ? 0 : avctx->bit_rate, (qp < 0) ? 32 : qp, preset);
> +
> + ff_vvenc_set_verbository(¶ms);
> +
> + if (avctx->thread_count > 0)
> + params.m_numThreads = avctx->thread_count;
> +
> + /* GOP settings (IDR/CRA) */
> + if (avctx->flags & AV_CODEC_FLAG_CLOSED_GOP)
> + params.m_DecodingRefreshType = VVENC_DRT_IDR;
> +
> + if (avctx->gop_size == 1) {
> + params.m_GOPSize = 1;
> + params.m_IntraPeriod = 1;
> + } else {
> + params.m_IntraPeriodSec = s->options.intraRefreshSec;
> + }
> +
> + params.m_AccessUnitDelimiter = true;
> + params.m_RCNumPasses = 1;
> +
> + params.m_usePerceptQPA = s->options.subjectiveOptimization;
> + params.m_level = (vvencLevel) s->options.levelIdc;
> + params.m_levelTier = (vvencTier) s->options.tier;
> +
> + ff_vvenc_set_framerate(avctx, ¶ms);
> +
> + ret = ff_vvenc_set_pic_format(avctx, ¶ms);
> + if( ret != 0 )
> + return ret;
> +
> + ff_vvenc_set_color_format(avctx, ¶ms);
> +
> + ret = ff_vvenc_parse_vvenc_params(avctx, ¶ms, &statsfile[0]);
> + if( ret != 0 )
> + return ret;
> +
> +
> + ret = ff_vvenc_set_rc_mode(avctx, ¶ms);
> + if( ret != 0 )
> + return ret;
> +
> + s->vvencEnc = vvenc_encoder_create();
> + if (NULL == s->vvencEnc) {
if (!s->vvencEnc) is the common check here.
> + av_log(avctx, AV_LOG_ERROR, "cannot create vvc encoder (vvenc)\n");
> + return AVERROR(ENOMEM);
> + }
> +
> + vvenc_set_msg_callback(¶ms, s->vvencEnc, ff_vvenc_log_callback);
> + ret = vvenc_encoder_open(s->vvencEnc, ¶ms);
> + if (0 != ret) {
> + av_log(avctx, AV_LOG_ERROR, "cannot open vvc encoder (vvenc): %s\n",
> + vvenc_get_last_error(s->vvencEnc));
> + vvenc_encoder_close(s->vvencEnc);
> + return AVERROR(EINVAL);
> + }
> +
> + vvenc_get_config(s->vvencEnc, ¶ms); /* get the adapted config */
> +
> + av_log(avctx, av_log_get_level(), "vvenc version: %s\n", vvenc_get_version());
> + av_log(avctx, av_log_get_level(), "%s\n",
> + vvenc_get_config_as_string(¶ms, params.m_verbosity));
> +
> + if (params.m_RCNumPasses == 2) {
> + ret = vvenc_init_pass(s->vvencEnc, params.m_RCPass - 1, &statsfile[0]);
> + if (0 != ret) {
> + av_log(avctx, AV_LOG_ERROR,
> + "cannot init pass %d for vvc encoder (vvenc): %s\n",
> + params.m_RCPass, vvenc_get_last_error(s->vvencEnc));
> + vvenc_encoder_close(s->vvencEnc);
> + return AVERROR(EINVAL);
> + }
> + }
> +
> + s->pAU = vvenc_accessUnit_alloc();
> + if( !s->pAU ){
> + av_log(avctx, AV_LOG_FATAL, "cannot allocate memory for AU payload\n");
1. There is a memory leak here, as you don't close the encoder.
2. This should be fixed by setting the FF_CODEC_CAP_INIT_CLEANUP
cap_internal; then you should (in fact, need to) remove all the
vvenc_encoder_close() calls.
(This also fixes leaks of pAU (it gets never freed on init errors at all
and leaks e.g. on ff_vvenc_init_extradata() errors.)
> + return AVERROR(ENOMEM);
> + }
> + vvenc_accessUnit_alloc_payload(s->pAU, avctx->width * avctx->height);
> + if( !s->pAU ){
This is a nonsense check: s->pAU is already set and can't be unset by
the call, so one has to check this somehow else.
> + av_log(avctx, AV_LOG_FATAL, "cannot allocate payload memory of size %d\n",
AV_LOG_ERROR, not AV_LOG_FATAL
> + avctx->width * avctx->height );
> + return AVERROR(ENOMEM);
> + }
> +
> + ret = ff_vvenc_init_extradata(avctx, s);
> + if( ret != 0 )
> + return ret;
> +
> + s->encodeDone = false;
> + return 0;
> +}
> +
> +static av_cold int ff_vvenc_encode_close(AVCodecContext * avctx)
> +{
> + VVenCContext *s = (VVenCContext *) avctx->priv_data;
> + if (s->vvencEnc) {
> + if (av_log_get_level() >= AV_LOG_VERBOSE)
> + vvenc_print_summary(s->vvencEnc);
> +
> + if (0 != vvenc_encoder_close(s->vvencEnc)) {
> + av_log(avctx, AV_LOG_ERROR, "cannot close vvenc\n");
> + return -1;
You should free the access unit even if vvenc_encoder_close() returned
an error.
> + }
> + }
> +
> + vvenc_accessUnit_free(s->pAU, true);
Can this handle the case in which s->pAU is NULL?
> +
> + return 0;
> +}
> +
> +static av_cold int ff_vvenc_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
> + const AVFrame *frame, int *got_packet)
> +{
> + VVenCContext *s = (VVenCContext *) avctx->priv_data;
> + vvencYUVBuffer *pyuvbuf;
> + vvencYUVBuffer yuvbuf;
> + int pict_type;
> + int ret;
> +
> + pyuvbuf = NULL;
> + if (frame) {
> + if (avctx->pix_fmt == AV_PIX_FMT_YUV420P10LE) {
> + vvenc_YUVBuffer_default(&yuvbuf);
> + yuvbuf.planes[0].ptr = (int16_t *) frame->data[0];
> + yuvbuf.planes[1].ptr = (int16_t *) frame->data[1];
> + yuvbuf.planes[2].ptr = (int16_t *) frame->data[2];
> +
> + yuvbuf.planes[0].width = frame->width;
> + yuvbuf.planes[0].height = frame->height;
> + /* stride is used in 16bitsamples (16bit) in vvenc, ffmpeg uses stride in bytes */
> + yuvbuf.planes[0].stride = frame->linesize[0] >> 1;
> +
> + yuvbuf.planes[1].width = frame->width >> 1;
> + yuvbuf.planes[1].height = frame->height >> 1;
> + yuvbuf.planes[1].stride = frame->linesize[1] >> 1;
> +
> + yuvbuf.planes[2].width = frame->width >> 1;
> + yuvbuf.planes[2].height = frame->height >> 1;
> + yuvbuf.planes[2].stride = frame->linesize[2] >> 1;
> +
> + yuvbuf.cts = frame->pts;
> + yuvbuf.ctsValid = true;
> + pyuvbuf = &yuvbuf;
> + } else {
> + av_log(avctx, AV_LOG_ERROR,
> + "unsupported input colorspace! input must be yuv420p10le");
> + return AVERROR(EINVAL);
Checks like these do not belong in a single encoder; they are simply
allowed to presume that avctx->pix_fmt does not change during an encode
session and that all frames reaching the decoder actually have this
pixel format.
> + }
> + }
> +
> + if (!s->encodeDone) {
> + ret = vvenc_encode(s->vvencEnc, pyuvbuf, s->pAU, &s->encodeDone);
> + if (ret != 0) {
> + av_log(avctx, AV_LOG_ERROR, "error in vvenc::encode - ret:%d\n",
> + ret);
> + return AVERROR(EINVAL);
This is not EINVAL. It is AVERROR_EXTERNAL.
> + }
> + } else {
> + *got_packet = 0;
Unecessary: *got_packet is already zero before we enter this function.
> + return 0;
> + }
> +
> + if (s->pAU->payloadUsedSize > 0) {
> + ret = ff_get_encode_buffer(avctx, pkt, s->pAU->payloadUsedSize, 0);
> + if (ret < 0) {
> + av_log(avctx, AV_LOG_ERROR, "Error getting output packet.\n");
Unnecessary, as ff_get_encode_buffer() already emits its own error messages.
> + return ret;
> + }
> +
> + memcpy(pkt->data, s->pAU->payload, s->pAU->payloadUsedSize);
> +
> + if (s->pAU->ctsValid)
> + pkt->pts = s->pAU->cts;
> + if (s->pAU->dtsValid)
> + pkt->dts = s->pAU->dts;
> + pkt->flags |= AV_PKT_FLAG_KEY * s->pAU->rap;
> +
> + switch (s->pAU->sliceType) {
> + case VVENC_I_SLICE:
> + pict_type = AV_PICTURE_TYPE_I;
> + break;
> + case VVENC_P_SLICE:
> + pict_type = AV_PICTURE_TYPE_P;
> + break;
> + case VVENC_B_SLICE:
> + pict_type = AV_PICTURE_TYPE_B;
> + break;
> + default:
> + av_log(avctx, AV_LOG_ERROR, "Unknown picture type encountered.\n");
> + return AVERROR_EXTERNAL;
> + }
> +
> + ff_side_data_set_encoder_stats(pkt, 0, NULL, 0, pict_type);
Pointless given that you do not really have meaningful statistics to set.
> +
> + *got_packet = 1;
> +
> + return 0;
> + } else {
> + *got_packet = 0;
> + return 0;
> + }
> +
> + return 0;
> +}
> +
> +static const enum AVPixelFormat pix_fmts_vvenc[] = {
> + AV_PIX_FMT_YUV420P10LE,
> + AV_PIX_FMT_NONE
> +};
> +
> +#define OFFSET(x) offsetof(VVenCContext, x)
> +#define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
> +static const AVOption libvvenc_options[] = {
> + {"preset", "set encoding preset(0: faster - 4: slower", OFFSET( options.preset), AV_OPT_TYPE_INT, {.i64 = 2} , 0 , 4 , VE, "preset"},
> + { "faster", "0", 0, AV_OPT_TYPE_CONST, {.i64 = VVENC_FASTER}, INT_MIN, INT_MAX, VE, "preset" },
> + { "fast", "1", 0, AV_OPT_TYPE_CONST, {.i64 = VVENC_FAST}, INT_MIN, INT_MAX, VE, "preset" },
> + { "medium", "2", 0, AV_OPT_TYPE_CONST, {.i64 = VVENC_MEDIUM}, INT_MIN, INT_MAX, VE, "preset" },
> + { "slow", "3", 0, AV_OPT_TYPE_CONST, {.i64 = VVENC_SLOW}, INT_MIN, INT_MAX, VE, "preset" },
> + { "slower", "4", 0, AV_OPT_TYPE_CONST, {.i64 = VVENC_SLOWER}, INT_MIN, INT_MAX, VE, "preset" },
> + { "qp" , "set quantization", OFFSET(options.qp), AV_OPT_TYPE_INT, {.i64 = -1}, -1 , 63 ,VE, "qp_mode" },
The "qp_mode" is unnecessary: AVOption.unit exists to link
AV_OPT_TYPE_CONST options to the actual options.
> + { "period" , "set (intra) refresh period in seconds", OFFSET(options.intraRefreshSec), AV_OPT_TYPE_INT, {.i64 = 1}, 1 , INT_MAX ,VE,"irefreshsec" },
We actually have AVCodecContext.gop_size, which is in frames
> + { "subjopt", "set subjective (perceptually motivated) optimization", OFFSET(options.subjectiveOptimization), AV_OPT_TYPE_BOOL, {.i64 = 1}, 0 , 1, VE},
> + { "bitdepth8", "set 8bit coding mode", OFFSET(options.flag8bitCoding), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0 , 1, VE},
Why are these separate options and not simply part of vvenc-params?
> + { "vvenc-params", "set the vvenc configuration using a :-separated list of key=value parameters", OFFSET(options.vvenc_opts), AV_OPT_TYPE_DICT, { 0 }, 0, 0, VE },
> + { "levelidc", "vvc level_idc", OFFSET( options.levelIdc), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 105, VE, "levelidc"},
> + { "0", "auto", 0, AV_OPT_TYPE_CONST, {.i64 = 0}, INT_MIN, INT_MAX, VE, "levelidc"},
> + { "1", "1" , 0, AV_OPT_TYPE_CONST, {.i64 = 16}, INT_MIN, INT_MAX, VE, "levelidc"},
> + { "2", "2" , 0, AV_OPT_TYPE_CONST, {.i64 = 32}, INT_MIN, INT_MAX, VE, "levelidc"},
> + { "2.1", "2.1" , 0, AV_OPT_TYPE_CONST, {.i64 = 35}, INT_MIN, INT_MAX, VE, "levelidc"},
> + { "3", "3" , 0, AV_OPT_TYPE_CONST, {.i64 = 48}, INT_MIN, INT_MAX, VE, "levelidc"},
> + { "3.1", "3.1" , 0, AV_OPT_TYPE_CONST, {.i64 = 51}, INT_MIN, INT_MAX, VE, "levelidc"},
> + { "4", "4" , 0, AV_OPT_TYPE_CONST, {.i64 = 64}, INT_MIN, INT_MAX, VE, "levelidc"},
> + { "4.1", "4.1" , 0, AV_OPT_TYPE_CONST, {.i64 = 67}, INT_MIN, INT_MAX, VE, "levelidc"},
> + { "5", "5" , 0, AV_OPT_TYPE_CONST, {.i64 = 80}, INT_MIN, INT_MAX, VE, "levelidc"},
> + { "5.1", "5.1" , 0, AV_OPT_TYPE_CONST, {.i64 = 83}, INT_MIN, INT_MAX, VE, "levelidc"},
> + { "5.2", "5.2" , 0, AV_OPT_TYPE_CONST, {.i64 = 86}, INT_MIN, INT_MAX, VE, "levelidc"},
> + { "6", "6" , 0, AV_OPT_TYPE_CONST, {.i64 = 96}, INT_MIN, INT_MAX, VE, "levelidc"},
> + { "6.1", "6.1" , 0, AV_OPT_TYPE_CONST, {.i64 = 99}, INT_MIN, INT_MAX, VE, "levelidc"},
> + { "6.2", "6.2" , 0, AV_OPT_TYPE_CONST, {.i64 = 102}, INT_MIN, INT_MAX, VE, "levelidc"},
> + { "6.3", "6.3" , 0, AV_OPT_TYPE_CONST, {.i64 = 105}, INT_MIN, INT_MAX, VE, "levelidc"},
We have a generic level option, so there is no need for this list above.
> + { "tier", "set vvc tier", OFFSET( options.tier), AV_OPT_TYPE_INT, {.i64 = 0}, 0 , 1 , VE, "tier"},
> + { "main", "main", 0, AV_OPT_TYPE_CONST, {.i64 = 0}, INT_MIN, INT_MAX, VE, "tier"},
> + { "high", "high", 0, AV_OPT_TYPE_CONST, {.i64 = 1}, INT_MIN, INT_MAX, VE, "tier"},
> + {NULL}
> +};
> +
> +static const AVClass class_libvvenc = {
> + .class_name = "libvvenc-vvc encoder",
> + .item_name = av_default_item_name,
> + .option = libvvenc_options,
> + .version = LIBAVUTIL_VERSION_INT,
> +};
> +
> +static const FFCodecDefault vvenc_defaults[] = {
> + { "b", "0" },
> + { "g", "-1" },
> + { NULL },
> +};
> +
> +FFCodec ff_libvvenc_encoder = {
Missing const
> + .p.name = "libvvenc",
> + CODEC_LONG_NAME("H.266 / VVC Encoder VVenC"),
> + .p.type = AVMEDIA_TYPE_VIDEO,
> + .p.id = AV_CODEC_ID_VVC,
> + .p.capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_OTHER_THREADS,
You can add AV_CODEC_CAP_DR1
> + .p.profiles = NULL_IF_CONFIG_SMALL(ff_vvc_profiles),
> + .p.priv_class = &class_libvvenc,
> + .p.wrapper_name = "libvvenc",
> + .priv_data_size = sizeof(VVenCContext),
> + .p.pix_fmts = pix_fmts_vvenc,
> + .init = ff_vvenc_encode_init,
> + FF_CODEC_ENCODE_CB(ff_vvenc_encode_frame),
> + .close = ff_vvenc_encode_close,
> + .defaults = vvenc_defaults,
> + .caps_internal = FF_CODEC_CAP_AUTO_THREADS,
> +};
More information about the ffmpeg-devel
mailing list