Message ID | 1533739917-31030-1-git-send-email-zhong.li@intel.com |
---|---|
State | Superseded |
Headers | show |
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@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. > 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); > } > >
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of James Almer > Sent: Wednesday, August 8, 2018 11:08 PM > To: ffmpeg-devel@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@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); } > > > > >
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); q->sum_frame_qp += enc_info->QP; 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); }
Add fix a memory leak issue as James's comments. Signed-off-by: Zhong Li <zhong.li@intel.com> --- libavcodec/qsvenc.c | 18 ++++++++++++++---- libavcodec/qsvenc_h264.c | 5 ----- 2 files changed, 14 insertions(+), 9 deletions(-)