[FFmpeg-devel] libvpxenc: extend auto-alt-ref range

Submitted by James Zern on Nov. 3, 2018, 9:02 p.m.

Details

Message ID 20181103210243.124306-1-jzern@google.com
State New
Headers show

Commit Message

James Zern Nov. 3, 2018, 9:02 p.m.
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(-)

Comments

James Zern Nov. 3, 2018, 9:04 p.m.
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
James Almer Nov. 3, 2018, 9:24 p.m.
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
>
James Zern Nov. 6, 2018, 12:45 a.m.
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
James Zern Nov. 6, 2018, 12:57 a.m.
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
James Zern Nov. 6, 2018, 1:03 a.m.
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
>
James Zern Nov. 8, 2018, 10:49 p.m.
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
> >

Patch hide | download patch | download mbox

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}, \