| Message ID | 1533888634-9533-1-git-send-email-zhong.li@intel.com |
|---|---|
| State | New |
| Headers | show |
On 8/10/2018 5:10 AM, Zhong Li wrote: > Add fix a memory leak issue as James's comments. > > V2: use a local pict_type since coded_frame is deprecated. > > Signed-off-by: Zhong Li <zhong.li@intel.com> > --- > libavcodec/qsvenc.c | 32 ++++++++++++++++++++++---------- > libavcodec/qsvenc.h | 2 -- > libavcodec/qsvenc_h264.c | 5 ----- > 3 files changed, 22 insertions(+), 17 deletions(-) > > diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c > index 65dae31..0a14e6a 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,7 +1270,9 @@ int ff_qsv_encode(AVCodecContext *avctx, QSVEncContext *q, > mfxSyncPoint *sync; > #if QSV_VERSION_ATLEAST(1, 26) > mfxExtAVCEncodedFrameInfo *enc_info; > + mfxExtBuffer **enc_buf; > #endif > + enum AVPictureType pict_type; > > av_fifo_generic_read(q->async_fifo, &new_pkt, sizeof(new_pkt), NULL); > av_fifo_generic_read(q->async_fifo, &sync, sizeof(sync), NULL); > @@ -1282,23 +1290,27 @@ int ff_qsv_encode(AVCodecContext *avctx, QSVEncContext *q, > bs->FrameType & MFX_FRAMETYPE_xIDR) > new_pkt.flags |= AV_PKT_FLAG_KEY; > > -#if FF_API_CODED_FRAME > -FF_DISABLE_DEPRECATION_WARNINGS > if (bs->FrameType & MFX_FRAMETYPE_I || bs->FrameType & MFX_FRAMETYPE_xI) > - avctx->coded_frame->pict_type = AV_PICTURE_TYPE_I; > + pict_type = AV_PICTURE_TYPE_I; > else if (bs->FrameType & MFX_FRAMETYPE_P || bs->FrameType & MFX_FRAMETYPE_xP) > - avctx->coded_frame->pict_type = AV_PICTURE_TYPE_P; > + pict_type = AV_PICTURE_TYPE_P; > else if (bs->FrameType & MFX_FRAMETYPE_B || bs->FrameType & MFX_FRAMETYPE_xB) > - avctx->coded_frame->pict_type = AV_PICTURE_TYPE_B; > + pict_type = AV_PICTURE_TYPE_B; > + > +#if FF_API_CODED_FRAME > +FF_DISABLE_DEPRECATION_WARNINGS > + avctx->coded_frame->pict_type = pict_type; > FF_ENABLE_DEPRECATION_WARNINGS > #endif > > #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); > - q->sum_frame_qp += enc_info->QP; > + ff_side_data_set_encoder_stats(&new_pkt, > + enc_info->QP * FF_QP2LAMBDA, NULL, 0, pict_type); > av_freep(&enc_info); > + av_freep(&enc_buf); > } > #endif > av_freep(&bs); > diff --git a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h > index 3784a82..b2d6355 100644 > --- a/libavcodec/qsvenc.h > +++ b/libavcodec/qsvenc.h > @@ -102,8 +102,6 @@ typedef struct QSVEncContext { > int width_align; > int height_align; > > - int sum_frame_qp; > - > mfxVideoParam param; > mfxFrameAllocRequest req; > > 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); > } Can't test, but looks ok.
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of James Almer > Sent: Friday, August 10, 2018 9:13 PM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH V2] lavc/qsvenc: add quality status to > side_data > > On 8/10/2018 5:10 AM, Zhong Li wrote: > > Add fix a memory leak issue as James's comments. > > > > V2: use a local pict_type since coded_frame is deprecated. > > > > Signed-off-by: Zhong Li <zhong.li@intel.com> > > --- > > libavcodec/qsvenc.c | 32 ++++++++++++++++++++++---------- > > libavcodec/qsvenc.h | 2 -- > > libavcodec/qsvenc_h264.c | 5 ----- > > 3 files changed, 22 insertions(+), 17 deletions(-) > > > > diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c index > > 65dae31..0a14e6a 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,7 +1270,9 @@ int ff_qsv_encode(AVCodecContext *avctx, > QSVEncContext *q, > > mfxSyncPoint *sync; > > #if QSV_VERSION_ATLEAST(1, 26) > > mfxExtAVCEncodedFrameInfo *enc_info; > > + mfxExtBuffer **enc_buf; > > #endif > > + enum AVPictureType pict_type; > > > > av_fifo_generic_read(q->async_fifo, &new_pkt, > sizeof(new_pkt), NULL); > > av_fifo_generic_read(q->async_fifo, &sync, sizeof(sync), > NULL); > > @@ -1282,23 +1290,27 @@ int ff_qsv_encode(AVCodecContext *avctx, > QSVEncContext *q, > > bs->FrameType & MFX_FRAMETYPE_xIDR) > > new_pkt.flags |= AV_PKT_FLAG_KEY; > > > > -#if FF_API_CODED_FRAME > > -FF_DISABLE_DEPRECATION_WARNINGS > > if (bs->FrameType & MFX_FRAMETYPE_I || bs->FrameType & > MFX_FRAMETYPE_xI) > > - avctx->coded_frame->pict_type = AV_PICTURE_TYPE_I; > > + pict_type = AV_PICTURE_TYPE_I; > > else if (bs->FrameType & MFX_FRAMETYPE_P || > bs->FrameType & MFX_FRAMETYPE_xP) > > - avctx->coded_frame->pict_type = AV_PICTURE_TYPE_P; > > + pict_type = AV_PICTURE_TYPE_P; > > else if (bs->FrameType & MFX_FRAMETYPE_B || > bs->FrameType & MFX_FRAMETYPE_xB) > > - avctx->coded_frame->pict_type = AV_PICTURE_TYPE_B; > > + pict_type = AV_PICTURE_TYPE_B; > > + > > +#if FF_API_CODED_FRAME > > +FF_DISABLE_DEPRECATION_WARNINGS > > + avctx->coded_frame->pict_type = pict_type; > > FF_ENABLE_DEPRECATION_WARNINGS > > #endif > > > > #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); > > - q->sum_frame_qp += enc_info->QP; > > + ff_side_data_set_encoder_stats(&new_pkt, > > + enc_info->QP * FF_QP2LAMBDA, NULL, 0, pict_type); > > av_freep(&enc_info); > > + av_freep(&enc_buf); > > } > > #endif > > av_freep(&bs); > > diff --git a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h index > > 3784a82..b2d6355 100644 > > --- a/libavcodec/qsvenc.h > > +++ b/libavcodec/qsvenc.h > > @@ -102,8 +102,6 @@ typedef struct QSVEncContext { > > int width_align; > > int height_align; > > > > - int sum_frame_qp; > > - > > mfxVideoParam param; > > mfxFrameAllocRequest req; > > > > 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); } > > Can't test, but looks ok. Thanks. Will apply after my regression test done.
diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c index 65dae31..0a14e6a 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,7 +1270,9 @@ int ff_qsv_encode(AVCodecContext *avctx, QSVEncContext *q, mfxSyncPoint *sync; #if QSV_VERSION_ATLEAST(1, 26) mfxExtAVCEncodedFrameInfo *enc_info; + mfxExtBuffer **enc_buf; #endif + enum AVPictureType pict_type; av_fifo_generic_read(q->async_fifo, &new_pkt, sizeof(new_pkt), NULL); av_fifo_generic_read(q->async_fifo, &sync, sizeof(sync), NULL); @@ -1282,23 +1290,27 @@ int ff_qsv_encode(AVCodecContext *avctx, QSVEncContext *q, bs->FrameType & MFX_FRAMETYPE_xIDR) new_pkt.flags |= AV_PKT_FLAG_KEY; -#if FF_API_CODED_FRAME -FF_DISABLE_DEPRECATION_WARNINGS if (bs->FrameType & MFX_FRAMETYPE_I || bs->FrameType & MFX_FRAMETYPE_xI) - avctx->coded_frame->pict_type = AV_PICTURE_TYPE_I; + pict_type = AV_PICTURE_TYPE_I; else if (bs->FrameType & MFX_FRAMETYPE_P || bs->FrameType & MFX_FRAMETYPE_xP) - avctx->coded_frame->pict_type = AV_PICTURE_TYPE_P; + pict_type = AV_PICTURE_TYPE_P; else if (bs->FrameType & MFX_FRAMETYPE_B || bs->FrameType & MFX_FRAMETYPE_xB) - avctx->coded_frame->pict_type = AV_PICTURE_TYPE_B; + pict_type = AV_PICTURE_TYPE_B; + +#if FF_API_CODED_FRAME +FF_DISABLE_DEPRECATION_WARNINGS + avctx->coded_frame->pict_type = pict_type; FF_ENABLE_DEPRECATION_WARNINGS #endif #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); - q->sum_frame_qp += enc_info->QP; + ff_side_data_set_encoder_stats(&new_pkt, + enc_info->QP * FF_QP2LAMBDA, NULL, 0, pict_type); av_freep(&enc_info); + av_freep(&enc_buf); } #endif av_freep(&bs); diff --git a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h index 3784a82..b2d6355 100644 --- a/libavcodec/qsvenc.h +++ b/libavcodec/qsvenc.h @@ -102,8 +102,6 @@ typedef struct QSVEncContext { int width_align; int height_align; - int sum_frame_qp; - mfxVideoParam param; mfxFrameAllocRequest req; 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. V2: use a local pict_type since coded_frame is deprecated. Signed-off-by: Zhong Li <zhong.li@intel.com> --- libavcodec/qsvenc.c | 32 ++++++++++++++++++++++---------- libavcodec/qsvenc.h | 2 -- libavcodec/qsvenc_h264.c | 5 ----- 3 files changed, 22 insertions(+), 17 deletions(-)