[FFmpeg-devel] libavcodec/libaomenc.c: Added code for computing PSNR/SSIM for libaom encoder.

Submitted by Sam John on Sept. 7, 2018, 11:51 p.m.

Details

Message ID 20180907235112.195643-1-samjohn@google.com
State New
Headers show

Commit Message

Sam John Sept. 7, 2018, 11:51 p.m.
---
 libavcodec/libaomenc.c | 117 +++++++++++++++++++++++++++++++++--------
 1 file changed, 96 insertions(+), 21 deletions(-)

Comments

James Almer Sept. 8, 2018, 7:22 p.m.
On 9/7/2018 8:51 PM, Sam John wrote:
> ---
>  libavcodec/libaomenc.c | 117 +++++++++++++++++++++++++++++++++--------
>  1 file changed, 96 insertions(+), 21 deletions(-)
> 
> diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
> index 9431179886..e62057177d 100644
> --- a/libavcodec/libaomenc.c
> +++ b/libavcodec/libaomenc.c
> @@ -50,6 +50,9 @@ struct FrameListData {
>      unsigned long duration;          /**< duration to show frame
>                                            (in timebase units) */
>      uint32_t flags;                  /**< flags for this frame */
> +    uint64_t sse[4];
> +    int have_sse;                    /**< true if we have pending sse[] */
> +    uint64_t frame_number;
>      struct FrameListData *next;
>  };
>  
> @@ -68,6 +71,9 @@ typedef struct AOMEncoderContext {
>      int static_thresh;
>      int drop_threshold;
>      int noise_sensitivity;
> +    uint64_t sse[4];
> +    int have_sse; /**< true if we have pending sse[] */
> +    uint64_t frame_number;
>  } AOMContext;
>  
>  static const char *const ctlidstr[] = {
> @@ -289,7 +295,8 @@ static av_cold int aom_init(AVCodecContext *avctx,
>  {
>      AOMContext *ctx = avctx->priv_data;
>      struct aom_codec_enc_cfg enccfg = { 0 };
> -    aom_codec_flags_t flags = 0;
> +    aom_codec_flags_t flags =
> +        (avctx->flags & AV_CODEC_FLAG_PSNR) ? AOM_CODEC_USE_PSNR : 0;
>      AVCPBProperties *cpb_props;
>      int res;
>      aom_img_fmt_t img_fmt;
> @@ -499,13 +506,30 @@ static av_cold int aom_init(AVCodecContext *avctx,
>  }
>  
>  static inline void cx_pktcpy(struct FrameListData *dst,
> -                             const struct aom_codec_cx_pkt *src)
> +                             const struct aom_codec_cx_pkt *src,
> +                             AOMContext *ctx)
>  {
>      dst->pts      = src->data.frame.pts;
>      dst->duration = src->data.frame.duration;
>      dst->flags    = src->data.frame.flags;
>      dst->sz       = src->data.frame.sz;
>      dst->buf      = src->data.frame.buf;
> +    dst->have_sse = 0;
> +    /* For alt-ref frame, don't store PSNR or increment frame_number */
> +    if (!(dst->flags & AOM_FRAME_IS_INVISIBLE)) {

Did you copy this chunk from the libvpxenc wrapper? Because I don't
think this is valid at all for libaom. A quick grep on their tree shows
that AOM_FRAME_IS_INVISIBLE is never set anywhere. It looks like a
leftover flag they forgot to remove from the libvpx codebase.

> +        dst->frame_number = ++ctx->frame_number;
> +        dst->have_sse = ctx->have_sse;
> +        if (ctx->have_sse) {
> +            /* associate last-seen SSE to the frame. */
> +            /* Transfers ownership from ctx to dst. */
> +            /* WARNING! This makes the assumption that PSNR_PKT comes
> +               just before the frame it refers to! */
> +            memcpy(dst->sse, ctx->sse, sizeof(dst->sse));
> +            ctx->have_sse = 0;
> +        }
> +    } else {
> +        dst->frame_number = -1;   /* sanity marker */
> +    }
>  }
>  
>  /**
> @@ -524,26 +548,68 @@ static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame,
>          av_log(avctx, AV_LOG_ERROR,
>                 "Error getting output packet of size %"SIZE_SPECIFIER".\n", cx_frame->sz);
>          return ret;
> -    }
> -    memcpy(pkt->data, cx_frame->buf, pkt->size);
> -    pkt->pts = pkt->dts = cx_frame->pts;
> +    } else {
> +        int pict_type;
> +        memcpy(pkt->data, cx_frame->buf, pkt->size);
> +        pkt->pts = pkt->dts = cx_frame->pts;
> +#if FF_API_CODED_FRAME
> +FF_DISABLE_DEPRECATION_WARNINGS
> +        avctx->coded_frame->pts       = cx_frame->pts;
> +        avctx->coded_frame->key_frame = !!(cx_frame->flags & AOM_FRAME_IS_KEY);

coded_frame is deprecated and it's not meant to be used in new modules.
It's left on old ones until removal for backwards compatibility reasons.

> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
> +
> +        if (!!(cx_frame->flags & AOM_FRAME_IS_KEY)) {
> +            pict_type = AV_PICTURE_TYPE_I;
> +#if FF_API_CODED_FRAME
> +FF_DISABLE_DEPRECATION_WARNINGS
> +            avctx->coded_frame->pict_type = pict_type;
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
> +            pkt->flags |= AV_PKT_FLAG_KEY;
> +        } else {
> +            pict_type = AV_PICTURE_TYPE_P;

I'm fairly sure libaom can return I frames that are not key frames. The
INTRA_ONLY type ones.

Does libaom have a flag or field to signal the type of the visible frame
in the returned packet?

> +#if FF_API_CODED_FRAME
> +FF_DISABLE_DEPRECATION_WARNINGS
> +            avctx->coded_frame->pict_type = pict_type;
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
> +        }
>  
> -    if (!!(cx_frame->flags & AOM_FRAME_IS_KEY))
> -        pkt->flags |= AV_PKT_FLAG_KEY;
> +        if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
> +            ret = av_bsf_send_packet(ctx->bsf, pkt);
> +            if (ret < 0) {
> +                av_log(avctx, AV_LOG_ERROR, "extract_extradata filter "
> +                        "failed to send input packet\n");
> +                return ret;
> +            }
> +            ret = av_bsf_receive_packet(ctx->bsf, pkt);
>  
> -    if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
> -        ret = av_bsf_send_packet(ctx->bsf, pkt);
> -        if (ret < 0) {
> -            av_log(avctx, AV_LOG_ERROR, "extract_extradata filter "
> -                   "failed to send input packet\n");
> -            return ret;
> +            if (ret < 0) {
> +                av_log(avctx, AV_LOG_ERROR, "extract_extradata filter "
> +                        "failed to receive output packet\n");
> +                return ret;
> +            }
>          }
> -        ret = av_bsf_receive_packet(ctx->bsf, pkt);
>  
> -        if (ret < 0) {
> -            av_log(avctx, AV_LOG_ERROR, "extract_extradata filter "
> -                   "failed to receive output packet\n");
> -            return ret;

Don't include reindentation when it's a lot of code. It makes the patch
harder to read.

> +        ff_side_data_set_encoder_stats(pkt, 0, cx_frame->sse + 1,
> +                                       cx_frame->have_sse ? 3 : 0, pict_type);
> +
> +        if (cx_frame->have_sse) {
> +            int i;
> +            /* Beware of the Y/U/V/all order! */
> +#if FF_API_CODED_FRAME
> +FF_DISABLE_DEPRECATION_WARNINGS
> +            avctx->coded_frame->error[0] = cx_frame->sse[1];
> +            avctx->coded_frame->error[1] = cx_frame->sse[2];
> +            avctx->coded_frame->error[2] = cx_frame->sse[3];
> +            avctx->coded_frame->error[3] = 0;    // alpha
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
> +            for (i = 0; i < 3; ++i) {
> +                avctx->error[i] += cx_frame->sse[i + 1];
> +            }
> +            cx_frame->have_sse = 0;
>          }
>      }
>      return pkt->size;
> @@ -585,7 +651,7 @@ static int queue_frames(AVCodecContext *avctx, AVPacket *pkt_out)
>                  /* avoid storing the frame when the list is empty and we haven't yet
>                   * provided a frame for output */
>                  av_assert0(!ctx->coded_frame_list);
> -                cx_pktcpy(&cx_frame, pkt);
> +                cx_pktcpy(&cx_frame, pkt, ctx);
>                  size = storeframe(avctx, &cx_frame, pkt_out);
>                  if (size < 0)
>                      return size;
> @@ -598,7 +664,7 @@ static int queue_frames(AVCodecContext *avctx, AVPacket *pkt_out)
>                             "Frame queue element alloc failed\n");
>                      return AVERROR(ENOMEM);
>                  }
> -                cx_pktcpy(cx_frame, pkt);
> +                cx_pktcpy(cx_frame, pkt, ctx);
>                  cx_frame->buf = av_malloc(cx_frame->sz);
>  
>                  if (!cx_frame->buf) {
> @@ -628,7 +694,16 @@ static int queue_frames(AVCodecContext *avctx, AVPacket *pkt_out)
>              stats->sz += pkt->data.twopass_stats.sz;
>              break;
>          }
> -        case AOM_CODEC_PSNR_PKT: // FIXME add support for AV_CODEC_FLAG_PSNR
> +        case AOM_CODEC_PSNR_PKT:
> +        {
> +            av_assert0(!ctx->have_sse);
> +            ctx->sse[0] = pkt->data.psnr.sse[0];
> +            ctx->sse[1] = pkt->data.psnr.sse[1];
> +            ctx->sse[2] = pkt->data.psnr.sse[2];
> +            ctx->sse[3] = pkt->data.psnr.sse[3];
> +            ctx->have_sse = 1;
> +            break;
> +        }
>          case AOM_CODEC_CUSTOM_PKT:
>              // ignore unsupported/unrecognized packet types
>              break;
>
Sam John Sept. 14, 2018, 8:53 p.m.
James,

