diff mbox

[FFmpeg-devel] Change libvpx-vpx default to crf=32.

Message ID 20190825234331.216291-1-elliottk@google.com
State Superseded
Headers show

Commit Message

elliottk Aug. 25, 2019, 11:43 p.m. UTC 
Current default is 200kbps, which produces inconsistent
results (too high for low-res, too low for hi-res). Use
CRF instead, which will adapt. Affects vp8/vp9. Also
have VP8 use a default bitrate of 256kbps.
---
 libavcodec/libvpxenc.c | 71 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 60 insertions(+), 11 deletions(-)

Comments

Michael Niedermayer Aug. 27, 2019, 3:05 p.m. UTC | #1 
On Sun, Aug 25, 2019 at 04:43:31PM -0700, Elliott Karpilovsky wrote:
> Current default is 200kbps, which produces inconsistent
> results (too high for low-res, too low for hi-res). Use
> CRF instead, which will adapt. Affects vp8/vp9. Also
> have VP8 use a default bitrate of 256kbps.
> ---
>  libavcodec/libvpxenc.c | 71 +++++++++++++++++++++++++++++++++++-------
>  1 file changed, 60 insertions(+), 11 deletions(-)

breaks:
./ffmpeg -i mm.mpg  -vcodec libvpx-vp9 -lossless 1 -vframes 1 -an  vpx-lossless.mkv
...
[libvpx-vp9 @ 0x2ed9080] Neither bitrate nor constrained quality specified, using default CRF of 32
[libvpx-vp9 @ 0x2ed9080] CQ level 32 must be between minimum and maximum quantizer value (0-0)
Error initializing output stream 0:0 -- Error while opening encoder for output stream #0:0 - maybe incorrect parameters such as bit_rate, rate, width or height
Conversion failed!

[...]
elliottk Aug. 28, 2019, 9:09 p.m. UTC | #2 
On Tue, Aug 27, 2019 at 8:05 AM Michael Niedermayer
<michael@niedermayer.cc> wrote:
>
> On Sun, Aug 25, 2019 at 04:43:31PM -0700, Elliott Karpilovsky wrote:
> > Current default is 200kbps, which produces inconsistent
> > results (too high for low-res, too low for hi-res). Use
> > CRF instead, which will adapt. Affects vp8/vp9. Also
> > have VP8 use a default bitrate of 256kbps.
> > ---
> >  libavcodec/libvpxenc.c | 71 +++++++++++++++++++++++++++++++++++-------
> >  1 file changed, 60 insertions(+), 11 deletions(-)
>
> breaks:
> ./ffmpeg -i mm.mpg  -vcodec libvpx-vp9 -lossless 1 -vframes 1 -an  vpx-lossless.mkv
> ...
> [libvpx-vp9 @ 0x2ed9080] Neither bitrate nor constrained quality specified, using default CRF of 32
> [libvpx-vp9 @ 0x2ed9080] CQ level 32 must be between minimum and maximum quantizer value (0-0)
> Error initializing output stream 0:0 -- Error while opening encoder for output stream #0:0 - maybe incorrect parameters such as bit_rate, rate, width or height
> Conversion failed!
>

Thanks for catching this. I will send an updated patch that accounts
for lossless mode. I checked and didn't see any other options that
could cause problems. If I missed any others, please let me know.
diff mbox

Patch

diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
index feb52ea0dd..18f5b742b7 100644
--- a/libavcodec/libvpxenc.c
+++ b/libavcodec/libvpxenc.c
@@ -510,6 +510,63 @@  static void set_color_range(AVCodecContext *avctx)
 #endif
 #endif
 
+/**
+ * Set the target bitrate to VPX library default. Also set CRF to 32 if needed.
+ */
+static void set_vp8_defaults(AVCodecContext *avctx,
+                             struct vpx_codec_enc_cfg *enccfg) {
+  VPxContext *ctx = avctx->priv_data;
+  av_assert0(!avctx->bit_rate);
+  avctx->bit_rate = enccfg->rc_target_bitrate * 1000;
+  if (enccfg->rc_end_usage == VPX_CQ) {
+    av_log(avctx, AV_LOG_WARNING,
+           "Bitrate not specified for constrained quality mode, using default of %dkbit/sec\n",
+           enccfg->rc_target_bitrate);
+  } else {
+    enccfg->rc_end_usage = VPX_CQ;
+    ctx->crf = 32;
+    av_log(avctx, AV_LOG_WARNING,
+           "Neither bitrate nor constrained quality specified, using default CRF of %d and bitrate of %dkbit/sec\n",
+           ctx->crf, enccfg->rc_target_bitrate);
+  }
+}
+
+
+#if CONFIG_LIBVPX_VP9_ENCODER
+/**
+ * Keep the target bitrate at 0 to engage constant quality mode. If CRF is not
+ * set, use 32.
+ */
+static void set_vp9_defaults(AVCodecContext *avctx,
+                             struct vpx_codec_enc_cfg *enccfg) {
+  VPxContext *ctx = avctx->priv_data;
+  av_assert0(!avctx->bit_rate);
+  if (enccfg->rc_end_usage != VPX_Q) {
+    enccfg->rc_end_usage = VPX_Q;
+    ctx->crf = 32;
+    av_log(avctx, AV_LOG_WARNING,
+           "Neither bitrate nor constrained quality specified, using default CRF of %d\n",
+           ctx->crf);
+  }
+}
+#endif
+
+/**
+ * Called when the bitrate is not set. It sets appropriate default values for
+ * bit-rate and CRF.
+ */
+static void set_vpx_defaults(AVCodecContext *avctx,
+                             struct vpx_codec_enc_cfg *enccfg) {
+  av_assert0(!avctx->bit_rate);
+#if CONFIG_LIBVPX_VP9_ENCODER
+  if (avctx->codec_id == AV_CODEC_ID_VP9) {
+    set_vp9_defaults(avctx, enccfg);
+    return;
+  }
+#endif
+  set_vp8_defaults(avctx, enccfg);
+}
+
 static av_cold int vpx_init(AVCodecContext *avctx,
                             const struct vpx_codec_iface *iface)
 {
@@ -580,18 +637,9 @@  static av_cold int vpx_init(AVCodecContext *avctx,
     if (avctx->bit_rate) {
         enccfg.rc_target_bitrate = av_rescale_rnd(avctx->bit_rate, 1, 1000,
                                                   AV_ROUND_NEAR_INF);
-#if CONFIG_LIBVPX_VP9_ENCODER
-    } else if (enccfg.rc_end_usage == VPX_Q) {
-#endif
     } else {
-        if (enccfg.rc_end_usage == VPX_CQ) {
-            enccfg.rc_target_bitrate = 1000000;
-        } else {
-            avctx->bit_rate = enccfg.rc_target_bitrate * 1000;
-            av_log(avctx, AV_LOG_WARNING,
-                   "Neither bitrate nor constrained quality specified, using default bitrate of %dkbit/sec\n",
-                   enccfg.rc_target_bitrate);
-        }
+      // Set bit-rate to default value. Also sets CRF to default if needed.
+      set_vpx_defaults(avctx, &enccfg);
     }
 
     if (avctx->codec_id == AV_CODEC_ID_VP9 && ctx->lossless == 1) {
@@ -1266,6 +1314,7 @@  static const AVOption vp9_options[] = {
 #undef LEGACY_OPTIONS
 
 static const AVCodecDefault defaults[] = {
+    { "b",                 "0" },
     { "qmin",             "-1" },
     { "qmax",             "-1" },
     { "g",                "-1" },