[FFmpeg-devel] [PATCH 06/50] avcodec/mpegvideo_enc: use av_packet_alloc() to allocate packets
Andreas Rheinhardt
andreas.rheinhardt at gmail.com
Mon Feb 8 16:50:48 EET 2021
James Almer:
> On 2/8/2021 11:46 AM, Andreas Rheinhardt wrote:
>> James Almer:
>>> Signed-off-by: James Almer <jamrial at gmail.com>
>>> ---
>>> libavcodec/mpegvideo_enc.c | 23 +++++++++++++----------
>>> 1 file changed, 13 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
>>> index 34dcf8c313..411cadeae7 100644
>>> --- a/libavcodec/mpegvideo_enc.c
>>> +++ b/libavcodec/mpegvideo_enc.c
>>> @@ -1366,23 +1366,20 @@ static int skip_check(MpegEncContext *s,
>>> Picture *p, Picture *ref)
>>> return 0;
>>> }
>>> -static int encode_frame(AVCodecContext *c, AVFrame *frame)
>>> +static int encode_frame(AVCodecContext *c, AVFrame *frame, AVPacket
>>> *pkt)
>>> {
>>> - AVPacket pkt = { 0 };
>>> int ret;
>>> int size = 0;
>>> - av_init_packet(&pkt);
>>> -
>>> ret = avcodec_send_frame(c, frame);
>>> if (ret < 0)
>>> return ret;
>>> do {
>>> - ret = avcodec_receive_packet(c, &pkt);
>>> + ret = avcodec_receive_packet(c, pkt);
>>> if (ret >= 0) {
>>> - size += pkt.size;
>>> - av_packet_unref(&pkt);
>>> + size += pkt->size;
>>> + av_packet_unref(pkt);
>>> } else if (ret < 0 && ret != AVERROR(EAGAIN) && ret !=
>>> AVERROR_EOF)
>>> return ret;
>>> } while (ret >= 0);
>>> @@ -1448,6 +1445,7 @@ static int estimate_best_b_count(MpegEncContext
>>> *s)
>>> for (j = 0; j < s->max_b_frames + 1; j++) {
>>> AVCodecContext *c;
>>> + AVPacket *pkt;
>>> int64_t rd = 0;
>>> if (!s->input_picture[j])
>>> @@ -1473,10 +1471,14 @@ static int
>>> estimate_best_b_count(MpegEncContext *s)
>>> if (ret < 0)
>>> goto fail;
>>
>> The av_packet_free in the fail code uses an uninitialized pointer.
>>
>>> + pkt = av_packet_alloc();
>>
>> You are adding s->max_b_frames + 1 allocations + frees per packet to be
>> encoded (if I am not mistaken). I am speechless.
>
> I can try to move it outside the loop. But said loop is already
> allocating that many AVCodecContexts, so hardly that much of a difference.
>
Still the wrong direction even when the current state is already bad.
>>
>>> + if (!pkt)
>>> + goto fail;
>>
>> You forgot to set ret.
>>
>>> +
>>> s->tmp_frames[0]->pict_type = AV_PICTURE_TYPE_I;
>>> s->tmp_frames[0]->quality = 1 * FF_QP2LAMBDA;
>>> - out_size = encode_frame(c, s->tmp_frames[0]);
>>> + out_size = encode_frame(c, s->tmp_frames[0], pkt);
>>> if (out_size < 0) {
>>> ret = out_size;
>>> goto fail;
>>> @@ -1491,7 +1493,7 @@ static int estimate_best_b_count(MpegEncContext
>>> *s)
>>> AV_PICTURE_TYPE_P :
>>> AV_PICTURE_TYPE_B;
>>> s->tmp_frames[i + 1]->quality = is_p ? p_lambda :
>>> b_lambda;
>>> - out_size = encode_frame(c, s->tmp_frames[i + 1]);
>>> + out_size = encode_frame(c, s->tmp_frames[i + 1], pkt);
>>> if (out_size < 0) {
>>> ret = out_size;
>>> goto fail;
>>> @@ -1501,7 +1503,7 @@ static int estimate_best_b_count(MpegEncContext
>>> *s)
>>> }
>>> /* get the delayed frames */
>>> - out_size = encode_frame(c, NULL);
>>> + out_size = encode_frame(c, NULL, pkt);
>>> if (out_size < 0) {
>>> ret = out_size;
>>> goto fail;
>>> @@ -1517,6 +1519,7 @@ static int estimate_best_b_count(MpegEncContext
>>> *s)
>>> fail:
>>> avcodec_free_context(&c);
>>> + av_packet_free(&pkt);
>>> if (ret < 0)
>>> return ret;
>>> }
>>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list