> Did you copy this chunk from the libvpxenc wrapper? Because I don't
>think this is valid at all for libaom. A quick grep on their tree shows
>that AOM_FRAME_IS_INVISIBLE is never set anywhere. It looks like a
>leftover flag they forgot to remove from the libvpx codebase

I used the same logic as libvpx. The flag AOM_FRAME_IS_INVISIBLE is not set
at present. But I hope that this flag will be updated soon. I will update
the code to remove this flag from my patch for now.

> Does libaom have a flag or field to signal the type of the visible frame
> in the returned packet?
At present libaom doesn't set the flag for the INTRA_ONLY in the returned
packet. Until this flag is updated, we can use the same logic as libvpx.

I will make these corrections and update the patch for review.






On Sat, Sep 8, 2018 at 12:23 PM James Almer <jamrial@gmail.com> wrote:

> On 9/7/2018 8:51 PM, Sam John wrote:
> > ---
> >  libavcodec/libaomenc.c | 117 +++++++++++++++++++++++++++++++++--------
> >  1 file changed, 96 insertions(+), 21 deletions(-)
> >
> > diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
> > index 9431179886..e62057177d 100644
> > --- a/libavcodec/libaomenc.c
> > +++ b/libavcodec/libaomenc.c
> > @@ -50,6 +50,9 @@ struct FrameListData {
> >      unsigned long duration;          /**< duration to show frame
> >                                            (in timebase units) */
> >      uint32_t flags;                  /**< flags for this frame */
> > +    uint64_t sse[4];
> > +    int have_sse;                    /**< true if we have pending sse[]
> */
> > +    uint64_t frame_number;
> >      struct FrameListData *next;
> >  };
> >
> > @@ -68,6 +71,9 @@ typedef struct AOMEncoderContext {
> >      int static_thresh;
> >      int drop_threshold;
> >      int noise_sensitivity;
> > +    uint64_t sse[4];
> > +    int have_sse; /**< true if we have pending sse[] */
> > +    uint64_t frame_number;
> >  } AOMContext;
> >
> >  static const char *const ctlidstr[] = {
> > @@ -289,7 +295,8 @@ static av_cold int aom_init(AVCodecContext *avctx,
> >  {
> >      AOMContext *ctx = avctx->priv_data;
> >      struct aom_codec_enc_cfg enccfg = { 0 };
> > -    aom_codec_flags_t flags = 0;
> > +    aom_codec_flags_t flags =
> > +        (avctx->flags & AV_CODEC_FLAG_PSNR) ? AOM_CODEC_USE_PSNR : 0;
> >      AVCPBProperties *cpb_props;
> >      int res;
> >      aom_img_fmt_t img_fmt;
> > @@ -499,13 +506,30 @@ static av_cold int aom_init(AVCodecContext *avctx,
> >  }
> >
> >  static inline void cx_pktcpy(struct FrameListData *dst,
> > -                             const struct aom_codec_cx_pkt *src)
> > +                             const struct aom_codec_cx_pkt *src,
> > +                             AOMContext *ctx)
> >  {
> >      dst->pts      = src->data.frame.pts;
> >      dst->duration = src->data.frame.duration;
> >      dst->flags    = src->data.frame.flags;
> >      dst->sz       = src->data.frame.sz;
> >      dst->buf      = src->data.frame.buf;
> > +    dst->have_sse = 0;
> > +    /* For alt-ref frame, don't store PSNR or increment frame_number */
> > +    if (!(dst->flags & AOM_FRAME_IS_INVISIBLE)) {
>
> Did you copy this chunk from the libvpxenc wrapper? Because I don't
> think this is valid at all for libaom. A quick grep on their tree shows
> that AOM_FRAME_IS_INVISIBLE is never set anywhere. It looks like a
> leftover flag they forgot to remove from the libvpx codebase.
>
> > +        dst->frame_number = ++ctx->frame_number;
> > +        dst->have_sse = ctx->have_sse;
> > +        if (ctx->have_sse) {
> > +            /* associate last-seen SSE to the frame. */
> > +            /* Transfers ownership from ctx to dst. */
> > +            /* WARNING! This makes the assumption that PSNR_PKT comes
> > +               just before the frame it refers to! */
> > +            memcpy(dst->sse, ctx->sse, sizeof(dst->sse));
> > +            ctx->have_sse = 0;
> > +        }
> > +    } else {
> > +        dst->frame_number = -1;   /* sanity marker */
> > +    }
> >  }
> >
> >  /**
> > @@ -524,26 +548,68 @@ static int storeframe(AVCodecContext *avctx,
> struct FrameListData *cx_frame,
> >          av_log(avctx, AV_LOG_ERROR,
> >                 "Error getting output packet of size
> %"SIZE_SPECIFIER".\n", cx_frame->sz);
> >          return ret;
> > -    }
> > -    memcpy(pkt->data, cx_frame->buf, pkt->size);
> > -    pkt->pts = pkt->dts = cx_frame->pts;
> > +    } else {
> > +        int pict_type;
> > +        memcpy(pkt->data, cx_frame->buf, pkt->size);
> > +        pkt->pts = pkt->dts = cx_frame->pts;
> > +#if FF_API_CODED_FRAME
> > +FF_DISABLE_DEPRECATION_WARNINGS
> > +        avctx->coded_frame->pts       = cx_frame->pts;
> > +        avctx->coded_frame->key_frame = !!(cx_frame->flags &
> AOM_FRAME_IS_KEY);
>
> coded_frame is deprecated and it's not meant to be used in new modules.
> It's left on old ones until removal for backwards compatibility reasons.
>
> > +FF_ENABLE_DEPRECATION_WARNINGS
> > +#endif
> > +
> > +        if (!!(cx_frame->flags & AOM_FRAME_IS_KEY)) {
> > +            pict_type = AV_PICTURE_TYPE_I;
> > +#if FF_API_CODED_FRAME
> > +FF_DISABLE_DEPRECATION_WARNINGS
> > +            avctx->coded_frame->pict_type = pict_type;
> > +FF_ENABLE_DEPRECATION_WARNINGS
> > +#endif
> > +            pkt->flags |= AV_PKT_FLAG_KEY;
> > +        } else {
> > +            pict_type = AV_PICTURE_TYPE_P;
>
> I'm fairly sure libaom can return I frames that are not key frames. The
> INTRA_ONLY type ones.
>
> Does libaom have a flag or field to signal the type of the visible frame
> in the returned packet?
>
> > +#if FF_API_CODED_FRAME
> > +FF_DISABLE_DEPRECATION_WARNINGS
> > +            avctx->coded_frame->pict_type = pict_type;
> > +FF_ENABLE_DEPRECATION_WARNINGS
> > +#endif
> > +        }
> >
> > -    if (!!(cx_frame->flags & AOM_FRAME_IS_KEY))
> > -        pkt->flags |= AV_PKT_FLAG_KEY;
> > +        if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
> > +            ret = av_bsf_send_packet(ctx->bsf, pkt);
> > +            if (ret < 0) {
> > +                av_log(avctx, AV_LOG_ERROR, "extract_extradata filter "
> > +                        "failed to send input packet\n");
> > +                return ret;
> > +            }
> > +            ret = av_bsf_receive_packet(ctx->bsf, pkt);
> >
> > -    if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
> > -        ret = av_bsf_send_packet(ctx->bsf, pkt);
> > -        if (ret < 0) {
> > -            av_log(avctx, AV_LOG_ERROR, "extract_extradata filter "
> > -                   "failed to send input packet\n");
> > -            return ret;
> > +            if (ret < 0) {
> > +                av_log(avctx, AV_LOG_ERROR, "extract_extradata filter "
> > +                        "failed to receive output packet\n");
> > +                return ret;
> > +            }
> >          }
> > -        ret = av_bsf_receive_packet(ctx->bsf, pkt);
> >
> > -        if (ret < 0) {
> > -            av_log(avctx, AV_LOG_ERROR, "extract_extradata filter "
> > -                   "failed to receive output packet\n");
> > -            return ret;
>
> Don't include reindentation when it's a lot of code. It makes the patch
> harder to read.
>
> > +        ff_side_data_set_encoder_stats(pkt, 0, cx_frame->sse + 1,
> > +                                       cx_frame->have_sse ? 3 : 0,
> pict_type);
> > +
> > +        if (cx_frame->have_sse) {
> > +            int i;
> > +            /* Beware of the Y/U/V/all order! */
> > +#if FF_API_CODED_FRAME
> > +FF_DISABLE_DEPRECATION_WARNINGS
> > +            avctx->coded_frame->error[0] = cx_frame->sse[1];
> > +            avctx->coded_frame->error[1] = cx_frame->sse[2];
> > +            avctx->coded_frame->error[2] = cx_frame->sse[3];
> > +            avctx->coded_frame->error[3] = 0;    // alpha
> > +FF_ENABLE_DEPRECATION_WARNINGS
> > +#endif
> > +            for (i = 0; i < 3; ++i) {
> > +                avctx->error[i] += cx_frame->sse[i + 1];
> > +            }
> > +            cx_frame->have_sse = 0;
> >          }
> >      }
> >      return pkt->size;
> > @@ -585,7 +651,7 @@ static int queue_frames(AVCodecContext *avctx,
> AVPacket *pkt_out)
> >                  /* avoid storing the frame when the list is empty and
> we haven't yet
> >                   * provided a frame for output */
> >                  av_assert0(!ctx->coded_frame_list);
> > -                cx_pktcpy(&cx_frame, pkt);
> > +                cx_pktcpy(&cx_frame, pkt, ctx);
> >                  size = storeframe(avctx, &cx_frame, pkt_out);
> >                  if (size < 0)
> >                      return size;
> > @@ -598,7 +664,7 @@ static int queue_frames(AVCodecContext *avctx,
> AVPacket *pkt_out)
> >                             "Frame queue element alloc failed\n");
> >                      return AVERROR(ENOMEM);
> >                  }
> > -                cx_pktcpy(cx_frame, pkt);
> > +                cx_pktcpy(cx_frame, pkt, ctx);
> >                  cx_frame->buf = av_malloc(cx_frame->sz);
> >
> >                  if (!cx_frame->buf) {
> > @@ -628,7 +694,16 @@ static int queue_frames(AVCodecContext *avctx,
> AVPacket *pkt_out)
> >              stats->sz += pkt->data.twopass_stats.sz;
> >              break;
> >          }
> > -        case AOM_CODEC_PSNR_PKT: // FIXME add support for
> AV_CODEC_FLAG_PSNR
> > +        case AOM_CODEC_PSNR_PKT:
> > +        {
> > +            av_assert0(!ctx->have_sse);
> > +            ctx->sse[0] = pkt->data.psnr.sse[0];
> > +            ctx->sse[1] = pkt->data.psnr.sse[1];
> > +            ctx->sse[2] = pkt->data.psnr.sse[2];
> > +            ctx->sse[3] = pkt->data.psnr.sse[3];
> > +            ctx->have_sse = 1;
> > +            break;
> > +        }
> >          case AOM_CODEC_CUSTOM_PKT:
> >              // ignore unsupported/unrecognized packet types
> >              break;
> >
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
James Almer Sept. 14, 2018, 10:10 p.m.
On 9/14/2018 5:53 PM, Sam John wrote:
> James,
> 
>> Did you copy this chunk from the libvpxenc wrapper? Because I don't
>> think this is valid at all for libaom. A quick grep on their tree shows
>> that AOM_FRAME_IS_INVISIBLE is never set anywhere. It looks like a
>> leftover flag they forgot to remove from the libvpx codebase
> 
> I used the same logic as libvpx. The flag AOM_FRAME_IS_INVISIBLE is not set
> at present. But I hope that this flag will be updated soon. I will update
> the code to remove this flag from my patch for now.

