| Message ID | 20181103210243.124306-1-jzern@google.com |
|---|---|
| State | Accepted |
| Headers | show |
On Sat, Nov 3, 2018 at 2:02 PM James Zern <jzern@google.com> wrote: > [...] > #define COMMON_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}, \ > + "frames (2-pass only)", OFFSET(auto_alt_ref), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE}, \ This was extended earlier [1], but probably should have been made codec specific. I can do that, but we might want to keep the 0-2 range for vp8 now for compatibility. Any preferences? [1] 41da4f8cb3 lavc/libvpxenc: fix -auto-alt-ref option type
On 11/3/2018 6:04 PM, James Zern wrote: > On Sat, Nov 3, 2018 at 2:02 PM James Zern <jzern@google.com> wrote: >> [...] >> #define COMMON_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}, \ >> + "frames (2-pass only)", OFFSET(auto_alt_ref), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE}, \ > > This was extended earlier [1], but probably should have been made > codec specific. I can do that, but we might want to keep the 0-2 range > for vp8 now for compatibility. Any preferences? What happens when you use the higher values while encoding VP8? Does it error out or silently clips it to 1 internally? If it errors out then yes, move it out of COMMON_OPTIONS and into each encoder AVOption array. > > [1] 41da4f8cb3 lavc/libvpxenc: fix -auto-alt-ref option type > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On Sat, Nov 3, 2018 at 2:31 PM James Almer <jamrial@gmail.com> wrote: > > On 11/3/2018 6:04 PM, James Zern wrote: > > On Sat, Nov 3, 2018 at 2:02 PM James Zern <jzern@google.com> wrote: > >> [...] > >> #define COMMON_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}, \ > >> + "frames (2-pass only)", OFFSET(auto_alt_ref), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE}, \ > > > > This was extended earlier [1], but probably should have been made > > codec specific. I can do that, but we might want to keep the 0-2 range > > for vp8 now for compatibility. Any preferences? > > What happens when you use the higher values while encoding VP8? Does it > error out or silently clips it to 1 internally? > > If it errors out then yes, move it out of COMMON_OPTIONS and into each > encoder AVOption array. > VP8 is clipped in this file currently because the library will error out with values > 1: codecctl_int(avctx, VP8E_SET_ENABLEAUTOALTREF, avctx->codec_id == AV_CODEC_ID_VP8 ? !!ctx->auto_alt_ref : ctx->auto_alt_ref); So moving it from COMMON_OPTIONS at this point will avoid any more drift in the reported range of '-h encoder=libvpx'. > > > > [1] 41da4f8cb3 lavc/libvpxenc: fix -auto-alt-ref option type > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On Mon, Nov 5, 2018 at 4:45 PM James Zern <jzern@google.com> wrote: > > On Sat, Nov 3, 2018 at 2:31 PM James Almer <jamrial@gmail.com> wrote: > > > > On 11/3/2018 6:04 PM, James Zern wrote: > > > On Sat, Nov 3, 2018 at 2:02 PM James Zern <jzern@google.com> wrote: > > >> [...] > > >> #define COMMON_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}, \ > > >> + "frames (2-pass only)", OFFSET(auto_alt_ref), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE}, \ > > > > > > This was extended earlier [1], but probably should have been made > > > codec specific. I can do that, but we might want to keep the 0-2 range > > > for vp8 now for compatibility. Any preferences? > > > > What happens when you use the higher values while encoding VP8? Does it > > error out or silently clips it to 1 internally? > > > > If it errors out then yes, move it out of COMMON_OPTIONS and into each > > encoder AVOption array. > > > > VP8 is clipped in this file currently because the library will error > out with values > 1: More precisely the value will fail to be set and the error will be logged, but encoding will continue and you'll get the library default. > codecctl_int(avctx, VP8E_SET_ENABLEAUTOALTREF, > avctx->codec_id == AV_CODEC_ID_VP8 ? > !!ctx->auto_alt_ref : ctx->auto_alt_ref); > > So moving it from COMMON_OPTIONS at this point will avoid any more > drift in the reported range of '-h encoder=libvpx'. > > > > > > > [1] 41da4f8cb3 lavc/libvpxenc: fix -auto-alt-ref option type > > > _______________________________________________ > > > ffmpeg-devel mailing list > > > ffmpeg-devel@ffmpeg.org > > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On Mon, Nov 5, 2018 at 5:02 PM James Zern <jzern@google.com> wrote: > > vp9 now supports [0, 6] > > Signed-off-by: James Zern <jzern@google.com> > --- > doc/encoders.texi | 1 + > libavcodec/libvpxenc.c | 6 ++++-- > 2 files changed, 5 insertions(+), 2 deletions(-) > This version moves the option from COMMON_OPTIONS for comparison. > diff --git a/doc/encoders.texi b/doc/encoders.texi > index 899faac49b..3c6f5cd70b 100644 > --- a/doc/encoders.texi > +++ b/doc/encoders.texi > @@ -1641,6 +1641,7 @@ means unlimited. > @table @option > @item auto-alt-ref > Enable use of alternate reference frames (2-pass only). > +Values greater than 1 enable multi-layer alternate reference frames (VP9 only). > @item arnr-max-frames > Set altref noise reduction max frame count. > @item arnr-type > diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c > index 09f7a88452..827df21fa5 100644 > --- a/libavcodec/libvpxenc.c > +++ b/libavcodec/libvpxenc.c > @@ -1067,8 +1067,6 @@ static int vpx_encode(AVCodecContext *avctx, AVPacket *pkt, > #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM > > #define COMMON_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}, \ > { "lag-in-frames", "Number of frames to look ahead for " \ > "alternate reference frame selection", OFFSET(lag_in_frames), AV_OPT_TYPE_INT, {.i64 = -1}, -1, INT_MAX, VE}, \ > { "arnr-maxframes", "altref noise reduction max frame count", OFFSET(arnr_max_frames), AV_OPT_TYPE_INT, {.i64 = -1}, -1, INT_MAX, VE}, \ > @@ -1112,6 +1110,8 @@ static int vpx_encode(AVCodecContext *avctx, AVPacket *pkt, > #if CONFIG_LIBVPX_VP8_ENCODER > static const AVOption vp8_options[] = { > COMMON_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}, > LEGACY_OPTIONS > { NULL } > @@ -1121,6 +1121,8 @@ static const AVOption vp8_options[] = { > #if CONFIG_LIBVPX_VP9_ENCODER > static const AVOption vp9_options[] = { > COMMON_OPTIONS > + { "auto-alt-ref", "Enable use of alternate reference " > + "frames (2-pass only)", OFFSET(auto_alt_ref), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE}, > { "cpu-used", "Quality/Speed ratio modifier", OFFSET(cpu_used), AV_OPT_TYPE_INT, {.i64 = 1}, -8, 8, VE}, > { "lossless", "Lossless mode", OFFSET(lossless), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 1, VE}, > { "tile-columns", "Number of tile columns to use, log2", OFFSET(tile_columns), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE}, > -- > 2.19.1.930.g4563a0d9d0-goog >
On Mon, Nov 5, 2018 at 5:03 PM James Zern <jzern@google.com> wrote: > > On Mon, Nov 5, 2018 at 5:02 PM James Zern <jzern@google.com> wrote: > > > > vp9 now supports [0, 6] > > > > Signed-off-by: James Zern <jzern@google.com> > > --- > > doc/encoders.texi | 1 + > > libavcodec/libvpxenc.c | 6 ++++-- > > 2 files changed, 5 insertions(+), 2 deletions(-) > > > > This version moves the option from COMMON_OPTIONS for comparison. > I'll submit this version soon if there aren't any comments. vp8 could be made a bool in a followup at the cost of compatibility, it's been in its current state for a couple years. > > diff --git a/doc/encoders.texi b/doc/encoders.texi > > index 899faac49b..3c6f5cd70b 100644 > > --- a/doc/encoders.texi > > +++ b/doc/encoders.texi > > @@ -1641,6 +1641,7 @@ means unlimited. > > @table @option > > @item auto-alt-ref > > Enable use of alternate reference frames (2-pass only). > > +Values greater than 1 enable multi-layer alternate reference frames (VP9 only). > > @item arnr-max-frames > > Set altref noise reduction max frame count. > > @item arnr-type > > diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c > > index 09f7a88452..827df21fa5 100644 > > --- a/libavcodec/libvpxenc.c > > +++ b/libavcodec/libvpxenc.c > > @@ -1067,8 +1067,6 @@ static int vpx_encode(AVCodecContext *avctx, AVPacket *pkt, > > #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM > > > > #define COMMON_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}, \ > > { "lag-in-frames", "Number of frames to look ahead for " \ > > "alternate reference frame selection", OFFSET(lag_in_frames), AV_OPT_TYPE_INT, {.i64 = -1}, -1, INT_MAX, VE}, \ > > { "arnr-maxframes", "altref noise reduction max frame count", OFFSET(arnr_max_frames), AV_OPT_TYPE_INT, {.i64 = -1}, -1, INT_MAX, VE}, \ > > @@ -1112,6 +1110,8 @@ static int vpx_encode(AVCodecContext *avctx, AVPacket *pkt, > > #if CONFIG_LIBVPX_VP8_ENCODER > > static const AVOption vp8_options[] = { > > COMMON_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}, > > LEGACY_OPTIONS > > { NULL } > > @@ -1121,6 +1121,8 @@ static const AVOption vp8_options[] = { > > #if CONFIG_LIBVPX_VP9_ENCODER > > static const AVOption vp9_options[] = { > > COMMON_OPTIONS > > + { "auto-alt-ref", "Enable use of alternate reference " > > + "frames (2-pass only)", OFFSET(auto_alt_ref), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE}, > > { "cpu-used", "Quality/Speed ratio modifier", OFFSET(cpu_used), AV_OPT_TYPE_INT, {.i64 = 1}, -8, 8, VE}, > > { "lossless", "Lossless mode", OFFSET(lossless), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 1, VE}, > > { "tile-columns", "Number of tile columns to use, log2", OFFSET(tile_columns), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE}, > > -- > > 2.19.1.930.g4563a0d9d0-goog > >
diff --git a/doc/encoders.texi b/doc/encoders.texi index 899faac49b..3c6f5cd70b 100644 --- a/doc/encoders.texi +++ b/doc/encoders.texi @@ -1641,6 +1641,7 @@ means unlimited. @table @option @item auto-alt-ref Enable use of alternate reference frames (2-pass only). +Values greater than 1 enable multi-layer alternate reference frames (VP9 only). @item arnr-max-frames Set altref noise reduction max frame count. @item arnr-type diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c index 09f7a88452..ad4480b2ae 100644 --- a/libavcodec/libvpxenc.c +++ b/libavcodec/libvpxenc.c @@ -1068,7 +1068,7 @@ static int vpx_encode(AVCodecContext *avctx, AVPacket *pkt, #define COMMON_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}, \ + "frames (2-pass only)", OFFSET(auto_alt_ref), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE}, \ { "lag-in-frames", "Number of frames to look ahead for " \ "alternate reference frame selection", OFFSET(lag_in_frames), AV_OPT_TYPE_INT, {.i64 = -1}, -1, INT_MAX, VE}, \ { "arnr-maxframes", "altref noise reduction max frame count", OFFSET(arnr_max_frames), AV_OPT_TYPE_INT, {.i64 = -1}, -1, INT_MAX, VE}, \
vp9 now supports [0, 6] Signed-off-by: James Zern <jzern@google.com> --- doc/encoders.texi | 1 + libavcodec/libvpxenc.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-)