[FFmpeg-devel] [PATCH v3] lavc/libvpxenc: add screen-content-mode option

[James Zern]

On Thu, Feb 8, 2024 at 1:58 PM Dariusz Marcinkiewicz via ffmpeg-devel
<ffmpeg-devel at ffmpeg.org> wrote:
>
> This exposes VP8E_SET_SCREEN_CONTENT_MODE option from libvpx and makes
> us retry encode if screen_content_mode == 2 and no output was produced
> by encoder.
>
> Co-authored-by: Erik Språng <sprang at webrtc.org>
> Signed-off-by: Dariusz Marcinkiewicz <darekm at google.com>
> ---
>  doc/encoders.texi      |  3 +++
>  libavcodec/libvpxenc.c | 57 ++++++++++++++++++++++++++++++++++++++----
>  libavcodec/version.h   |  2 +-
>  3 files changed, 56 insertions(+), 6 deletions(-)
>
> diff --git a/doc/encoders.texi b/doc/encoders.texi
> index c9fe6d6143..0868aa66db 100644
> --- a/doc/encoders.texi
> +++ b/doc/encoders.texi
> @@ -2150,6 +2150,9 @@ of quality.
>  Set a change threshold on blocks below which they will be skipped by the
>  encoder.
>
> + at item screen-content-mode
> +Screen content mode, one of: off (0), screen (1), screen with more aggressive rate control (2).
> +
>  @item slices (@emph{token-parts})
>  Note that FFmpeg's @option{slices} option gives the total number of partitions,
>  while @command{vpxenc}'s @option{token-parts} is given as
> diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
> index 80988a2608..7571a8577a 100644
> --- a/libavcodec/libvpxenc.c
> +++ b/libavcodec/libvpxenc.c
> @@ -114,6 +114,7 @@ typedef struct VPxEncoderContext {
>      int crf;
>      int static_thresh;
>      int max_intra_rate;
> +    int screen_content_mode;

Move this to a VP8-only section similar to VP9.

>      int rc_undershoot_pct;
>      int rc_overshoot_pct;
>
> @@ -164,6 +165,7 @@ static const char *const ctlidstr[] = {
>      [VP8E_SET_MAX_INTRA_BITRATE_PCT] = "VP8E_SET_MAX_INTRA_BITRATE_PCT",
>      [VP8E_SET_SHARPNESS]               = "VP8E_SET_SHARPNESS",
>      [VP8E_SET_TEMPORAL_LAYER_ID]       = "VP8E_SET_TEMPORAL_LAYER_ID",
> +    [VP8E_SET_SCREEN_CONTENT_MODE]     = "VP8E_SET_SCREEN_CONTENT_MODE",
>  #if CONFIG_LIBVPX_VP9_ENCODER
>      [VP9E_SET_LOSSLESS]                = "VP9E_SET_LOSSLESS",
>      [VP9E_SET_TILE_COLUMNS]            = "VP9E_SET_TILE_COLUMNS",
> @@ -1262,6 +1264,16 @@ static av_cold int vpx_init(AVCodecContext *avctx,
>  #endif
>      }
>  #endif
> +#ifdef VPX_CTRL_VP8E_SET_SCREEN_CONTENT_MODE

This control was available in libvpx 1.4.0, the minimum version
supported. You can remove this check.

> +    if (avctx->codec_id == AV_CODEC_ID_VP8 && ctx->screen_content_mode >= 0) {
> +      if (ctx->screen_content_mode == 2 && ctx->is_alpha) {

Indent is 4 spaces here and throughout the patch.

> [...]
>
> -    coded_size = queue_frames(avctx, &ctx->encoder, &ctx->coded_frame_list, pkt);
> +    coded_size = queue_frames(avctx, &ctx->encoder, &ctx->coded_frame_list,
> +                              pkt, ctx->is_alpha, &frame_enc);
> +    if (avctx->codec_id == AV_CODEC_ID_VP8 && frame_enc == 0 &&
> +        ctx->screen_content_mode == 2 && frame) {
> +        // VP8 tuned for screen content with aggresive rate control - returned

aggressive.

> +        // OK status code but produced no output, this indicates frame was
> +        // rolled back due to bitrate overshoot - try to encode it again.

This is a little weird given there's no adjustment to the encoder. I
think this should be a separate patch at least. If the encoder decided
to drop the frame in this mode it seems like the right decision given
the setting description. If it works as part of the drop frames
threshold then maybe the recode should be in the library based on some
threshold.

> [...]
> @@ -1946,6 +1987,12 @@ static const AVOption vp8_options[] = {
>      { "auto-alt-ref",    "Enable use of alternate reference "
>                           "frames (2-pass only)",                        OFFSET(auto_alt_ref),    AV_OPT_TYPE_INT, {.i64 = -1}, -1,  2, VE},
>      { "cpu-used",        "Quality/Speed ratio modifier",                OFFSET(cpu_used),        AV_OPT_TYPE_INT, {.i64 = 1}, -16, 16, VE},
> +#ifdef VPX_CTRL_VP8E_SET_SCREEN_CONTENT_MODE
> +    { "screen-content-mode",     "Encoder screen content mode",         OFFSET(screen_content_mode), AV_OPT_TYPE_INT, {.i64 = -1}, -1,  2, VE, "screen_content_mode"},
> +    { "off",          "Screen content mode off",                        0,                      AV_OPT_TYPE_CONST, {.i64 = 0}, 0,  0, VE, "screen_content_mode"},
> +    { "on",           "Screen content mode on",                         0,                      AV_OPT_TYPE_CONST, {.i64 = 1}, 0,  0, VE, "screen_content_mode"},
> +    { "on-agressive-rate-control", "Screen content mode on with aggressive rate control", 0,    AV_OPT_TYPE_CONST, {.i64 = 2}, 0,  0, VE, "screen_content_mode"},

aggressive.
There's no string equivalent in vpxenc though, so this should probably
just be ints.

> +#endif
>      LEGACY_OPTIONS
>      { NULL }
>  };
> diff --git a/libavcodec/version.h b/libavcodec/version.h
> index 0fae3d06d3..4b618d740f 100644
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -30,7 +30,7 @@
>  #include "version_major.h"
>
>  #define LIBAVCODEC_VERSION_MINOR  38
> -#define LIBAVCODEC_VERSION_MICRO 100
> +#define LIBAVCODEC_VERSION_MICRO 101
>

This will need a rebase.