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 <jamr...@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 > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel