Xiang, Haihao:
> 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.rheinha...@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 for the review. Here is a sample that allows to reproduce the
crash: http://streams.videolan.org/streams/ts/CC/NewsStream-608-ac3.ts
in case you haven't already have one.
- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
