[FFmpeg-devel] [PATCH 06/13] avcodec/mpegvideo_enc: Don't overallocate arrays

[Andreas Rheinhardt]

Michael Niedermayer:
> On Fri, Oct 06, 2023 at 04:46:29AM +0200, Andreas Rheinhardt wrote:
>> Only entries 0..max_b_frames are ever used.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
>> ---
>>  libavcodec/mpegvideo_enc.c | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
>> index 1e0aed8db9..c06fdd08fe 100644
>> --- a/libavcodec/mpegvideo_enc.c
>> +++ b/libavcodec/mpegvideo_enc.c
>> @@ -819,8 +819,8 @@ av_cold int ff_mpv_encode_init(AVCodecContext *avctx)
>>          !FF_ALLOCZ_TYPED_ARRAY(s->q_intra_matrix16,        32) ||
>>          !FF_ALLOCZ_TYPED_ARRAY(s->q_chroma_intra_matrix16, 32) ||
>>          !FF_ALLOCZ_TYPED_ARRAY(s->q_inter_matrix16,        32) ||
>> -        !FF_ALLOCZ_TYPED_ARRAY(s->input_picture,           MAX_PICTURE_COUNT) ||
>> -        !FF_ALLOCZ_TYPED_ARRAY(s->reordered_input_picture, MAX_PICTURE_COUNT))
>> +        !FF_ALLOCZ_TYPED_ARRAY(s->input_picture,           MAX_B_FRAMES + 1) ||
>> +        !FF_ALLOCZ_TYPED_ARRAY(s->reordered_input_picture, MAX_B_FRAMES + 1))
>>          return AVERROR(ENOMEM);
>>  
>>      /* Allocate MV tables; the MV and MB tables will be copied
>> @@ -1231,7 +1231,7 @@ static int load_input_picture(MpegEncContext *s, const AVFrame *pic_arg)
>>      }
>>  
>>      /* shift buffer entries */
>> -    for (i = flush_offset; i < MAX_PICTURE_COUNT /*s->encoding_delay + 1*/; i++)
>> +    for (int i = flush_offset; i <= MAX_B_FRAMES; i++)
>>          s->input_picture[i - flush_offset] = s->input_picture[i];
>>  
>>      s->input_picture[encoding_delay] = pic;
>> @@ -1450,9 +1450,9 @@ static int select_input_picture(MpegEncContext *s)
>>  {
>>      int i, ret;
>>  
>> -    for (i = 1; i < MAX_PICTURE_COUNT; i++)
>> +    for (int i = 1; i <= MAX_B_FRAMES; i++)
>>          s->reordered_input_picture[i - 1] = s->reordered_input_picture[i];
> 
> I see the addition of "int" and that seems neither needed nor
> explained why in the commit message
> 

It's part of the general switch to the loop-based iterators wherever
possible (it is better because it automatically indicates that the value
at the end of the loop doesn't matter and it also allows to more easily
move blocks of code around). I always use them when I touch a loop.

If it matters: That i in the outer scope survives this patchset, but it
won't survive for long.

- Andreas