[FFmpeg-devel] [PATCH] lavc/qsvenc: add quality status to side_data

[Li, Zhong]

> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
> Of James Almer
> Sent: Wednesday, August 8, 2018 11:08 PM
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] lavc/qsvenc: add quality status to
> side_data
> 
> On 8/8/2018 11:51 AM, Zhong Li wrote:
> > Add fix a memory leak issue as James's comments.
> >
> > Signed-off-by: Zhong Li <zhong.li at intel.com>
> > ---
> >  libavcodec/qsvenc.c      | 18 ++++++++++++++----
> >  libavcodec/qsvenc_h264.c |  5 -----
> >  2 files changed, 14 insertions(+), 9 deletions(-)
> >
> > diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c index
> > 65dae31..2107c5b 100644
> > --- a/libavcodec/qsvenc.c
> > +++ b/libavcodec/qsvenc.c
> > @@ -1200,8 +1200,10 @@ static int encode_frame(AVCodecContext
> *avctx, QSVEncContext *q,
> >      if (!sync) {
> >          av_freep(&bs);
> >   #if QSV_VERSION_ATLEAST(1, 26)
> > -        if (avctx->codec_id == AV_CODEC_ID_H264)
> > +        if (avctx->codec_id == AV_CODEC_ID_H264) {
> >              av_freep(&enc_info);
> > +            av_freep(&enc_buf);
> > +        }
> >   #endif
> >          av_packet_unref(&new_pkt);
> >          return AVERROR(ENOMEM);
> > @@ -1220,8 +1222,10 @@ static int encode_frame(AVCodecContext
> *avctx, QSVEncContext *q,
> >          av_packet_unref(&new_pkt);
> >          av_freep(&bs);
> >  #if QSV_VERSION_ATLEAST(1, 26)
> > -        if (avctx->codec_id == AV_CODEC_ID_H264)
> > +        if (avctx->codec_id == AV_CODEC_ID_H264) {
> >              av_freep(&enc_info);
> > +            av_freep(&enc_buf);
> > +        }
> >  #endif
> >          av_freep(&sync);
> >          return (ret == MFX_ERR_MORE_DATA) ?
> > @@ -1240,8 +1244,10 @@ static int encode_frame(AVCodecContext
> *avctx, QSVEncContext *q,
> >          av_packet_unref(&new_pkt);
> >          av_freep(&bs);
> >  #if QSV_VERSION_ATLEAST(1, 26)
> > -        if (avctx->codec_id == AV_CODEC_ID_H264)
> > +        if (avctx->codec_id == AV_CODEC_ID_H264) {
> >              av_freep(&enc_info);
> > +            av_freep(&enc_buf);
> > +        }
> >  #endif
> >      }
> >
> > @@ -1264,6 +1270,7 @@ int ff_qsv_encode(AVCodecContext *avctx,
> QSVEncContext *q,
> >          mfxSyncPoint *sync;
> >  #if QSV_VERSION_ATLEAST(1, 26)
> >          mfxExtAVCEncodedFrameInfo *enc_info;
> > +        mfxExtBuffer **enc_buf;
> >  #endif
> >
> >          av_fifo_generic_read(q->async_fifo, &new_pkt,
> > sizeof(new_pkt), NULL); @@ -1295,10 +1302,13 @@
> > FF_ENABLE_DEPRECATION_WARNINGS
> >
> >  #if QSV_VERSION_ATLEAST(1, 26)
> >          if (avctx->codec_id == AV_CODEC_ID_H264) {
> > +            enc_buf = bs->ExtParam;
> >              enc_info = (mfxExtAVCEncodedFrameInfo
> *)(*bs->ExtParam);
> > -            av_log(avctx, AV_LOG_DEBUG, "QP is %d\n",
> enc_info->QP);
> > +            ff_side_data_set_encoder_stats(&new_pkt,
> > +                enc_info->QP * FF_QP2LAMBDA, NULL, 0,
> > + avctx->coded_frame->pict_type);
> 
> This will generate a deprecated warning and will need to be changed once
> coded_frame is removed.
> 
> Add a local pict_type variable instead and set it using the existing checks.
> Then use it here and to set avctx->coded_frame->pict_type.

Thanks for your suggestion. It has been updated. 

> 
> >              q->sum_frame_qp += enc_info->QP;
> 
> This is unused now that you removed the log message below.
> 
> >              av_freep(&enc_info);
> > +            av_freep(&enc_buf);
> >          }
> >  #endif
> >          av_freep(&bs);
> > diff --git a/libavcodec/qsvenc_h264.c b/libavcodec/qsvenc_h264.c index
> > b87bef6..5c262e5 100644
> > --- a/libavcodec/qsvenc_h264.c
> > +++ b/libavcodec/qsvenc_h264.c
> > @@ -95,11 +95,6 @@ static av_cold int qsv_enc_close(AVCodecContext
> > *avctx)  {
> >      QSVH264EncContext *q = avctx->priv_data;
> >
> > -#if QSV_VERSION_ATLEAST(1, 26)
> > -    av_log(avctx, AV_LOG_VERBOSE, "encoded %d frames, avarge qp
> is %.2f\n",
> > -        avctx->frame_number,(double)q->qsv.sum_frame_qp /
> avctx->frame_number);
> > -#endif
> > -
> >      return ff_qsv_enc_close(avctx, &q->qsv);  }
> >
> >
>