[libav-devel] [PATCH 3/3] Support AV1 encoding using libaom

Diego Biurrun diego at biurrun.de
Mon Mar 12 11:27:13 CET 2018


On Thu, Mar 08, 2018 at 09:50:40PM +0100, Luca Barbato wrote:
> ---
>  configure              |   1 +
>  libavcodec/Makefile    |   1 +
>  libavcodec/allcodecs.c |   2 +-
>  libavcodec/avcodec.h   |   4 +
>  libavcodec/libaomenc.c | 584 +++++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 591 insertions(+), 1 deletion(-)
>  create mode 100644 libavcodec/libaomenc.c

missing docs update and changelog entry

> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -2551,6 +2551,10 @@ typedef struct AVCodecContext {
>  #define FF_PROFILE_HEVC_MAIN_STILL_PICTURE          3
>  #define FF_PROFILE_HEVC_REXT                        4
>  
> +#define FF_PROFILE_AV1_0                            0
> +#define FF_PROFILE_AV1_1                            1
> +#define FF_PROFILE_AV1_2                            2

I think it's a mistake to add more FF_-prefixed stuff into avcodec.h
and I don't see why these defines have to be in a public header. They
are only used in libaomenc.c, I don't see a reason to place them
outside of that file.

> --- /dev/null
> +++ b/libavcodec/libaomenc.c
> @@ -0,0 +1,584 @@
> +static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame,
> +                      AVPacket *pkt)
> +{
> +    int ret = ff_alloc_packet(pkt, cx_frame->sz);
> +    if (ret >= 0) {
> +        memcpy(pkt->data, cx_frame->buf, pkt->size);
> +        pkt->pts = pkt->dts = cx_frame->pts;
> +
> +        if (!!(cx_frame->flags & AOM_FRAME_IS_KEY)) {
> +            pkt->flags |= AV_PKT_FLAG_KEY;
> +        }
> +    } else {
> +        av_log(avctx, AV_LOG_ERROR,
> +               "Error getting output packet of size %zu.\n", cx_frame->sz);
> +        return ret;
> +    }
> +    return pkt->size;
> +}

simpler logic:


  int ret = ff_alloc_packet(pkt, cx_frame->sz);
  if (ret < 0) {
      av_log(avctx, AV_LOG_ERROR,
             "Error getting output packet of size %zu.\n", cx_frame->sz);
      return ret;
  }
  memcpy(pkt->data, cx_frame->buf, pkt->size);
  pkt->pts = pkt->dts = cx_frame->pts;

  if (!!(cx_frame->flags & AOM_FRAME_IS_KEY))
      pkt->flags |= AV_PKT_FLAG_KEY;
  return pkt->size;

> +static const AVOption options[] = {
> +    { "cpu-used",        "Quality/Speed ratio modifier",           OFFSET(cpu_used),        AV_OPT_TYPE_INT, {.i64 = 1}, INT_MIN, INT_MAX, VE},
> +    { "auto-alt-ref",    "Enable use of alternate reference "
> +                         "frames (2-pass only)",                   OFFSET(auto_alt_ref),    AV_OPT_TYPE_INT, {.i64 = -1},      -1,      1,       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},

to look ahead at for

> +    { "error-resilient", "Error resilience configuration", OFFSET(error_resilient), AV_OPT_TYPE_FLAGS, {.i64 = 0}, INT_MIN, INT_MAX, VE, "er"},

I think this should be named error-resilience.

nit: spaces inside {} inside the whole block

Diego


More information about the libav-devel mailing list