I filed a bug in the libaom bug tracker to request the removal of this
flag. Afaik it's not even used for VP9 in the libvpx codebase, only VP8.
aom_codec_get_cx_data() can only return a complete TU, which must have a
visible frame, so i don't think it's appropriate for AV1.

https://bugs.chromium.org/p/aomedia/issues/detail?id=2157

> 
>> Does libaom have a flag or field to signal the type of the visible frame
>> in the returned packet?
> At present libaom doesn't set the flag for the INTRA_ONLY in the returned
> packet. Until this flag is updated, we can use the same logic as libvpx.

Given its name, AOM_FRAME_IS_KEY should only be set if frame_type ==
KEY_FRAME, and not for any other kind of frame type.
Knowing that the visible frame is not of type KEY_FRAME alone does not
guarantee that it's an inter frame, so setting pict_type to
AV_PICTURE_TYPE_P in that case is just not correct (I know INTRA_ONLY
frames are rare, but we shouldn't risk reporting an intra frame as inter).

A new AOM_FRAME_IS_INTRA flag set to the derived value "FrameIsIntra"
defined in the Uncompressed Header from the spec, or a new "type" field
in aom_codec_cx_pkt_t.data.frame would probably be enough to solve this.

> 
> I will make these corrections and update the patch for review.
>

Patch hide | download patch | download mbox

diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
index 9431179886..e62057177d 100644
--- a/libavcodec/libaomenc.c
+++ b/libavcodec/libaomenc.c
@@ -50,6 +50,9 @@  struct FrameListData {
     unsigned long duration;          /**< duration to show frame
                                           (in timebase units) */
     uint32_t flags;                  /**< flags for this frame */
+    uint64_t sse[4];
+    int have_sse;                    /**< true if we have pending sse[] */
+    uint64_t frame_number;
     struct FrameListData *next;
 };
 
@@ -68,6 +71,9 @@  typedef struct AOMEncoderContext {
     int static_thresh;
     int drop_threshold;
     int noise_sensitivity;
+    uint64_t sse[4];
+    int have_sse; /**< true if we have pending sse[] */
+    uint64_t frame_number;
 } AOMContext;
 
 static const char *const ctlidstr[] = {
@@ -289,7 +295,8 @@  static av_cold int aom_init(AVCodecContext *avctx,
 {
     AOMContext *ctx = avctx->priv_data;
     struct aom_codec_enc_cfg enccfg = { 0 };
-    aom_codec_flags_t flags = 0;
+    aom_codec_flags_t flags =
+        (avctx->flags & AV_CODEC_FLAG_PSNR) ? AOM_CODEC_USE_PSNR : 0;
     AVCPBProperties *cpb_props;
     int res;
     aom_img_fmt_t img_fmt;
@@ -499,13 +506,30 @@  static av_cold int aom_init(AVCodecContext *avctx,
 }
 
 static inline void cx_pktcpy(struct FrameListData *dst,
-                             const struct aom_codec_cx_pkt *src)
+                             const struct aom_codec_cx_pkt *src,
+                             AOMContext *ctx)
 {
     dst->pts      = src->data.frame.pts;
     dst->duration = src->data.frame.duration;
     dst->flags    = src->data.frame.flags;
     dst->sz       = src->data.frame.sz;
     dst->buf      = src->data.frame.buf;
+    dst->have_sse = 0;
+    /* For alt-ref frame, don't store PSNR or increment frame_number */
+    if (!(dst->flags & AOM_FRAME_IS_INVISIBLE)) {
+        dst->frame_number = ++ctx->frame_number;
+        dst->have_sse = ctx->have_sse;
+        if (ctx->have_sse) {
+            /* associate last-seen SSE to the frame. */
+            /* Transfers ownership from ctx to dst. */
+            /* WARNING! This makes the assumption that PSNR_PKT comes
+               just before the frame it refers to! */
+            memcpy(dst->sse, ctx->sse, sizeof(dst->sse));
+            ctx->have_sse = 0;
+        }
+    } else {
+        dst->frame_number = -1;   /* sanity marker */
+    }
 }
 
 /**
@@ -524,26 +548,68 @@  static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame,
         av_log(avctx, AV_LOG_ERROR,
                "Error getting output packet of size %"SIZE_SPECIFIER".\n", cx_frame->sz);
         return ret;
-    }
-    memcpy(pkt->data, cx_frame->buf, pkt->size);
-    pkt->pts = pkt->dts = cx_frame->pts;
+    } else {
+        int pict_type;
+        memcpy(pkt->data, cx_frame->buf, pkt->size);
+        pkt->pts = pkt->dts = cx_frame->pts;
+#if FF_API_CODED_FRAME
+FF_DISABLE_DEPRECATION_WARNINGS
+        avctx->coded_frame->pts       = cx_frame->pts;
+        avctx->coded_frame->key_frame = !!(cx_frame->flags & AOM_FRAME_IS_KEY);
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
+
+        if (!!(cx_frame->flags & AOM_FRAME_IS_KEY)) {
+            pict_type = AV_PICTURE_TYPE_I;
+#if FF_API_CODED_FRAME
+FF_DISABLE_DEPRECATION_WARNINGS
+            avctx->coded_frame->pict_type = pict_type;
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
+            pkt->flags |= AV_PKT_FLAG_KEY;
+        } else {
+            pict_type = AV_PICTURE_TYPE_P;
+#if FF_API_CODED_FRAME
+FF_DISABLE_DEPRECATION_WARNINGS
+            avctx->coded_frame->pict_type = pict_type;
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
+        }
 
-    if (!!(cx_frame->flags & AOM_FRAME_IS_KEY))
-        pkt->flags |= AV_PKT_FLAG_KEY;
+        if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
+            ret = av_bsf_send_packet(ctx->bsf, pkt);
+            if (ret < 0) {
+                av_log(avctx, AV_LOG_ERROR, "extract_extradata filter "
+                        "failed to send input packet\n");
+                return ret;
+            }
+            ret = av_bsf_receive_packet(ctx->bsf, pkt);
 
-    if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
-        ret = av_bsf_send_packet(ctx->bsf, pkt);
-        if (ret < 0) {
-            av_log(avctx, AV_LOG_ERROR, "extract_extradata filter "
-                   "failed to send input packet\n");
-            return ret;
+            if (ret < 0) {
+                av_log(avctx, AV_LOG_ERROR, "extract_extradata filter "
+                        "failed to receive output packet\n");
+                return ret;
+            }
         }
-        ret = av_bsf_receive_packet(ctx->bsf, pkt);
 
-        if (ret < 0) {
-            av_log(avctx, AV_LOG_ERROR, "extract_extradata filter "
-                   "failed to receive output packet\n");
-            return ret;
+        ff_side_data_set_encoder_stats(pkt, 0, cx_frame->sse + 1,
+                                       cx_frame->have_sse ? 3 : 0, pict_type);
+
+        if (cx_frame->have_sse) {
+            int i;
+            /* Beware of the Y/U/V/all order! */
+#if FF_API_CODED_FRAME
+FF_DISABLE_DEPRECATION_WARNINGS
+            avctx->coded_frame->error[0] = cx_frame->sse[1];
+            avctx->coded_frame->error[1] = cx_frame->sse[2];
+            avctx->coded_frame->error[2] = cx_frame->sse[3];
+            avctx->coded_frame->error[3] = 0;    // alpha
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
+            for (i = 0; i < 3; ++i) {
+                avctx->error[i] += cx_frame->sse[i + 1];
+            }
+            cx_frame->have_sse = 0;
         }
     }
     return pkt->size;
@@ -585,7 +651,7 @@  static int queue_frames(AVCodecContext *avctx, AVPacket *pkt_out)
                 /* avoid storing the frame when the list is empty and we haven't yet
                  * provided a frame for output */
                 av_assert0(!ctx->coded_frame_list);
-                cx_pktcpy(&cx_frame, pkt);
+                cx_pktcpy(&cx_frame, pkt, ctx);
                 size = storeframe(avctx, &cx_frame, pkt_out);
                 if (size < 0)
                     return size;
@@ -598,7 +664,7 @@  static int queue_frames(AVCodecContext *avctx, AVPacket *pkt_out)
                            "Frame queue element alloc failed\n");
                     return AVERROR(ENOMEM);
                 }
-                cx_pktcpy(cx_frame, pkt);
+                cx_pktcpy(cx_frame, pkt, ctx);
                 cx_frame->buf = av_malloc(cx_frame->sz);
 
                 if (!cx_frame->buf) {
@@ -628,7 +694,16 @@  static int queue_frames(AVCodecContext *avctx, AVPacket *pkt_out)
             stats->sz += pkt->data.twopass_stats.sz;
             break;
         }
-        case AOM_CODEC_PSNR_PKT: // FIXME add support for AV_CODEC_FLAG_PSNR
+        case AOM_CODEC_PSNR_PKT:
+        {
+            av_assert0(!ctx->have_sse);
+            ctx->sse[0] = pkt->data.psnr.sse[0];
+            ctx->sse[1] = pkt->data.psnr.sse[1];
+            ctx->sse[2] = pkt->data.psnr.sse[2];
+            ctx->sse[3] = pkt->data.psnr.sse[3];
+            ctx->have_sse = 1;
+            break;
+        }
         case AOM_CODEC_CUSTOM_PKT:
             // ignore unsupported/unrecognized packet types
             break;