[FFmpeg-devel] [PATCH 06/50] avcodec/mpegvideo_enc: use av_packet_alloc() to allocate packets

[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.

> 
>> +        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".
>