| Message ID | 20240208215649.3090001-2-darekm@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v3] lavc/libvpxenc: add screen-content-mode option | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/configure_loongarch64 | warning | Failed to apply patch |
| andriy/configure_x86 | warning | Failed to apply patch |
On Thu, Feb 8, 2024 at 1:58 PM Dariusz Marcinkiewicz via ffmpeg-devel <ffmpeg-devel@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@webrtc.org> > Signed-off-by: Dariusz Marcinkiewicz <darekm@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. > > +@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.
Hello. On Fri, Feb 9, 2024 at 7:28 PM James Zern <jzern@google.com> wrote: > Just sent v4, which addresses the below comments. Thank you. > On Thu, Feb 8, 2024 at 1:58 PM Dariusz Marcinkiewicz via ffmpeg-devel > <ffmpeg-devel@ffmpeg.org> wrote: ... > > --- 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. > Done. ... > > @@ -164,6 +165,7 @@ static const char *const ctlidstr[] = { ... > > +#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. > Done. ... > > + 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. > Done. > > [...] > > > > - 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. > Dropped this part. ... > > [...] > > @@ -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. > Removed. ... > > 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. Done.
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. +@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; 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 + if (avctx->codec_id == AV_CODEC_ID_VP8 && ctx->screen_content_mode >= 0) { + if (ctx->screen_content_mode == 2 && ctx->is_alpha) { + av_log(avctx, AV_LOG_ERROR, + "Transparency encoding with screen mode with aggressive rate control not supported\n"); + return AVERROR(EINVAL); + } + codecctl_int(avctx, VP8E_SET_SCREEN_CONTENT_MODE, ctx->screen_content_mode); + } +#endif av_log(avctx, AV_LOG_DEBUG, "Using deadline: %d\n", ctx->deadline); @@ -1379,14 +1391,15 @@ static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame, * @return AVERROR(ENOMEM) on coded frame queue data allocation error */ static int queue_frames(AVCodecContext *avctx, struct vpx_codec_ctx *encoder, - struct FrameListData **frame_list, AVPacket *pkt_out) + struct FrameListData **frame_list, AVPacket *pkt_out, + int queue_only, int *frame_enc) { VPxContext *ctx = avctx->priv_data; const struct vpx_codec_cx_pkt *pkt; const void *iter = NULL; int size = 0; - if (!ctx->is_alpha && *frame_list) { + if (!queue_only && *frame_list) { struct FrameListData *cx_frame = *frame_list; /* return the leading frame if we've already begun queueing */ size = storeframe(avctx, cx_frame, NULL, pkt_out); @@ -1401,7 +1414,7 @@ static int queue_frames(AVCodecContext *avctx, struct vpx_codec_ctx *encoder, while (pkt = vpx_codec_get_cx_data(encoder, &iter)) { switch (pkt->kind) { case VPX_CODEC_CX_FRAME_PKT: - if (!ctx->is_alpha && !size) { + if (!queue_only && !size) { struct FrameListData cx_frame; /* avoid storing the frame when the list is empty and we haven't yet @@ -1411,6 +1424,8 @@ static int queue_frames(AVCodecContext *avctx, struct vpx_codec_ctx *encoder, size = storeframe(avctx, &cx_frame, NULL, pkt_out); if (size < 0) return size; + if (size > 0) + *frame_enc = 1; } else { struct FrameListData *cx_frame = av_malloc(sizeof(*cx_frame)); @@ -1430,6 +1445,8 @@ static int queue_frames(AVCodecContext *avctx, struct vpx_codec_ctx *encoder, return AVERROR(ENOMEM); } memcpy(cx_frame->buf, pkt->data.frame.buf, pkt->data.frame.sz); + if (pkt->data.frame.sz > 0) + *frame_enc = 1; coded_frame_add(frame_list, cx_frame); } break; @@ -1693,6 +1710,7 @@ static int vpx_encode(AVCodecContext *avctx, AVPacket *pkt, vpx_svc_layer_id_t layer_id; int layer_id_valid = 0; unsigned long duration = 0; + int frame_enc = 0; if (avctx->qmax >= 0 && enccfg->rc_max_quantizer != avctx->qmax) { struct vpx_codec_enc_cfg cfg = *enccfg; @@ -1856,9 +1874,32 @@ FF_ENABLE_DEPRECATION_WARNINGS } } - 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 + // OK status code but produced no output, this indicates frame was + // rolled back due to bitrate overshoot - try to encode it again. + av_log(avctx, AV_LOG_VERBOSE, + "Attempting to reencode dropped frame.\n"); + res = vpx_codec_encode(&ctx->encoder, rawimg, timestamp, + duration, flags, ctx->deadline); + if (res != VPX_CODEC_OK) { + log_encoder_error(avctx, "Error encoding frame"); + return AVERROR_INVALIDDATA; + } + if (!coded_size) + coded_size = queue_frames(avctx, &ctx->encoder, &ctx->coded_frame_list, + pkt, /*queue_only*/0, &frame_enc); + else + queue_frames(avctx, &ctx->encoder, &ctx->coded_frame_list, pkt, + /*queue_only*/1, &frame_enc); + } + if (ctx->is_alpha) { - queue_frames(avctx, &ctx->encoder_alpha, &ctx->alpha_coded_frame_list, NULL); + queue_frames(avctx, &ctx->encoder_alpha, &ctx->alpha_coded_frame_list, + NULL, /*queue_only*/1, &frame_enc); if (ctx->coded_frame_list && ctx->alpha_coded_frame_list) { struct FrameListData *cx_frame = ctx->coded_frame_list; @@ -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"}, +#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 #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \ LIBAVCODEC_VERSION_MINOR, \
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@webrtc.org> Signed-off-by: Dariusz Marcinkiewicz <darekm@google.com> --- doc/encoders.texi | 3 +++ libavcodec/libvpxenc.c | 57 ++++++++++++++++++++++++++++++++++++++---- libavcodec/version.h | 2 +- 3 files changed, 56 insertions(+), 6 deletions(-)