* [FFmpeg-devel] [PATCH v3 0/2] Add support for H266/VVC encoding
@ 2024-05-14 15:09 Christian Bartnik
2024-05-14 15:09 ` [FFmpeg-devel] [PATCH v3 1/2] avcodec: add external enc libvvenc for H266/VVC Christian Bartnik
2024-05-14 15:09 ` [FFmpeg-devel] [PATCH v3 2/2] avcodec: add external dec libvvdec " Christian Bartnik
0 siblings, 2 replies; 21+ messages in thread
From: Christian Bartnik @ 2024-05-14 15:09 UTC (permalink / raw)
To: ffmpeg-devel
This patchset is based on the latest patchset from Thomas Siedel
(thomas.ff@spin-digital.com).
Since almost all changes from the patchset but libvvenc and libvvdec has been
merged this patch only implements the libvvenc and libvvdec wrapper
implementation.
As ffmpeg already has it´s own vvc decoder, feel free to cherry pick libvvenc
only.
The libvvdec patch has been cleaned up by removing the extradata parsing files
and using existing code from cbs_h266.
The libvvenc patch only has been cleaned up with following changes:
- add defaults struct vvenc_defaults
- fix: init qp value (typo)
- cleanup init function (move code into sub functions)
- cleanup verbosity
- add check for payload allocation
- vvenc-params return error for invalid options or values
- add support for capped CRF mode (QP + subj.Optimization + max. bitrate) if vvenc version >= 1.11.0
- add vvenc documentation in doc/encoders.texi
Christian Bartnik (2):
avcodec: add external enc libvvenc for H266/VVC
avcodec: add external dec libvvdec for H266/VVC
configure | 9 +
doc/encoders.texi | 65 +++++
libavcodec/Makefile | 2 +
libavcodec/allcodecs.c | 2 +
libavcodec/libvvdec.c | 617 +++++++++++++++++++++++++++++++++++++++++
libavcodec/libvvenc.c | 566 +++++++++++++++++++++++++++++++++++++
6 files changed, 1261 insertions(+)
create mode 100644 libavcodec/libvvdec.c
create mode 100644 libavcodec/libvvenc.c
--
2.34.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 21+ messages in thread
* [FFmpeg-devel] [PATCH v3 1/2] avcodec: add external enc libvvenc for H266/VVC
2024-05-14 15:09 [FFmpeg-devel] [PATCH v3 0/2] Add support for H266/VVC encoding Christian Bartnik
@ 2024-05-14 15:09 ` Christian Bartnik
2024-05-14 16:49 ` Andreas Rheinhardt
2024-05-14 15:09 ` [FFmpeg-devel] [PATCH v3 2/2] avcodec: add external dec libvvdec " Christian Bartnik
1 sibling, 1 reply; 21+ messages in thread
From: Christian Bartnik @ 2024-05-14 15:09 UTC (permalink / raw)
To: ffmpeg-devel
From: Thomas Siedel <thomas.ff@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@gmail.com
Signed-off-by: Christian Bartnik <chris10317h5@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/}
+@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
+@option{--enable-libvvenc}.
+
+The VVenC project website is at
+@url{https://github.com/fraunhoferhhi/vvenc}.
+
+@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.
+
+@subsection Options
+
+@table @option
+@item b
+Sets target video bitrate.
+
+@item g
+Set the GOP size. Currently support for g=1 (Intra only) or default.
+
+@item preset
+Set the VVenC preset.
+
+@item levelidc
+Set level idc.
+
+@item tier
+Set vvc tier.
+
+@item qp
+Set constant quantization parameter.
+
+@item subopt @var{boolean}
+Set subjective (perceptually motivated) optimization. Default is 1 (on).
+
+@item bitdepth8 @var{boolean}
+Set 8bit coding mode instead of using 10bit. Default is 0 (off).
+
+@item period
+set (intra) refresh period in seconds.
+
+@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:
+
+@example
+intraperiod=64:decodingrefreshtype=idr:poc0idr=1:internalbitdepth=8
+@end example
+
+For example the encoding options for 2-pass encoding might be provided with @option{-vvenc-params}:
+
+@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
+@end example
+
+@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"
+
+#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;
+} VVenCContext;
+
+
+static av_cold void ff_vvenc_log_callback(void *ctx, int level,
+ const char *fmt, va_list args)
+{
+ 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 )
+{
+ params->m_verbosity = VVENC_VERBOSE;
+ if (av_log_get_level() >= AV_LOG_DEBUG)
+ 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;
+
+ 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);
+ }
+
+ 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;
+ s =(VVenCContext *) avctx->priv_data;
+ ret = 0;
+
+ while ((en = av_dict_get(s->options.vvenc_opts, "", en,
+ AV_DICT_IGNORE_SUFFIX))) {
+ 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) {
+ strncpy(statsfile, en->value, strlen(statsfile));
+ statsfile[strlen(statsfile)] = '\0';
+ }
+ }
+ 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 */
+
+ if(avctx->rc_max_rate) {
+#if VVENC_VERSION_MAJOR > 1 || (VVENC_VERSION_MAJOR == 1 && VVENC_VERSION_MINOR > 8)
+ 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);
+ 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);
+ }
+ 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;
+ }
+
+ vvenc_config_default(¶ms);
+
+ framerate = avctx->time_base.den / avctx->time_base.num;
+ 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) {
+ 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");
+ return AVERROR(ENOMEM);
+ }
+ vvenc_accessUnit_alloc_payload(s->pAU, avctx->width * avctx->height);
+ if( !s->pAU ){
+ av_log(avctx, AV_LOG_FATAL, "cannot allocate payload memory of size %d\n",
+ 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;
+ }
+ }
+
+ vvenc_accessUnit_free(s->pAU, true);
+
+ 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);
+ }
+ }
+
+ 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);
+ }
+ } else {
+ *got_packet = 0;
+ 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");
+ 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);
+
+ *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" },
+ { "period" , "set (intra) refresh period in seconds", OFFSET(options.intraRefreshSec), AV_OPT_TYPE_INT, {.i64 = 1}, 1 , INT_MAX ,VE,"irefreshsec" },
+ { "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},
+ { "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"},
+ { "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 = {
+ .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,
+ .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,
+};
--
2.34.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 21+ messages in thread
* [FFmpeg-devel] [PATCH v3 2/2] avcodec: add external dec libvvdec for H266/VVC
2024-05-14 15:09 [FFmpeg-devel] [PATCH v3 0/2] Add support for H266/VVC encoding Christian Bartnik
2024-05-14 15:09 ` [FFmpeg-devel] [PATCH v3 1/2] avcodec: add external enc libvvenc for H266/VVC Christian Bartnik
@ 2024-05-14 15:09 ` Christian Bartnik
2024-05-14 16:28 ` Lynne via ffmpeg-devel
1 sibling, 1 reply; 21+ messages in thread
From: Christian Bartnik @ 2024-05-14 15:09 UTC (permalink / raw)
To: ffmpeg-devel
From: Thomas Siedel <thomas.ff@spin-digital.com>
Add external decoder VVdeC for H266/VVC decoding.
Register new decoder libvvdec.
Add libvvdec to wrap the vvdec interface.
Enable decoder by adding --enable-libvvdec in configure step.
Co-authored-by: Christian Bartnik chris10317h5@gmail.com
Signed-off-by: Christian Bartnik <chris10317h5@gmail.com>
---
configure | 5 +
libavcodec/Makefile | 1 +
libavcodec/allcodecs.c | 1 +
libavcodec/libvvdec.c | 617 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 624 insertions(+)
create mode 100644 libavcodec/libvvdec.c
diff --git a/configure b/configure
index 5d9a14821b..a5df482215 100755
--- a/configure
+++ b/configure
@@ -294,6 +294,7 @@ External library support:
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-libvvdec enable H.266/VVC decoding via vvdec [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]
@@ -1968,6 +1969,7 @@ EXTERNAL_LIBRARY_LIST="
libvorbis
libvpx
libvvenc
+ libvvdec
libwebp
libxevd
libxeve
@@ -3561,6 +3563,8 @@ libvpx_vp8_encoder_deps="libvpx"
libvpx_vp9_decoder_deps="libvpx"
libvpx_vp9_encoder_deps="libvpx"
libvvenc_encoder_deps="libvvenc"
+libvvdec_decoder_deps="libvvdec"
+libvvdec_decoder_select="vvc_mp4toannexb_bsf"
libwebp_encoder_deps="libwebp"
libwebp_anim_encoder_deps="libwebp"
libx262_encoder_deps="libx262"
@@ -7029,6 +7033,7 @@ enabled libvpx && {
fi
}
enabled libvvenc && require_pkg_config libvvenc "libvvenc >= 1.6.1" "vvenc/vvenc.h" vvenc_get_version
+enabled libvvdec && require_pkg_config libvvdec "libvvdec >= 1.6.0" "vvdec/vvdec.h" vvdec_get_version
enabled libwebp && {
enabled libwebp_encoder && require_pkg_config libwebp "libwebp >= 0.2.0" webp/encode.h WebPGetEncoderVersion
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 5d7349090e..318b22a1fa 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -1154,6 +1154,7 @@ 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_LIBVVDEC_DECODER) += libvvdec.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 59d36dbd56..4120681d17 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -801,6 +801,7 @@ 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;
+extern const FFCodec ff_libvvdec_decoder;
/* preferred over libwebp */
extern const FFCodec ff_libwebp_anim_encoder;
extern const FFCodec ff_libwebp_encoder;
diff --git a/libavcodec/libvvdec.c b/libavcodec/libvvdec.c
new file mode 100644
index 0000000000..7f94a81b37
--- /dev/null
+++ b/libavcodec/libvvdec.c
@@ -0,0 +1,617 @@
+/*
+ * H.266 decoding using the VVdeC 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"
+
+#include <vvdec/vvdec.h>
+
+#include "libavutil/common.h"
+#include "libavutil/avutil.h"
+#include "libavutil/pixdesc.h"
+#include "libavutil/opt.h"
+#include "libavutil/imgutils.h"
+#include "libavutil/frame.h"
+#include "libavutil/mastering_display_metadata.h"
+#include "libavutil/log.h"
+
+#include "avcodec.h"
+#include "codec_internal.h"
+#include "decode.h"
+#include "internal.h"
+#include "profiles.h"
+
+#include "cbs_h266.h"
+
+typedef struct VVdeCContext {
+ AVClass *av_class;
+ vvdecDecoder *vvdecDec;
+ vvdecParams vvdecParams;
+ bool bFlush;
+ AVBufferPool *pools[3]; /** Pools for each data plane. */
+ int pool_size[3];
+ CodedBitstreamContext *cbc;
+ CodedBitstreamFragment current_frame;
+} VVdeCContext;
+
+
+static void ff_vvdec_log_callback(void *avctx, int level, const char *fmt,
+ va_list args)
+{
+ vfprintf(level == 1 ? stderr : stdout, fmt, args);
+}
+
+static void *ff_vvdec_buffer_allocator(void *ctx, vvdecComponentType comp,
+ uint32_t size, uint32_t alignment,
+ void **allocator)
+{
+ AVBufferRef *buf;
+ VVdeCContext *s;
+ int plane;
+
+ uint32_t alignedsize = FFALIGN(size, alignment);
+ s = (VVdeCContext *) ctx;
+ plane = (int) comp;
+
+ if (plane < 0 || plane > 3)
+ return NULL;
+
+ if (alignedsize != s->pool_size[plane]) {
+ av_buffer_pool_uninit(&s->pools[plane]);
+ s->pools[plane] = av_buffer_pool_init(alignedsize, NULL);
+ if (!s->pools[plane]) {
+ s->pool_size[plane] = 0;
+ return NULL;
+ }
+ s->pool_size[plane] = alignedsize;
+ }
+
+ buf = av_buffer_pool_get(s->pools[plane]);
+ if (!buf)
+ return NULL;
+
+ *allocator = (void *) buf;
+ return buf->data;
+}
+
+static void ff_vvdec_buffer_unref(void *ctx, void *allocator)
+{
+ AVBufferRef *buf = (AVBufferRef *) allocator;
+ av_buffer_unref(&buf);
+}
+
+static void ff_vvdec_printParameterInfo(AVCodecContext *avctx,
+ vvdecParams *params)
+{
+ av_log(avctx, AV_LOG_DEBUG, "Version info: vvdec %s ( threads %d)\n",
+ vvdec_get_version(), params->threads);
+}
+
+static int ff_vvdec_set_pix_fmt(AVCodecContext *avctx, vvdecFrame *frame)
+{
+ if (NULL != frame->picAttributes && NULL != frame->picAttributes->vui &&
+ frame->picAttributes->vui->colourDescriptionPresentFlag) {
+ avctx->color_trc = frame->picAttributes->vui->transferCharacteristics;
+ avctx->color_primaries = frame->picAttributes->vui->colourPrimaries;
+ avctx->colorspace = frame->picAttributes->vui->matrixCoefficients;
+ } else {
+ avctx->color_primaries = AVCOL_PRI_UNSPECIFIED;
+ avctx->color_trc = AVCOL_TRC_UNSPECIFIED;
+ avctx->colorspace = AVCOL_SPC_UNSPECIFIED;
+ }
+
+ if (NULL != frame->picAttributes && NULL != frame->picAttributes->vui &&
+ frame->picAttributes->vui->videoSignalTypePresentFlag) {
+ avctx->color_range = frame->picAttributes->vui->videoFullRangeFlag ?
+ AVCOL_RANGE_JPEG : AVCOL_RANGE_MPEG;
+ } else {
+ avctx->color_range = AVCOL_RANGE_MPEG;
+ }
+
+ switch (frame->colorFormat) {
+ case VVDEC_CF_YUV420_PLANAR:
+ case VVDEC_CF_YUV400_PLANAR:
+ if (frame->bitDepth == 8) {
+ avctx->pix_fmt = frame->numPlanes == 1 ?
+ AV_PIX_FMT_GRAY8 : AV_PIX_FMT_YUV420P;
+ avctx->profile = FF_PROFILE_VVC_MAIN_10;
+ return 0;
+ } else if (frame->bitDepth == 10) {
+ avctx->pix_fmt = frame->numPlanes == 1 ?
+ AV_PIX_FMT_GRAY10 : AV_PIX_FMT_YUV420P10;
+ avctx->profile = FF_PROFILE_VVC_MAIN_10;
+ return 0;
+ } else {
+ return AVERROR_INVALIDDATA;
+ }
+ case VVDEC_CF_YUV422_PLANAR:
+ case VVDEC_CF_YUV444_PLANAR:
+ if (frame->bitDepth == 8) {
+ avctx->pix_fmt = frame->colorFormat == VVDEC_CF_YUV444_PLANAR ?
+ AV_PIX_FMT_YUV444P : AV_PIX_FMT_YUV422P;
+ if (avctx->profile != FF_PROFILE_VVC_MAIN_10_444)
+ avctx->profile = FF_PROFILE_VVC_MAIN_10_444;
+ return 0;
+ } else if (frame->bitDepth == 10) {
+ avctx->pix_fmt = frame->colorFormat == VVDEC_CF_YUV444_PLANAR ?
+ AV_PIX_FMT_YUV444P10 : AV_PIX_FMT_YUV422P10;
+ if (avctx->profile != FF_PROFILE_VVC_MAIN_10_444)
+ avctx->profile = FF_PROFILE_VVC_MAIN_10_444;
+ return 0;
+ } else {
+ return AVERROR_INVALIDDATA;
+ }
+ default:
+ return AVERROR_INVALIDDATA;
+ }
+}
+
+static int set_side_data(AVCodecContext *avctx, AVFrame *avframe,
+ vvdecFrame *frame)
+{
+ vvdecSEI *sei;
+ VVdeCContext *s = (VVdeCContext *) avctx->priv_data;
+
+ sei = vvdec_find_frame_sei(s->vvdecDec,
+ VVDEC_MASTERING_DISPLAY_COLOUR_VOLUME, frame);
+ if (sei) {
+ // VVC uses a g,b,r ordering, which we convert to a more natural r,g,b
+ const int mapping[3] = { 2, 0, 1 };
+ const int chroma_den = 50000;
+ const int luma_den = 10000;
+ int i;
+ vvdecSEIMasteringDisplayColourVolume *p;
+ AVMasteringDisplayMetadata *metadata =
+ av_mastering_display_metadata_create_side_data(avframe);
+ p = (vvdecSEIMasteringDisplayColourVolume *) (sei->payload);
+ if (p && metadata) {
+ for (i = 0; i < 3; i++) {
+ const int j = mapping[i];
+ metadata->display_primaries[i][0].num = p->primaries[j][0];
+ metadata->display_primaries[i][0].den = chroma_den;
+ metadata->display_primaries[i][1].num = p->primaries[j][1];
+ metadata->display_primaries[i][1].den = chroma_den;
+ }
+ metadata->white_point[0].num = p->whitePoint[0];
+ metadata->white_point[0].den = chroma_den;
+ metadata->white_point[1].num = p->whitePoint[1];
+ metadata->white_point[1].den = chroma_den;
+
+ metadata->max_luminance.num = p->maxLuminance;
+ metadata->max_luminance.den = luma_den;
+ metadata->min_luminance.num = p->minLuminance;
+ metadata->min_luminance.den = luma_den;
+ metadata->has_luminance = 1;
+ metadata->has_primaries = 1;
+
+ av_log(avctx, AV_LOG_DEBUG, "Mastering Display Metadata:\n");
+ av_log(avctx, AV_LOG_DEBUG,
+ "r(%5.4f,%5.4f) g(%5.4f,%5.4f) b(%5.4f %5.4f) wp(%5.4f, %5.4f)\n",
+ av_q2d(metadata->display_primaries[0][0]),
+ av_q2d(metadata->display_primaries[0][1]),
+ av_q2d(metadata->display_primaries[1][0]),
+ av_q2d(metadata->display_primaries[1][1]),
+ av_q2d(metadata->display_primaries[2][0]),
+ av_q2d(metadata->display_primaries[2][1]),
+ av_q2d(metadata->white_point[0]),
+ av_q2d(metadata->white_point[1]));
+ av_log(avctx, AV_LOG_DEBUG, "min_luminance=%f, max_luminance=%f\n",
+ av_q2d(metadata->min_luminance),
+ av_q2d(metadata->max_luminance));
+ }
+ return 0;
+ }
+
+ sei = vvdec_find_frame_sei(s->vvdecDec, VVDEC_CONTENT_LIGHT_LEVEL_INFO,
+ frame);
+ if (sei) {
+ vvdecSEIContentLightLevelInfo *p = NULL;
+ AVContentLightMetadata *light =
+ av_content_light_metadata_create_side_data(avframe);
+ p = (vvdecSEIContentLightLevelInfo *) (sei->payload);
+ if (p && light) {
+ light->MaxCLL = p->maxContentLightLevel;
+ light->MaxFALL = p->maxPicAverageLightLevel;
+ }
+
+ av_log(avctx, AV_LOG_DEBUG, "Content Light Level Metadata:\n");
+ av_log(avctx, AV_LOG_DEBUG, "MaxCLL=%d, MaxFALL=%d\n",
+ light->MaxCLL, light->MaxFALL);
+ }
+
+ return 0;
+}
+
+static int set_pixel_format(AVCodecContext *avctx, const H266RawSPS *sps)
+{
+ enum AVPixelFormat pix_fmt = AV_PIX_FMT_NONE;
+ const AVPixFmtDescriptor *desc;
+ switch (sps->sps_bitdepth_minus8+8) {
+ case 8:
+ if (sps->sps_chroma_format_idc == 0)
+ pix_fmt = AV_PIX_FMT_GRAY8;
+ if (sps->sps_chroma_format_idc == 1)
+ pix_fmt = AV_PIX_FMT_YUV420P;
+ if (sps->sps_chroma_format_idc == 2)
+ pix_fmt = AV_PIX_FMT_YUV422P;
+ if (sps->sps_chroma_format_idc == 3)
+ pix_fmt = AV_PIX_FMT_YUV444P;
+ break;
+ case 9:
+ if (sps->sps_chroma_format_idc == 0)
+ pix_fmt = AV_PIX_FMT_GRAY9;
+ if (sps->sps_chroma_format_idc == 1)
+ pix_fmt = AV_PIX_FMT_YUV420P9;
+ if (sps->sps_chroma_format_idc == 2)
+ pix_fmt = AV_PIX_FMT_YUV422P9;
+ if (sps->sps_chroma_format_idc == 3)
+ pix_fmt = AV_PIX_FMT_YUV444P9;
+ break;
+ case 10:
+ if (sps->sps_chroma_format_idc == 0)
+ pix_fmt = AV_PIX_FMT_GRAY10;
+ if (sps->sps_chroma_format_idc == 1)
+ pix_fmt = AV_PIX_FMT_YUV420P10;
+ if (sps->sps_chroma_format_idc == 2)
+ pix_fmt = AV_PIX_FMT_YUV422P10;
+ if (sps->sps_chroma_format_idc == 3)
+ pix_fmt = AV_PIX_FMT_YUV444P10;
+ break;
+ case 12:
+ if (sps->sps_chroma_format_idc == 0)
+ pix_fmt = AV_PIX_FMT_GRAY12;
+ if (sps->sps_chroma_format_idc == 1)
+ pix_fmt = AV_PIX_FMT_YUV420P12;
+ if (sps->sps_chroma_format_idc == 2)
+ pix_fmt = AV_PIX_FMT_YUV422P12;
+ if (sps->sps_chroma_format_idc == 3)
+ pix_fmt = AV_PIX_FMT_YUV444P12;
+ break;
+ default:
+ av_log(avctx, AV_LOG_ERROR,
+ "The following bit-depths are currently specified: 8, 9, 10 and 12 bits, "
+ "sps_chroma_format_idc is %d, depth is %d\n",
+ sps->sps_chroma_format_idc, sps->sps_bitdepth_minus8+8);
+ return AVERROR_INVALIDDATA;
+ }
+
+ desc = av_pix_fmt_desc_get(pix_fmt);
+ if (!desc)
+ return AVERROR(EINVAL);
+
+ avctx->pix_fmt = pix_fmt;
+
+ return 0;
+}
+
+static void export_stream_params(AVCodecContext *avctx, const H266RawSPS *sps)
+{
+ avctx->coded_width = sps->sps_pic_width_max_in_luma_samples;
+ avctx->coded_height = sps->sps_pic_height_max_in_luma_samples;
+ avctx->width = sps->sps_pic_width_max_in_luma_samples -
+ sps->sps_conf_win_left_offset -
+ sps->sps_conf_win_right_offset;
+ avctx->height = sps->sps_pic_height_max_in_luma_samples -
+ sps->sps_conf_win_top_offset -
+ sps->sps_conf_win_bottom_offset;
+ avctx->has_b_frames = sps->sps_max_sublayers_minus1+1;
+ avctx->profile = sps->profile_tier_level.general_profile_idc;
+ avctx->level = sps->profile_tier_level.general_level_idc;
+
+ set_pixel_format( avctx, sps);
+
+ avctx->color_range = sps->vui.vui_full_range_flag ? AVCOL_RANGE_JPEG :
+ AVCOL_RANGE_MPEG;
+
+ if (sps->vui.vui_colour_description_present_flag) {
+ avctx->color_primaries = sps->vui.vui_colour_primaries;
+ avctx->color_trc = sps->vui.vui_transfer_characteristics;
+ avctx->colorspace = sps->vui.vui_matrix_coeffs;
+ } else {
+ avctx->color_primaries = AVCOL_PRI_UNSPECIFIED;
+ avctx->color_trc = AVCOL_TRC_UNSPECIFIED;
+ avctx->colorspace = AVCOL_SPC_UNSPECIFIED;
+ }
+
+ avctx->chroma_sample_location = AVCHROMA_LOC_UNSPECIFIED;
+ if (sps->sps_chroma_format_idc == 1) {
+ if (sps->vui.vui_chroma_loc_info_present_flag) {
+ if (sps->vui.vui_chroma_sample_loc_type_top_field <= 5)
+ avctx->chroma_sample_location =
+ sps->vui.vui_chroma_sample_loc_type_top_field + 1;
+ } else
+ avctx->chroma_sample_location = AVCHROMA_LOC_LEFT;
+ }
+
+ if (sps->sps_timing_hrd_params_present_flag &&
+ sps->sps_general_timing_hrd_parameters.num_units_in_tick &&
+ sps->sps_general_timing_hrd_parameters.time_scale) {
+ av_reduce(&avctx->framerate.den, &avctx->framerate.num,
+ sps->sps_general_timing_hrd_parameters.num_units_in_tick,
+ sps->sps_general_timing_hrd_parameters.time_scale, INT_MAX);
+ }
+}
+
+static av_cold int ff_vvdec_decode_init(AVCodecContext *avctx)
+{
+ int i, ret;
+ VVdeCContext *s = (VVdeCContext *) avctx->priv_data;
+
+ vvdec_params_default(&s->vvdecParams);
+ s->vvdecParams.logLevel = VVDEC_DETAILS;
+
+ if (av_log_get_level() >= AV_LOG_DEBUG)
+ s->vvdecParams.logLevel = VVDEC_DETAILS;
+ else if (av_log_get_level() >= AV_LOG_VERBOSE)
+ s->vvdecParams.logLevel = VVDEC_INFO; // VVDEC_INFO will output per picture info
+ else if (av_log_get_level() >= AV_LOG_INFO)
+ s->vvdecParams.logLevel = VVDEC_WARNING; // AV_LOG_INFO is ffmpeg default
+ else
+ s->vvdecParams.logLevel = VVDEC_SILENT;
+
+ if (avctx->thread_count > 0)
+ s->vvdecParams.threads = avctx->thread_count; // number of worker threads (should not exceed the number of physical cpu's)
+ else
+ s->vvdecParams.threads = -1; // get max cpus
+
+ ff_vvdec_printParameterInfo(avctx, &s->vvdecParams);
+
+ // using buffer allocation by using AVBufferPool
+ s->vvdecParams.opaque = avctx->priv_data;
+ s->vvdecDec = vvdec_decoder_open_with_allocator(&s->vvdecParams,
+ ff_vvdec_buffer_allocator,
+ ff_vvdec_buffer_unref);
+
+
+ if (!s->vvdecDec) {
+ av_log(avctx, AV_LOG_ERROR, "cannot init vvdec decoder\n");
+ return -1;
+ }
+
+ vvdec_set_logging_callback(s->vvdecDec, ff_vvdec_log_callback);
+
+ s->bFlush = false;
+
+ for (i = 0; i < FF_ARRAY_ELEMS(s->pools); i++) {
+ s->pools[i] = NULL;
+ s->pool_size[i] = 0;
+ }
+
+ ret = ff_cbs_init(&s->cbc, AV_CODEC_ID_VVC, avctx);
+ if (ret)
+ return ret;
+
+ if (!avctx->internal->is_copy) {
+ if (avctx->extradata_size > 0 && avctx->extradata) {
+ const CodedBitstreamH266Context *h266 = s->cbc->priv_data;
+ ff_cbs_fragment_reset(&s->current_frame);
+ ret = ff_cbs_read_extradata_from_codec(s->cbc, &s->current_frame, avctx);
+ if (ret < 0)
+ return ret;
+
+ if ( h266->sps[0] != NULL)
+ export_stream_params(avctx, h266->sps[0]);
+ }
+ }
+
+ return 0;
+}
+
+static av_cold int ff_vvdec_decode_close(AVCodecContext *avctx)
+{
+ VVdeCContext *s = (VVdeCContext *) avctx->priv_data;
+
+ for (int i = 0; i < FF_ARRAY_ELEMS(s->pools); i++) {
+ av_buffer_pool_uninit(&s->pools[i]);
+ s->pool_size[i] = 0;
+ }
+
+ if (0 != vvdec_decoder_close(s->vvdecDec)) {
+ av_log(avctx, AV_LOG_ERROR, "cannot close vvdec\n");
+ return -1;
+ }
+
+ ff_cbs_fragment_free(&s->current_frame);
+ ff_cbs_close(&s->cbc);
+
+ s->bFlush = false;
+ return 0;
+}
+
+static av_cold int ff_vvdec_decode_frame(AVCodecContext *avctx, AVFrame *data,
+ int *got_frame, AVPacket *avpkt)
+{
+ VVdeCContext *s = avctx->priv_data;
+ AVFrame *avframe = data;
+
+ int ret = 0;
+ vvdecFrame *frame = NULL;
+
+ if (avframe) {
+ if (!avpkt->size && !s->bFlush)
+ s->bFlush = true;
+
+ if (s->bFlush)
+ ret = vvdec_flush(s->vvdecDec, &frame);
+ else {
+ vvdecAccessUnit accessUnit;
+ vvdec_accessUnit_default(&accessUnit);
+ accessUnit.payload = avpkt->data;
+ accessUnit.payloadSize = avpkt->size;
+ accessUnit.payloadUsedSize = avpkt->size;
+
+ accessUnit.cts = avpkt->pts;
+ accessUnit.ctsValid = true;
+ accessUnit.dts = avpkt->dts;
+ accessUnit.dtsValid = true;
+
+ ret = vvdec_decode(s->vvdecDec, &accessUnit, &frame);
+ }
+
+ if (ret < 0) {
+ if (ret == VVDEC_EOF)
+ s->bFlush = true;
+ else if (ret != VVDEC_TRY_AGAIN) {
+ av_log(avctx, AV_LOG_ERROR,
+ "error in vvdec::decode - ret:%d - %s %s\n", ret,
+ vvdec_get_last_error(s->vvdecDec), vvdec_get_last_additional_error( s->vvdecDec));
+ ret=AVERROR_EXTERNAL;
+ goto fail;
+ }
+ } else if (NULL != frame) {
+ const uint8_t *src_data[4] = { frame->planes[0].ptr,
+ frame->planes[1].ptr,
+ frame->planes[2].ptr, NULL };
+ const int src_linesizes[4] = { (int) frame->planes[0].stride,
+ (int) frame->planes[1].stride,
+ (int) frame->planes[2].stride, 0 };
+
+ if ((ret = ff_vvdec_set_pix_fmt(avctx, frame)) < 0) {
+ av_log(avctx, AV_LOG_ERROR,
+ "Unsupported output colorspace (%d) / bit_depth (%d)\n",
+ frame->colorFormat, frame->bitDepth);
+ goto fail;
+ }
+
+ if ((int) frame->width != avctx->width ||
+ (int) frame->height != avctx->height) {
+ av_log(avctx, AV_LOG_INFO, "dimension change! %dx%d -> %dx%d\n",
+ avctx->width, avctx->height, frame->width, frame->height);
+
+ ret = ff_set_dimensions(avctx, frame->width, frame->height);
+ if (ret < 0)
+ goto fail;
+ }
+
+ if (frame->planes[0].allocator)
+ avframe->buf[0] =
+ av_buffer_ref((AVBufferRef *) frame->planes[0].allocator);
+ if (frame->planes[1].allocator)
+ avframe->buf[1] =
+ av_buffer_ref((AVBufferRef *) frame->planes[1].allocator);
+ if (frame->planes[2].allocator)
+ avframe->buf[2] =
+ av_buffer_ref((AVBufferRef *) frame->planes[2].allocator);
+
+ for (int i = 0; i < 4; i++) {
+ avframe->data[i] = (uint8_t *) src_data[i];
+ avframe->linesize[i] = src_linesizes[i];
+ }
+
+ ret = ff_decode_frame_props(avctx, avframe);
+ if (ret < 0)
+ goto fail;
+
+ if (frame->picAttributes) {
+ if (frame->picAttributes->isRefPic)
+ avframe->flags |= AV_FRAME_FLAG_KEY;
+ else
+ avframe->flags &= ~AV_FRAME_FLAG_KEY;
+
+ avframe->pict_type = (frame->picAttributes->sliceType !=
+ VVDEC_SLICETYPE_UNKNOWN) ?
+ frame->picAttributes->sliceType + 1 : AV_PICTURE_TYPE_NONE;
+ }
+
+ if (frame->ctsValid)
+ avframe->pts = frame->cts;
+
+ ret = set_side_data(avctx, avframe, frame);
+ if (ret < 0)
+ goto fail;
+
+ if (0 != vvdec_frame_unref(s->vvdecDec, frame))
+ av_log(avctx, AV_LOG_ERROR, "cannot free picture memory\n");
+
+ *got_frame = 1;
+ }
+ }
+
+ return avpkt->size;
+
+ fail:
+ if (frame) {
+ if (frame->planes[0].allocator)
+ av_buffer_unref((AVBufferRef **) &frame->planes[0].allocator);
+ if (frame->planes[1].allocator)
+ av_buffer_unref((AVBufferRef **) &frame->planes[1].allocator);
+ if (frame->planes[2].allocator)
+ av_buffer_unref((AVBufferRef **) &frame->planes[2].allocator);
+
+ vvdec_frame_unref(s->vvdecDec, frame);
+ }
+ return ret;
+}
+
+static av_cold void ff_vvdec_decode_flush(AVCodecContext *avctx)
+{
+ VVdeCContext *s = (VVdeCContext *) avctx->priv_data;
+
+ if (0 != vvdec_decoder_close(s->vvdecDec))
+ av_log(avctx, AV_LOG_ERROR, "cannot close vvdec during flush\n");
+
+ s->vvdecDec = vvdec_decoder_open_with_allocator(&s->vvdecParams,
+ ff_vvdec_buffer_allocator,
+ ff_vvdec_buffer_unref);
+ if (!s->vvdecDec)
+ av_log(avctx, AV_LOG_ERROR, "cannot reinit vvdec during flush\n");
+
+ vvdec_set_logging_callback(s->vvdecDec, ff_vvdec_log_callback);
+
+ s->bFlush = false;
+}
+
+static const enum AVPixelFormat pix_fmts_vvdec[] = {
+ AV_PIX_FMT_GRAY8,
+ AV_PIX_FMT_GRAY10,
+ AV_PIX_FMT_YUV420P,
+ AV_PIX_FMT_YUV422P,
+ AV_PIX_FMT_YUV444P,
+ AV_PIX_FMT_YUV420P10LE,
+ AV_PIX_FMT_YUV422P10LE,
+ AV_PIX_FMT_YUV444P10LE,
+ AV_PIX_FMT_NONE
+};
+
+static const AVClass class_libvvdec = {
+ .class_name = "libvvdec-vvc decoder",
+ .item_name = av_default_item_name,
+ .version = LIBAVUTIL_VERSION_INT,
+};
+
+FFCodec ff_libvvdec_decoder = {
+ .p.name = "libvvdec",
+ CODEC_LONG_NAME("H.266 / VVC Decoder VVdeC"),
+ .p.type = AVMEDIA_TYPE_VIDEO,
+ .p.id = AV_CODEC_ID_VVC,
+ .p.capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_OTHER_THREADS,
+ .p.profiles = NULL_IF_CONFIG_SMALL(ff_vvc_profiles),
+ .p.priv_class = &class_libvvdec,
+ .p.wrapper_name = "libvvdec",
+ .priv_data_size = sizeof(VVdeCContext),
+ .p.pix_fmts = pix_fmts_vvdec,
+ .init = ff_vvdec_decode_init,
+ FF_CODEC_DECODE_CB(ff_vvdec_decode_frame),
+ .close = ff_vvdec_decode_close,
+ .flush = ff_vvdec_decode_flush,
+ .bsfs = "vvc_mp4toannexb",
+ .caps_internal = FF_CODEC_CAP_AUTO_THREADS,
+};
--
2.34.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [FFmpeg-devel] [PATCH v3 2/2] avcodec: add external dec libvvdec for H266/VVC
2024-05-14 15:09 ` [FFmpeg-devel] [PATCH v3 2/2] avcodec: add external dec libvvdec " Christian Bartnik
@ 2024-05-14 16:28 ` Lynne via ffmpeg-devel
2024-05-17 10:15 ` Christian
[not found] ` <66AD9495-91D5-40C7-B3FA-CFA222ED3D7E@cosmin.at>
0 siblings, 2 replies; 21+ messages in thread
From: Lynne via ffmpeg-devel @ 2024-05-14 16:28 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Lynne
[-- Attachment #1.1.1.1: Type: text/plain, Size: 770 bytes --]
On 14/05/2024 17:09, Christian Bartnik wrote:
> From: Thomas Siedel <thomas.ff@spin-digital.com>
>
> Add external decoder VVdeC for H266/VVC decoding.
> Register new decoder libvvdec.
> Add libvvdec to wrap the vvdec interface.
> Enable decoder by adding --enable-libvvdec in configure step.
>
> Co-authored-by: Christian Bartnik chris10317h5@gmail.com
> Signed-off-by: Christian Bartnik <chris10317h5@gmail.com>
> ---
> configure | 5 +
> libavcodec/Makefile | 1 +
> libavcodec/allcodecs.c | 1 +
> libavcodec/libvvdec.c | 617 +++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 624 insertions(+)
> create mode 100644 libavcodec/libvvdec.c
I would prefer to have this one skipped, as initially suggested.
[-- Attachment #1.1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 637 bytes --]
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [FFmpeg-devel] [PATCH v3 1/2] avcodec: add external enc libvvenc for H266/VVC
2024-05-14 15:09 ` [FFmpeg-devel] [PATCH v3 1/2] avcodec: add external enc libvvenc for H266/VVC Christian Bartnik
@ 2024-05-14 16:49 ` Andreas Rheinhardt
2024-05-16 15:22 ` Christian
0 siblings, 1 reply; 21+ messages in thread
From: Andreas Rheinhardt @ 2024-05-14 16:49 UTC (permalink / raw)
To: ffmpeg-devel
Christian Bartnik:
> From: Thomas Siedel <thomas.ff@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@gmail.com
> Signed-off-by: Christian Bartnik <chris10317h5@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/}
>
> +@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
> +@option{--enable-libvvenc}.
> +
> +The VVenC project website is at
> +@url{https://github.com/fraunhoferhhi/vvenc}.
> +
> +@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.
> +
> +@subsection Options
> +
> +@table @option
> +@item b
> +Sets target video bitrate.
> +
> +@item g
> +Set the GOP size. Currently support for g=1 (Intra only) or default.
> +
> +@item preset
> +Set the VVenC preset.
> +
> +@item levelidc
> +Set level idc.
> +
> +@item tier
> +Set vvc tier.
> +
> +@item qp
> +Set constant quantization parameter.
> +
> +@item subopt @var{boolean}
> +Set subjective (perceptually motivated) optimization. Default is 1 (on).
> +
> +@item bitdepth8 @var{boolean}
> +Set 8bit coding mode instead of using 10bit. Default is 0 (off).
> +
> +@item period
> +set (intra) refresh period in seconds.
> +
> +@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:
> +
> +@example
> +intraperiod=64:decodingrefreshtype=idr:poc0idr=1:internalbitdepth=8
> +@end example
> +
> +For example the encoding options for 2-pass encoding might be provided with @option{-vvenc-params}:
> +
> +@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
> +@end example
> +
> +@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,
> +};
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [FFmpeg-devel] [PATCH v3 1/2] avcodec: add external enc libvvenc for H266/VVC
2024-05-14 16:49 ` Andreas Rheinhardt
@ 2024-05-16 15:22 ` Christian
2024-05-24 13:03 ` Nuo Mi
0 siblings, 1 reply; 21+ messages in thread
From: Christian @ 2024-05-16 15:22 UTC (permalink / raw)
To: FFmpeg development discussions and patches
> On 14. May 2024, at 18:49, Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote:
>
> Christian Bartnik:
>> From: Thomas Siedel <thomas.ff@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@gmail.com
>> Signed-off-by: Christian Bartnik <chris10317h5@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/}
>>
>> +@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
>> +@option{--enable-libvvenc}.
>> +
>> +The VVenC project website is at
>> +@url{https://github.com/fraunhoferhhi/vvenc}.
>> +
>> +@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.
>> +
>> +@subsection Options
>> +
>> +@table @option
>> +@item b
>> +Sets target video bitrate.
>> +
>> +@item g
>> +Set the GOP size. Currently support for g=1 (Intra only) or default.
>> +
>> +@item preset
>> +Set the VVenC preset.
>> +
>> +@item levelidc
>> +Set level idc.
>> +
>> +@item tier
>> +Set vvc tier.
>> +
>> +@item qp
>> +Set constant quantization parameter.
>> +
>> +@item subopt @var{boolean}
>> +Set subjective (perceptually motivated) optimization. Default is 1 (on).
>> +
>> +@item bitdepth8 @var{boolean}
>> +Set 8bit coding mode instead of using 10bit. Default is 0 (off).
>> +
>> +@item period
>> +set (intra) refresh period in seconds.
>> +
>> +@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:
>> +
>> +@example
>> +intraperiod=64:decodingrefreshtype=idr:poc0idr=1:internalbitdepth=8
>> +@end example
>> +
>> +For example the encoding options for 2-pass encoding might be provided with @option{-vvenc-params}:
>> +
>> +@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
>> +@end example
>> +
>> +@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.
>
Thanks, I will remove it.
>> +
>> +#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.
>
Thanks, I will change the members.
>> +} 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).
>
Thanks. I was not aware of this.
>> +{
>> + 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.
>
Thanks, it´s a type. I´ll fix it.
>> +{
>> + 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.
>
Good point. I´ll change it.
>> + 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.
>
Thanks. I´ll remove it.
>> +
>> + 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?
>
It was just for double check. I change it to AV_PIX_FMT_YUV420P10
and remove the check.
>> + }
>> +
>> + 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.
>
Thanks. I´ll remove it.
>> + ret = 0;
>> +
>> + while ((en = av_dict_get(s->options.vvenc_opts, "", en,
>> + AV_DICT_IGNORE_SUFFIX))) {
>
> av_dict_iterate()
I was using libx264,libx265,libaomenc as reference and aligned to
the existing code.
I will change it to 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()?
Good point. I´ll remove the memcmp.
>
>> + 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?
>
The 2pass approach was quite some of a hack by using the vvenc-
params.
Thanks for the review and hints to improve it.
I will change the code and align to x264 behavior by using the
'passlogfile'/'stats' options and use 2pass flags AV_CODEC_FLAG_PASS1 |
AV_CODEC_FLAG_PASS2 instead of using vvenc-params.
I also will use the default ffmpeg 2pass logfile 'ffmpeg2pass-0.log' as
default as libx264 is using it.
Currently there is no way in vvenc to use an allocated buffer for
write/read statistics.
>> + }
>> + }
>> + 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.
>
I´ll change the 2pass behavior as mentioned above.
>> +
>> + 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?
>
Thanks I will use a integer version for better readability.
>> + 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.
>
Sorry, but I was using libx265 as reference where all these error
messages are implemented in the same way.
Should I just remove the error messages and return an error?
>> + 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().
>
Indeed. I´ll remove it.
>> + }
>> + 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?)
>
VVC has interlaces support as HEVC does. There are no special tools
but only signalizing in high-level syntax.
Currently there is no support in vvenc to signalize fields, so I added
an error.
What would be the best way to do it?
Just throw a warning or using an error and return?
I would change it to:
av_log(avctx, AV_LOG_ERROR, "interlaced encoding not supported\n");
return AVERROR(EINVAL);
>> + }
>> +
>> + 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?
>
The interface call 'vvenc_init_default' is expecting an int
framerate and is setting the the ntsc timing depending on the
framerate internally (e.g. 59 is mapped to 60000/1001).
The correct time base is set later in the init call in function
'vvenc_set_framerate'
..
'params->m_FrameRate = avctx->time_base.den;'
'params->m_FrameScale = avctx->time_base.num;'
But indeed I was aware that AVCodecContext.framerate and
AVCodecContext.time_base are handled differently.
I will change it, thanks.
>> + 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.
>
Thanks.
>> + 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.)
>
Thanks. I will return an error.
I´ll add FF_CODEC_CAP_INIT_CLEANUP and remove all vvenc_encoder_close()
calls.
>> + 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.
>
Thanks for seeing this. It have to be
if (!s->pAU->payload){
I´ll change it.
>> + 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.
Thanks. I´ll move the vvenc_accessUnit_free to the top before
calling vvenc_encoder_close.
>
>> + }
>> + }
>> +
>> + vvenc_accessUnit_free(s->pAU, true);
>
> Can this handle the case in which s->pAU is NULL?
>
Yes, the function vvenc_accessUnit_free will check this internally
as well. However I add a null-check to make it clear.
>> +
>> + 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.
>
Thanks, I will remove the check.
>> + }
>> + }
>> +
>> + 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.
>
Thanks. I´ll change it.
>> + }
>> + } else {
>> + *got_packet = 0;
>
> Unecessary: *got_packet is already zero before we enter this function.
>
good point. I´ll remove it.
>> + 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.
>
Good to know. I´ll remove it.
>> + 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.
>
vvenc currently only returns the picture type as statistics.
I added this code to call all basic ffmpeg routines during an encoding
step.
However a picture type is also a statistic value.
Shall I remove this code block or leave it to add more statistics in
upcoming versions?
>> +
>> + *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.
>
Thanks, I will remove it.
>> + { "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
I know, but 'period' is in seconds for convenience and not in
frames.
In vvenc GOP is a hierarchically group of pictures (size 16 or 32).
The decoding refresh (which you are calling gop) is independent and can
be set by using vvenc-params intraperiod=N, where N=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?
>
There are vvenc-params options. It´s just for convenience. It´s
also possible to use
-vvenc-params qpa=1:internalbitdepth=8
Shall I remove these options?
>> + { "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.
>
Thanks, I will have a look into that.
>> + { "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
It´s not consistent as some codecs are const other not.
e.g. ff_libaom_av1_encoder, ff_libx265_encoder
I´ll fix it.
>
>> + .p.name = "libvvenc",
>> + CODEC_LONG_NAME("H.266 / VVC Encoder VVenC"),
>> + .p.type = AVMEDIA_TYPE_VIDEO,
>> + .p.id <http://p.id/> = AV_CODEC_ID_VVC,
>> + .p.capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_OTHER_THREADS,
>
> You can add AV_CODEC_CAP_DR1
>
Thanks. I´ll add it.
>> + .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,
>> +};
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [FFmpeg-devel] [PATCH v3 2/2] avcodec: add external dec libvvdec for H266/VVC
2024-05-14 16:28 ` Lynne via ffmpeg-devel
@ 2024-05-17 10:15 ` Christian
[not found] ` <66AD9495-91D5-40C7-B3FA-CFA222ED3D7E@cosmin.at>
1 sibling, 0 replies; 21+ messages in thread
From: Christian @ 2024-05-17 10:15 UTC (permalink / raw)
To: FFmpeg development discussions and patches
> On 14. May 2024, at 18:28, Lynne via ffmpeg-devel <ffmpeg-devel@ffmpeg.org> wrote:
>
> On 14/05/2024 17:09, Christian Bartnik wrote:
>> From: Thomas Siedel <thomas.ff@spin-digital.com>
>> Add external decoder VVdeC for H266/VVC decoding.
>> Register new decoder libvvdec.
>> Add libvvdec to wrap the vvdec interface.
>> Enable decoder by adding --enable-libvvdec in configure step.
>> Co-authored-by: Christian Bartnik chris10317h5@gmail.com
>> Signed-off-by: Christian Bartnik <chris10317h5@gmail.com>
>> ---
>> configure | 5 +
>> libavcodec/Makefile | 1 +
>> libavcodec/allcodecs.c | 1 +
>> libavcodec/libvvdec.c | 617 +++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 624 insertions(+)
>> create mode 100644 libavcodec/libvvdec.c
>
> I would prefer to have this one skipped, as initially suggested.
Ok, the next patch version will only contain libvvenc
> <OpenPGP_0xA2FEA5F03F034464.asc>_______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [FFmpeg-devel] [PATCH v3 2/2] avcodec: add external dec libvvdec for H266/VVC
[not found] ` <66AD9495-91D5-40C7-B3FA-CFA222ED3D7E@cosmin.at>
@ 2024-05-17 17:20 ` Cosmin Stejerean via ffmpeg-devel
2024-05-17 19:14 ` Kieran Kunhya
2024-05-18 14:04 ` Nuo Mi
0 siblings, 2 replies; 21+ messages in thread
From: Cosmin Stejerean via ffmpeg-devel @ 2024-05-17 17:20 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Cosmin Stejerean
> On May 14, 2024, at 9:28 AM, Lynne via ffmpeg-devel <ffmpeg-devel@ffmpeg.org> wrote:
>
> On 14/05/2024 17:09, Christian Bartnik wrote:
>> From: Thomas Siedel <thomas.ff@spin-digital.com>
>> Add external decoder VVdeC for H266/VVC decoding.
>> Register new decoder libvvdec.
>> Add libvvdec to wrap the vvdec interface.
>> Enable decoder by adding --enable-libvvdec in configure step.
>> Co-authored-by: Christian Bartnik chris10317h5@gmail.com
>> Signed-off-by: Christian Bartnik <chris10317h5@gmail.com>
>> ---
>> configure | 5 +
>> libavcodec/Makefile | 1 +
>> libavcodec/allcodecs.c | 1 +
>> libavcodec/libvvdec.c | 617 +++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 624 insertions(+)
>> create mode 100644 libavcodec/libvvdec.c
>
> I would prefer to have this one skipped, as initially suggested.
Why? I tried to look back through the list but didn't see anything.
- Cosmin
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [FFmpeg-devel] [PATCH v3 2/2] avcodec: add external dec libvvdec for H266/VVC
2024-05-17 17:20 ` Cosmin Stejerean via ffmpeg-devel
@ 2024-05-17 19:14 ` Kieran Kunhya
2024-05-18 14:04 ` Nuo Mi
1 sibling, 0 replies; 21+ messages in thread
From: Kieran Kunhya @ 2024-05-17 19:14 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Cosmin Stejerean
In this project we prefer internal decoders to external libs.
On Fri, 17 May 2024, 18:20 Cosmin Stejerean via ffmpeg-devel, <
ffmpeg-devel@ffmpeg.org> wrote:
>
>
> > On May 14, 2024, at 9:28 AM, Lynne via ffmpeg-devel <
> ffmpeg-devel@ffmpeg.org> wrote:
> >
> > On 14/05/2024 17:09, Christian Bartnik wrote:
> >> From: Thomas Siedel <thomas.ff@spin-digital.com>
> >> Add external decoder VVdeC for H266/VVC decoding.
> >> Register new decoder libvvdec.
> >> Add libvvdec to wrap the vvdec interface.
> >> Enable decoder by adding --enable-libvvdec in configure step.
> >> Co-authored-by: Christian Bartnik chris10317h5@gmail.com
> >> Signed-off-by: Christian Bartnik <chris10317h5@gmail.com>
> >> ---
> >> configure | 5 +
> >> libavcodec/Makefile | 1 +
> >> libavcodec/allcodecs.c | 1 +
> >> libavcodec/libvvdec.c | 617 +++++++++++++++++++++++++++++++++++++++++
> >> 4 files changed, 624 insertions(+)
> >> create mode 100644 libavcodec/libvvdec.c
> >
> > I would prefer to have this one skipped, as initially suggested.
>
> Why? I tried to look back through the list but didn't see anything.
>
> - Cosmin
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [FFmpeg-devel] [PATCH v3 2/2] avcodec: add external dec libvvdec for H266/VVC
2024-05-17 17:20 ` Cosmin Stejerean via ffmpeg-devel
2024-05-17 19:14 ` Kieran Kunhya
@ 2024-05-18 14:04 ` Nuo Mi
[not found] ` <592133D9-BC0A-4220-B717-66408E9F435B@cosmin.at>
1 sibling, 1 reply; 21+ messages in thread
From: Nuo Mi @ 2024-05-18 14:04 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Sat, May 18, 2024 at 1:20 AM Cosmin Stejerean via ffmpeg-devel <
ffmpeg-devel@ffmpeg.org> wrote:
>
>
> > On May 14, 2024, at 9:28 AM, Lynne via ffmpeg-devel <
> ffmpeg-devel@ffmpeg.org> wrote:
> >
> > On 14/05/2024 17:09, Christian Bartnik wrote:
> >> From: Thomas Siedel <thomas.ff@spin-digital.com>
> >> Add external decoder VVdeC for H266/VVC decoding.
> >> Register new decoder libvvdec.
> >> Add libvvdec to wrap the vvdec interface.
> >> Enable decoder by adding --enable-libvvdec in configure step.
> >> Co-authored-by: Christian Bartnik chris10317h5@gmail.com
> >> Signed-off-by: Christian Bartnik <chris10317h5@gmail.com>
> >> ---
> >> configure | 5 +
> >> libavcodec/Makefile | 1 +
> >> libavcodec/allcodecs.c | 1 +
> >> libavcodec/libvvdec.c | 617 +++++++++++++++++++++++++++++++++++++++++
> >> 4 files changed, 624 insertions(+)
> >> create mode 100644 libavcodec/libvvdec.c
> >
> > I would prefer to have this one skipped, as initially suggested.
>
> Why? I tried to look back through the list but didn't see anything.
>
Hi Cosmin,
This happened many years ago. See the discussion here:
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20201221060710.12230-6-nuomi2021@gmail.com/#60589
Now that we have an internal vvc decoder, we can focus on improving it.
As for the encoder, it is far more complex than the decoder. Reasonable to
wrapper other libraries just like libx264 and libx265...
>
> - Cosmin
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [FFmpeg-devel] [PATCH v3 2/2] avcodec: add external dec libvvdec for H266/VVC
[not found] ` <592133D9-BC0A-4220-B717-66408E9F435B@cosmin.at>
@ 2024-05-18 18:55 ` Cosmin Stejerean via ffmpeg-devel
2024-05-19 5:48 ` Rémi Denis-Courmont
0 siblings, 1 reply; 21+ messages in thread
From: Cosmin Stejerean via ffmpeg-devel @ 2024-05-18 18:55 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Cosmin Stejerean
> On May 18, 2024, at 7:04 AM, Nuo Mi <nuomi2021@gmail.com> wrote:
>
> This happened many years ago. See the discussion here:
> https://patchwork.ffmpeg.org/project/ffmpeg/patch/20201221060710.12230-6-nuomi2021@gmail.com/#60589
> Now that we have an internal vvc decoder, we can focus on improving it.
> As for the encoder, it is far more complex than the decoder. Reasonable to
> wrapper other libraries just like libx264 and libx265...
I'm all for improving the internal decoder, and I agree that the internal decoder should be preferred and be used by default, but it's not clear why that should preclude adding an external decoder as an option.
Sometimes bugs are found in the internal decoders or some functionality may be missing, and having the option to fallback to an external decoder as a workaround is very useful in practice.
For example there's a native AAC decoder in ffmpeg but it's still useful to have FDK-AAC available for decoding when running into edge cases on the native AAC decoder, or to decode USAC while the new decoder is under development, etc.
- Cosmin
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [FFmpeg-devel] [PATCH v3 2/2] avcodec: add external dec libvvdec for H266/VVC
2024-05-18 18:55 ` Cosmin Stejerean via ffmpeg-devel
@ 2024-05-19 5:48 ` Rémi Denis-Courmont
[not found] ` <7C02B7A4-B6C6-477F-A185-0E8D082788B2@cosmin.at>
[not found] ` <8FDC7AD3-E2E8-4E40-9DAE-63C99AE64C49@cosmin.at>
0 siblings, 2 replies; 21+ messages in thread
From: Rémi Denis-Courmont @ 2024-05-19 5:48 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Hi,
Le 18 mai 2024 21:55:04 GMT+03:00, Cosmin Stejerean via ffmpeg-devel <ffmpeg-devel@ffmpeg.org> a écrit :
>
>
>> On May 18, 2024, at 7:04 AM, Nuo Mi <nuomi2021@gmail.com> wrote:
>>
>> This happened many years ago. See the discussion here:
>> https://patchwork.ffmpeg.org/project/ffmpeg/patch/20201221060710.12230-6-nuomi2021@gmail.com/#60589
>> Now that we have an internal vvc decoder, we can focus on improving it.
>> As for the encoder, it is far more complex than the decoder. Reasonable to
>> wrapper other libraries just like libx264 and libx265...
>
>I'm all for improving the internal decoder, and I agree that the internal decoder should be preferred and be used by default, but it's not clear why that should preclude adding an external decoder as an option.
Adding a disabled-by-default decoder is adding bloat, if there is not a specific known reason why that is needed.
This is especially true for video decoders, where the external decoders typically will have no or worse optimisations for DSP functions, no thread support (or not accessible via libavcodec), and no integration with hwaccel.
The later point has become a problem even with dav1d...
>Sometimes bugs are found in the internal decoders
Providing a non-default choice is a poor excuse for a bug. Most users won't know how to do that, and many FFmpeg reverse dependencies don't even allow it.
> or some functionality may be missing, and having the option to fallback to an external decoder as a workaround is very useful in practice.
That I can agree, but what do you find missing in the VVC decoder compared to libvvcdec?
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [FFmpeg-devel] [PATCH v3 2/2] avcodec: add external dec libvvdec for H266/VVC
[not found] ` <7C02B7A4-B6C6-477F-A185-0E8D082788B2@cosmin.at>
@ 2024-05-20 16:24 ` Cosmin Stejerean via ffmpeg-devel
0 siblings, 0 replies; 21+ messages in thread
From: Cosmin Stejerean via ffmpeg-devel @ 2024-05-20 16:24 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Cosmin Stejerean
On May 18, 2024, at 10:48 PM, Rémi Denis-Courmont <remi@remlab.net> wrote:
Le 18 mai 2024 21:55:04 GMT+03:00, Cosmin Stejerean via ffmpeg-devel <ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> > a écrit :
On May 18, 2024, at 7:04 AM, Nuo Mi <nuomi2021@gmail.com <mailto:nuomi2021@gmail.com> > wrote:
This happened many years ago. See the discussion here:
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20201221060710.12230-6-nuomi2021@gmail.com/#60589 <https://patchwork.ffmpeg.org/project/ffmpeg/patch/20201221060710.12230-6-nuomi2021@gmail.com/#60589>
Now that we have an internal vvc decoder, we can focus on improving it.
As for the encoder, it is far more complex than the decoder. Reasonable to
wrapper other libraries just like libx264 and libx265...
I'm all for improving the internal decoder, and I agree that the internal decoder should be preferred and be used by default, but it's not clear why that should preclude adding an external decoder as an option.
Adding a disabled-by-default decoder is adding bloat, if there is not a specific known reason why that is needed.
This is especially true for video decoders, where the external decoders typically will have no or worse optimisations for DSP functions, no thread support (or not accessible via libavcodec),
Is this an actual problem with vvdec? To me it seems like a reasonably optimized decoder with support for threading, etc.
and no integration with hwaccel.
The later point has become a problem even with dav1d...
hwaccel decoding seems somewhat orthogonal
Sometimes bugs are found in the internal decoders
Providing a non-default choice is a poor excuse for a bug. Most users won't know how to do that,
need better documentation?
and many FFmpeg reverse dependencies don't even allow it.
not sure that designing for the lowest common denominator of reverse dependencies is the best way to do things, but at a minimum one can rebuild ffmpeg with only one decoder enabled to get around this
or some functionality may be missing, and having the option to fallback to an external decoder as a workaround is very useful in practice.
That I can agree, but what do you find missing in the VVC decoder compared to libvvcdec?
_______________________________________________
This I'm not sure, perhaps there is no feature gap? Does the internal VVC decoder supports all features of the Main10 profile?
- Cosmin
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [FFmpeg-devel] [PATCH v3 2/2] avcodec: add external dec libvvdec for H266/VVC
[not found] ` <8FDC7AD3-E2E8-4E40-9DAE-63C99AE64C49@cosmin.at>
@ 2024-05-20 16:33 ` Cosmin Stejerean via ffmpeg-devel
2024-05-20 18:03 ` Rémi Denis-Courmont
0 siblings, 1 reply; 21+ messages in thread
From: Cosmin Stejerean via ffmpeg-devel @ 2024-05-20 16:33 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Cosmin Stejerean
Trying again with better formatting, hopefully
> On May 18, 2024, at 10:48 PM, Rémi Denis-Courmont <remi@remlab.net> wrote:
>
> Hi,
>
> Le 18 mai 2024 21:55:04 GMT+03:00, Cosmin Stejerean via ffmpeg-devel <ffmpeg-devel@ffmpeg.org> a écrit :
>>
>>
>>> On May 18, 2024, at 7:04 AM, Nuo Mi <nuomi2021@gmail.com> wrote:
>>>
>>> This happened many years ago. See the discussion here:
>>> https://patchwork.ffmpeg.org/project/ffmpeg/patch/20201221060710.12230-6-nuomi2021@gmail.com/#60589
>>> Now that we have an internal vvc decoder, we can focus on improving it.
>>> As for the encoder, it is far more complex than the decoder. Reasonable to
>>> wrapper other libraries just like libx264 and libx265...
>>
>> I'm all for improving the internal decoder, and I agree that the internal decoder should be preferred and be used by default, but it's not clear why that should preclude adding an external decoder as an option.
>
> Adding a disabled-by-default decoder is adding bloat, if there is not a specific known reason why that is needed.
>
> This is especially true for video decoders, where the external decoders typically will have no or worse optimisations for DSP functions, no thread support (or not accessible via libavcodec), and no integration with hwaccel.
Is this an actual problem with vvdec? To me it seems like a reasonably optimized decoder with support for threading, etc. hwaccel decoding seems somewhat orthogonal
>
> The later point has become a problem even with dav1d...
>
>> Sometimes bugs are found in the internal decoders
>
> Providing a non-default choice is a poor excuse for a bug. Most users won't know how to do that, and many FFmpeg reverse dependencies don't even allow it.
Need better documentation then? Not sure that designing for the lowest common denominator of reverse dependencies is the best way to do things, but at a minimum one can rebuild ffmpeg with only one decoder enabled to get around this.
>
>> or some functionality may be missing, and having the option to fallback to an external decoder as a workaround is very useful in practice.
>
> That I can agree, but what do you find missing in the VVC decoder compared to libvvcdec?
This I'm not sure, perhaps there is no feature gap? Does the internal VVC decoder supports all features of the Main10 profile?
- Cosmin
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [FFmpeg-devel] [PATCH v3 2/2] avcodec: add external dec libvvdec for H266/VVC
2024-05-20 16:33 ` Cosmin Stejerean via ffmpeg-devel
@ 2024-05-20 18:03 ` Rémi Denis-Courmont
[not found] ` <6BF4ADA7-76D0-48C6-9F48-588766A72F85@cosmin.at>
0 siblings, 1 reply; 21+ messages in thread
From: Rémi Denis-Courmont @ 2024-05-20 18:03 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Le maanantaina 20. toukokuuta 2024, 19.33.43 EEST Cosmin Stejerean via ffmpeg-
devel a écrit :
> > Adding a disabled-by-default decoder is adding bloat, if there is not a
> > specific known reason why that is needed.
> > This is especially true for video decoders, where the external decoders
> > typically will have no or worse optimisations for DSP functions, no
> > thread support (or not accessible via libavcodec), and no integration
> > with hwaccel.
>
> Is this an actual problem with vvdec?
Yes?
> To me it seems like a reasonably optimized decoder with support for
> threading, etc.
It seems to be vendoring SIMDe, which is 1) a bad thing according to most
distro policies, 2) not as good as FFmpeg optimisations generally speaking and
3) lacking several ISAs featured by FFmpeg.
> hwaccel decoding seems somewhat orthogonal
How exactly will that work then? Either vvdec is the default, and hwaccel
won't work, or vvdec is not the default and it's essentially dead code.
> > Providing a non-default choice is a poor excuse for a bug. Most users
> > won't know how to do that, and many FFmpeg reverse dependencies don't
> > even allow it.
>
> Need better documentation then?
Well, good luck with that. Documentation for working around as yet unknown
bugs sounds a bit difficult to write, and even then, requiring manual decoder
selection seems very user-hostile to me.
> Not sure that designing for the lowest common denominator of reverse
> dependencies is the best way to do things, but at a minimum one can rebuild
> ffmpeg with only one decoder enabled to get around this.
To the contrary, gstreamer, mpv, VLC et al. are better equipped to handle
this, starting with a proper plugin architecture.
--
レミ・デニ-クールモン
http://www.remlab.net/
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [FFmpeg-devel] [PATCH v3 2/2] avcodec: add external dec libvvdec for H266/VVC
[not found] ` <6BF4ADA7-76D0-48C6-9F48-588766A72F85@cosmin.at>
@ 2024-05-20 18:39 ` Cosmin Stejerean via ffmpeg-devel
2024-05-20 19:01 ` Rémi Denis-Courmont
0 siblings, 1 reply; 21+ messages in thread
From: Cosmin Stejerean via ffmpeg-devel @ 2024-05-20 18:39 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Cosmin Stejerean
> On May 20, 2024, at 11:03 AM, Rémi Denis-Courmont <remi@remlab.net> wrote:
>
> Le maanantaina 20. toukokuuta 2024, 19.33.43 EEST Cosmin Stejerean via ffmpeg-
> devel a écrit :
>
>> hwaccel decoding seems somewhat orthogonal
>
> How exactly will that work then? Either vvdec is the default, and hwaccel
> won't work, or vvdec is not the default and it's essentially dead code.
The same way using FDK-AAC as a decoder works, if you want to use it as the *AAC decoder you have to specify the decoder with -c:a libfdk_aac before the input.-
>
>>> Providing a non-default choice is a poor excuse for a bug. Most users
>>> won't know how to do that, and many FFmpeg reverse dependencies don't
>>> even allow it.
>>
>> Need better documentation then?
>
> Well, good luck with that. Documentation for working around as yet unknown
> bugs sounds a bit difficult to write, and even then, requiring manual decoder
> selection seems very user-hostile to me.
Where is the "requiring" part coming in? I'm saying that manual decoder selection is an option in the ffmpeg CLI, and can be used to select an alternate decoder if the default one is not sufficient for some reason.
This is a thing I've had to do repeatedly over the past few years for FDK-AAC.
>
>> Not sure that designing for the lowest common denominator of reverse
>> dependencies is the best way to do things, but at a minimum one can rebuild
>> ffmpeg with only one decoder enabled to get around this.
>
> To the contrary, gstreamer, mpv, VLC et al. are better equipped to handle
> this, starting with a proper plugin architecture.
Better equipped to handle what? For the usecase of transcoding it doesn't matter whether or not downstream player dependencies do or don't expose alternate decoders in libavcodec, to the extent they even use libavcodec for decoding.
So it seems weird to reject a patch to add a decoder option because it may or may not be useful to the downstream players.
- Cosmin
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [FFmpeg-devel] [PATCH v3 2/2] avcodec: add external dec libvvdec for H266/VVC
2024-05-20 18:39 ` Cosmin Stejerean via ffmpeg-devel
@ 2024-05-20 19:01 ` Rémi Denis-Courmont
[not found] ` <8C6DE791-418E-4464-82E4-9A6D50E4E4D0@cosmin.at>
0 siblings, 1 reply; 21+ messages in thread
From: Rémi Denis-Courmont @ 2024-05-20 19:01 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Le maanantaina 20. toukokuuta 2024, 21.39.18 EEST Cosmin Stejerean via ffmpeg-
devel a écrit :
> The same way using FDK-AAC as a decoder works, if you want to use it as the
> *AAC decoder you have to specify the decoder with -c:a libfdk_aac before
> the input.-
AFAIK, we don't have hwaccel for audio codecs. That sentence makes zero sense.
And again, you can't expect users to select decoders manually. If vvdec is the
default, hwaccel won't work. If vvdec is not the default, then it's dead code.
> Where is the "requiring" part coming in? I'm saying that manual decoder
> selection is an option in the ffmpeg CLI, and can be used to select an
> alternate decoder if the default one is not sufficient for some reason.
So most people use libavcodec through higher-level frameworks or applications,
not the CLI tool, and that is especially true for playback. If that's the
super-niche use-case for vvdec, then I agree with Kieran that it's just bloat.
> > To the contrary, gstreamer, mpv, VLC et al. are better equipped to handle
> > this, starting with a proper plugin architecture.
>
>
> Better equipped to handle what?
To handle multiple implementations of decoding for a given codec, and not
force them as installation and load-time dependencies.
--
レミ・デニ-クールモン
http://www.remlab.net/
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [FFmpeg-devel] [PATCH v3 2/2] avcodec: add external dec libvvdec for H266/VVC
[not found] ` <8C6DE791-418E-4464-82E4-9A6D50E4E4D0@cosmin.at>
@ 2024-05-20 19:33 ` Cosmin Stejerean via ffmpeg-devel
2024-05-20 20:05 ` Rémi Denis-Courmont
0 siblings, 1 reply; 21+ messages in thread
From: Cosmin Stejerean via ffmpeg-devel @ 2024-05-20 19:33 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Cosmin Stejerean
> On May 20, 2024, at 12:01 PM, Rémi Denis-Courmont <remi@remlab.net> wrote:
>
> Le maanantaina 20. toukokuuta 2024, 21.39.18 EEST Cosmin Stejerean via ffmpeg-
> devel a écrit :
>> The same way using FDK-AAC as a decoder works, if you want to use it as the
>> *AAC decoder you have to specify the decoder with -c:a libfdk_aac before
>> the input.-
>
> AFAIK, we don't have hwaccel for audio codecs. That sentence makes zero sense.
This is unrelated to hwaccel, I'm illustrating how a non-default decoder is selected in practice. It just so happens I always need to do this for audio currently.
>
> And again, you can't expect users to select decoders manually. If vvdec is the
> default, hwaccel won't work. If vvdec is not the default, then it's dead code.
Not sure why you keep returning to this false dichotomy.
You absolutely can have a decoder that's not the default, there is a facility for selecting it, and I use this to select the non-default decoder despite you claiming that non-default decoders are dead code, or that manually selecting them isn't a thing that users do.
>> Where is the "requiring" part coming in? I'm saying that manual decoder
>> selection is an option in the ffmpeg CLI, and can be used to select an
>> alternate decoder if the default one is not sufficient for some reason.
>
> So most people use libavcodec through higher-level frameworks or applications,
> not the CLI tool, and that is especially true for playback. If that's the
> super-niche use-case for vvdec, then I agree with Kieran that it's just bloat.
Yes, the use case would be to be an alternate decoder that's available to users that want to use it. If that's not your use case that's ok, don't build with --enable-libvvdec.
- Cosmin
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [FFmpeg-devel] [PATCH v3 2/2] avcodec: add external dec libvvdec for H266/VVC
2024-05-20 19:33 ` Cosmin Stejerean via ffmpeg-devel
@ 2024-05-20 20:05 ` Rémi Denis-Courmont
[not found] ` <2E0614A4-8D72-4100-BE9C-D91B28F8EDB6@cosmin.at>
0 siblings, 1 reply; 21+ messages in thread
From: Rémi Denis-Courmont @ 2024-05-20 20:05 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Le maanantaina 20. toukokuuta 2024, 22.33.45 EEST Cosmin Stejerean via ffmpeg-
devel a écrit :
> > And again, you can't expect users to select decoders manually. If vvdec is
> > the
> > default, hwaccel won't work. If vvdec is not the default, then it's
> > dead code.
>
> Not sure why you keep returning to this false dichotomy.
It is the default or it is not the default. This is a true dichotomy and it is
very disingenous to claim otherwise, so I will leave it at that.
--
雷米‧德尼-库尔蒙
http://www.remlab.net/
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [FFmpeg-devel] [PATCH v3 2/2] avcodec: add external dec libvvdec for H266/VVC
[not found] ` <2E0614A4-8D72-4100-BE9C-D91B28F8EDB6@cosmin.at>
@ 2024-05-20 20:38 ` Cosmin Stejerean via ffmpeg-devel
0 siblings, 0 replies; 21+ messages in thread
From: Cosmin Stejerean via ffmpeg-devel @ 2024-05-20 20:38 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Cosmin Stejerean
> On May 20, 2024, at 1:05 PM, Rémi Denis-Courmont <remi@remlab.net> wrote:
>
> Le maanantaina 20. toukokuuta 2024, 22.33.45 EEST Cosmin Stejerean via ffmpeg-
> devel a écrit :
>>> And again, you can't expect users to select decoders manually. If vvdec is
>>> the
>>> default, hwaccel won't work. If vvdec is not the default, then it's
>>> dead code.
>>
>> Not sure why you keep returning to this false dichotomy.
>
> It is the default or it is not the default. This is a true dichotomy and it is
> very disingenous to claim otherwise, so I will leave it at that.
You stated it's either the default or dead code. This is the false dichotomy I was referring to. Seems disingenuous to claim otherwise.
- Cosmin
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [FFmpeg-devel] [PATCH v3 1/2] avcodec: add external enc libvvenc for H266/VVC
2024-05-16 15:22 ` Christian
@ 2024-05-24 13:03 ` Nuo Mi
0 siblings, 0 replies; 21+ messages in thread
From: Nuo Mi @ 2024-05-24 13:03 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Hi @Andreas,
Thank you for the review.
Seems Christian still needs some guidance from you, see 1, 2, 3
On Thu, May 16, 2024 at 11:22 PM Christian <chris10317h5@gmail.com> wrote:
>
>
> > On 14. May 2024, at 18:49, Andreas Rheinhardt <
> andreas.rheinhardt@outlook.com> wrote:
> >
> > Christian Bartnik:
> >> From: Thomas Siedel <thomas.ff@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@gmail.com
> >> Signed-off-by: Christian Bartnik <chris10317h5@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/}
> >>
> >> +@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
> >> +@option{--enable-libvvenc}.
> >> +
> >> +The VVenC project website is at
> >> +@url{https://github.com/fraunhoferhhi/vvenc}.
> >> +
> >> +@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.
> >> +
> >> +@subsection Options
> >> +
> >> +@table @option
> >> +@item b
> >> +Sets target video bitrate.
> >> +
> >> +@item g
> >> +Set the GOP size. Currently support for g=1 (Intra only) or default.
> >> +
> >> +@item preset
> >> +Set the VVenC preset.
> >> +
> >> +@item levelidc
> >> +Set level idc.
> >> +
> >> +@item tier
> >> +Set vvc tier.
> >> +
> >> +@item qp
> >> +Set constant quantization parameter.
> >> +
> >> +@item subopt @var{boolean}
> >> +Set subjective (perceptually motivated) optimization. Default is 1
> (on).
> >> +
> >> +@item bitdepth8 @var{boolean}
> >> +Set 8bit coding mode instead of using 10bit. Default is 0 (off).
> >> +
> >> +@item period
> >> +set (intra) refresh period in seconds.
> >> +
> >> +@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:
> >> +
> >> +@example
> >> +intraperiod=64:decodingrefreshtype=idr:poc0idr=1:internalbitdepth=8
> >> +@end example
> >> +
> >> +For example the encoding options for 2-pass encoding might be provided
> with @option{-vvenc-params}:
> >> +
> >> +@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
> >> +@end example
> >> +
> >> +@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.
> >
> Thanks, I will remove it.
>
> >> +
> >> +#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.
> >
> Thanks, I will change the members.
>
> >> +} 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).
> >
> Thanks. I was not aware of this.
>
> >> +{
> >> + 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.
> >
> Thanks, it´s a type. I´ll fix it.
>
> >> +{
> >> + 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.
> >
> Good point. I´ll change it.
>
> >> + 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.
> >
> Thanks. I´ll remove it.
>
> >> +
> >> + 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?
> >
> It was just for double check. I change it to AV_PIX_FMT_YUV420P10
> and remove the check.
>
> >> + }
> >> +
> >> + 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.
> >
> Thanks. I´ll remove it.
>
> >> + ret = 0;
> >> +
> >> + while ((en = av_dict_get(s->options.vvenc_opts, "", en,
> >> + AV_DICT_IGNORE_SUFFIX))) {
> >
> > av_dict_iterate()
>
> I was using libx264,libx265,libaomenc as reference and aligned to
> the existing code.
> I will change it to 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()?
>
> Good point. I´ll remove the memcmp.
>
> >
> >> + 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?
> >
> The 2pass approach was quite some of a hack by using the vvenc-
> params.
> Thanks for the review and hints to improve it.
> I will change the code and align to x264 behavior by using the
> 'passlogfile'/'stats' options and use 2pass flags AV_CODEC_FLAG_PASS1 |
> AV_CODEC_FLAG_PASS2 instead of using vvenc-params.
> I also will use the default ffmpeg 2pass logfile 'ffmpeg2pass-0.log' as
> default as libx264 is using it.
> Currently there is no way in vvenc to use an allocated buffer for
> write/read statistics.
>
> >> + }
> >> + }
> >> + 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.
> >
> I´ll change the 2pass behavior as mentioned above.
>
> >> +
> >> + 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?
> >
> Thanks I will use a integer version for better readability.
>
> >> + 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.
> >
> Sorry, but I was using libx265 as reference where all these error
> messages are implemented in the same way.
> Should I just remove the error messages and return an error?
>
1.
>
> >> + 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().
> >
> Indeed. I´ll remove it.
>
> >> + }
> >> + 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?)
> >
> VVC has interlaces support as HEVC does. There are no special tools
> but only signalizing in high-level syntax.
> Currently there is no support in vvenc to signalize fields, so I added
> an error.
> What would be the best way to do it?
> Just throw a warning or using an error and return?
> I would change it to:
> av_log(avctx, AV_LOG_ERROR, "interlaced encoding not supported\n");
> return AVERROR(EINVAL);
>
2.
>
> >> + }
> >> +
> >> + 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?
> >
> The interface call 'vvenc_init_default' is expecting an int
> framerate and is setting the the ntsc timing depending on the
> framerate internally (e.g. 59 is mapped to 60000/1001).
> The correct time base is set later in the init call in function
> 'vvenc_set_framerate'
> ..
> 'params->m_FrameRate = avctx->time_base.den;'
> 'params->m_FrameScale = avctx->time_base.num;'
>
> But indeed I was aware that AVCodecContext.framerate and
> AVCodecContext.time_base are handled differently.
> I will change it, thanks.
>
> >> + 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.
> >
> Thanks.
>
> >> + 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.)
> >
> Thanks. I will return an error.
> I´ll add FF_CODEC_CAP_INIT_CLEANUP and remove all vvenc_encoder_close()
> calls.
>
> >> + 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.
> >
> Thanks for seeing this. It have to be
> if (!s->pAU->payload){
> I´ll change it.
>
> >> + 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.
>
> Thanks. I´ll move the vvenc_accessUnit_free to the top before
> calling vvenc_encoder_close.
>
> >
> >> + }
> >> + }
> >> +
> >> + vvenc_accessUnit_free(s->pAU, true);
> >
> > Can this handle the case in which s->pAU is NULL?
> >
> Yes, the function vvenc_accessUnit_free will check this internally
> as well. However I add a null-check to make it clear.
>
> >> +
> >> + 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.
> >
> Thanks, I will remove the check.
>
> >> + }
> >> + }
> >> +
> >> + 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.
> >
> Thanks. I´ll change it.
>
> >> + }
> >> + } else {
> >> + *got_packet = 0;
> >
> > Unecessary: *got_packet is already zero before we enter this function.
> >
> good point. I´ll remove it.
>
> >> + 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.
> >
> Good to know. I´ll remove it.
>
> >> + 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.
> >
> vvenc currently only returns the picture type as statistics.
> I added this code to call all basic ffmpeg routines during an encoding
> step.
> However a picture type is also a statistic value.
> Shall I remove this code block or leave it to add more statistics in
> upcoming versions?
>
3.
>
> >> +
> >> + *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.
> >
> Thanks, I will remove it.
>
> >> + { "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
>
> I know, but 'period' is in seconds for convenience and not in
> frames.
> In vvenc GOP is a hierarchically group of pictures (size 16 or 32).
> The decoding refresh (which you are calling gop) is independent and can
> be set by using vvenc-params intraperiod=N, where N=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?
> >
> There are vvenc-params options. It´s just for convenience. It´s
> also possible to use
> -vvenc-params qpa=1:internalbitdepth=8
>
> Shall I remove these options?
>
Yes, please remove it. It's an implementation detail that may be removed in
the future. There's no need to expose it explicitly
>
> >> + { "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.
> >
> Thanks, I will have a look into that.
>
> >> + { "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
>
> It´s not consistent as some codecs are const other not.
> e.g. ff_libaom_av1_encoder, ff_libx265_encoder
>
> I´ll fix it.
>
> >
> >> + .p.name = "libvvenc",
> >> + CODEC_LONG_NAME("H.266 / VVC Encoder VVenC"),
> >> + .p.type = AVMEDIA_TYPE_VIDEO,
> >> + .p.id <http://p.id/> = AV_CODEC_ID_VVC,
> >> + .p.capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_OTHER_THREADS,
> >
> > You can add AV_CODEC_CAP_DR1
> >
> Thanks. I´ll add it.
>
> >> + .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,
> >> +};
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org>
> with subject "unsubscribe".
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2024-05-24 13:03 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-14 15:09 [FFmpeg-devel] [PATCH v3 0/2] Add support for H266/VVC encoding Christian Bartnik
2024-05-14 15:09 ` [FFmpeg-devel] [PATCH v3 1/2] avcodec: add external enc libvvenc for H266/VVC Christian Bartnik
2024-05-14 16:49 ` Andreas Rheinhardt
2024-05-16 15:22 ` Christian
2024-05-24 13:03 ` Nuo Mi
2024-05-14 15:09 ` [FFmpeg-devel] [PATCH v3 2/2] avcodec: add external dec libvvdec " Christian Bartnik
2024-05-14 16:28 ` Lynne via ffmpeg-devel
2024-05-17 10:15 ` Christian
[not found] ` <66AD9495-91D5-40C7-B3FA-CFA222ED3D7E@cosmin.at>
2024-05-17 17:20 ` Cosmin Stejerean via ffmpeg-devel
2024-05-17 19:14 ` Kieran Kunhya
2024-05-18 14:04 ` Nuo Mi
[not found] ` <592133D9-BC0A-4220-B717-66408E9F435B@cosmin.at>
2024-05-18 18:55 ` Cosmin Stejerean via ffmpeg-devel
2024-05-19 5:48 ` Rémi Denis-Courmont
[not found] ` <7C02B7A4-B6C6-477F-A185-0E8D082788B2@cosmin.at>
2024-05-20 16:24 ` Cosmin Stejerean via ffmpeg-devel
[not found] ` <8FDC7AD3-E2E8-4E40-9DAE-63C99AE64C49@cosmin.at>
2024-05-20 16:33 ` Cosmin Stejerean via ffmpeg-devel
2024-05-20 18:03 ` Rémi Denis-Courmont
[not found] ` <6BF4ADA7-76D0-48C6-9F48-588766A72F85@cosmin.at>
2024-05-20 18:39 ` Cosmin Stejerean via ffmpeg-devel
2024-05-20 19:01 ` Rémi Denis-Courmont
[not found] ` <8C6DE791-418E-4464-82E4-9A6D50E4E4D0@cosmin.at>
2024-05-20 19:33 ` Cosmin Stejerean via ffmpeg-devel
2024-05-20 20:05 ` Rémi Denis-Courmont
[not found] ` <2E0614A4-8D72-4100-BE9C-D91B28F8EDB6@cosmin.at>
2024-05-20 20:38 ` Cosmin Stejerean via ffmpeg-devel
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git