[FFmpeg-devel] [PATCH 1/6] avcodec/qsvenc: Fix leak and crash when encoding H.264 due to A53_CC
Xiang, Haihao
haihao.xiang at intel.com
Mon Sep 27 11:14:34 EEST 2021
On Sun, 2021-09-26 at 08:32 +0200, Andreas Rheinhardt wrote:
> Since commit 3bbe0c210b05fc6fbd7b1d4bbd8479db7f2cf957, the Payloads
> array of every QSVFrame leaks as soon as the frame is reused;
> the leak is small and not very noticeable, but if there is an attempt
> to use said array the ensuing crash is much more noticeable.
> This happens when encoding H.264 with A53 CC side data.
>
> Furthermore, if said array can not be allocated at all, an AVFrame
> leaks.
>
> Fix all of this by not allocating the array separately at all; put it
> in QSVFrame instead and restore the Payloads array upon reusing the
> frame.
>
> Finally, use av_freep() instead of av_free() to free the payload
> entries.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
> ---
> libavcodec/qsv_internal.h | 2 ++
> libavcodec/qsvenc.c | 10 +++-------
> 2 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/libavcodec/qsv_internal.h b/libavcodec/qsv_internal.h
> index 8090b748b3..fe9d5319c4 100644
> --- a/libavcodec/qsv_internal.h
> +++ b/libavcodec/qsv_internal.h
> @@ -76,6 +76,8 @@ typedef struct QSVFrame {
> mfxExtDecodedFrameInfo dec_info;
> mfxExtBuffer *ext_param;
>
> + mfxPayload *payloads[QSV_MAX_ENC_PAYLOAD]; ///< used for enc_ctrl.Payload
> +
> int queued;
> int used;
>
> diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
> index 06f55604b5..66f79bb021 100644
> --- a/libavcodec/qsvenc.c
> +++ b/libavcodec/qsvenc.c
> @@ -1259,7 +1259,7 @@ static void free_encoder_ctrl_payloads(mfxEncodeCtrl*
> enc_ctrl)
> if (enc_ctrl) {
> int i;
> for (i = 0; i < enc_ctrl->NumPayload && i < QSV_MAX_ENC_PAYLOAD; i++)
> {
> - av_free(enc_ctrl->Payload[i]);
> + av_freep(&enc_ctrl->Payload[i]);
> }
> enc_ctrl->NumPayload = 0;
> }
> @@ -1273,6 +1273,7 @@ static void clear_unused_frames(QSVEncContext *q)
> free_encoder_ctrl_payloads(&cur->enc_ctrl);
> //do not reuse enc_ctrl from previous frame
> memset(&cur->enc_ctrl, 0, sizeof(cur->enc_ctrl));
> + cur->enc_ctrl.Payload = cur->payloads;
> if (cur->frame->format == AV_PIX_FMT_QSV) {
> av_frame_unref(cur->frame);
> }
> @@ -1309,11 +1310,7 @@ static int get_free_frame(QSVEncContext *q, QSVFrame
> **f)
> av_freep(&frame);
> return AVERROR(ENOMEM);
> }
> - frame->enc_ctrl.Payload = av_mallocz(sizeof(mfxPayload*) *
> QSV_MAX_ENC_PAYLOAD);
> - if (!frame->enc_ctrl.Payload) {
> - av_freep(&frame);
> - return AVERROR(ENOMEM);
> - }
> + frame->enc_ctrl.Payload = frame->payloads;
> *last = frame;
>
> *f = frame;
> @@ -1615,7 +1612,6 @@ int ff_qsv_enc_close(AVCodecContext *avctx,
> QSVEncContext *q)
> while (cur) {
> q->work_frames = cur->next;
> av_frame_free(&cur->frame);
> - av_free(cur->enc_ctrl.Payload);
> av_freep(&cur);
> cur = q->work_frames;
> }
I'm sorry commit 3bbe0c2 caused a regression, your patch looks good to me.
Thanks
Haihao
More information about the ffmpeg-devel
mailing list