[FFmpeg-devel] [PATCH 3/4] avformat/flvenc: Use array instead of linked list for index

James Almer jamrial at gmail.com
Fri Oct 25 23:57:44 EEST 2019


On 10/25/2019 5:44 PM, Michael Niedermayer wrote:
> On Fri, Oct 25, 2019 at 11:11:46AM +0200, Andreas Rheinhardt wrote:
>> Using a linked list had very much overhead (the pointer to the next
>> entry increased the size of the index entry struct from 16 to 24 bytes,
>> not to mention the overhead of having separate allocations), so it is
>> better to (re)allocate a continuous array for the index.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
>> ---
>>  libavformat/flvenc.c | 58 +++++++++++++++-----------------------------
>>  1 file changed, 19 insertions(+), 39 deletions(-)
>>
>> diff --git a/libavformat/flvenc.c b/libavformat/flvenc.c
>> index 0e6c66a5ff..a2bd791c59 100644
>> --- a/libavformat/flvenc.c
>> +++ b/libavformat/flvenc.c
>> @@ -74,7 +74,6 @@ typedef enum {
>>  typedef struct FLVFileposition {
>>      int64_t keyframe_position;
>>      double keyframe_timestamp;
>> -    struct FLVFileposition *next;
>>  } FLVFileposition;
>>  
>>  typedef struct FLVContext {
>> @@ -108,9 +107,9 @@ typedef struct FLVContext {
>>      int acurframeindex;
>>      int64_t keyframes_info_offset;
>>  
>> -    int64_t filepositions_count;
>>      FLVFileposition *filepositions;
>> -    FLVFileposition *head_filepositions;
>> +    size_t filepositions_allocated;
>> +    int64_t filepositions_count;
>>  
>>      AVCodecParameters *audio_par;
>>      AVCodecParameters *video_par;
>> @@ -549,27 +548,19 @@ static void flv_write_codec_header(AVFormatContext* s, AVCodecParameters* par, i
>>  
>>  static int flv_append_keyframe_info(AVFormatContext *s, FLVContext *flv, double ts, int64_t pos)
>>  {
>> -    FLVFileposition *position = av_malloc(sizeof(FLVFileposition));
>> -
>> -    if (!position) {
>> -        av_log(s, AV_LOG_WARNING, "no mem for add keyframe index!\n");
>> -        return AVERROR(ENOMEM);
>> -    }
>> -
>> -    position->keyframe_timestamp = ts;
>> -    position->keyframe_position = pos;
>> -
>> -    if (!flv->filepositions_count) {
>> -        flv->filepositions = position;
>> -        flv->head_filepositions = flv->filepositions;
>> -        position->next = NULL;
>> -    } else {
>> -        flv->filepositions->next = position;
>> -        position->next = NULL;
>> -        flv->filepositions = flv->filepositions->next;
>> +    if (flv->filepositions_count >= flv->filepositions_allocated) {
>> +        void *pos = av_realloc_array(flv->filepositions,
>> +                                     2 * flv->filepositions_allocated + 1,
>> +                                     sizeof(*flv->filepositions));
> 
> can the 2* overflow ?
> av_fast_realloc() would check for that
> i wonder if a av_fast_realloc_array() would make sense

av_fast_realloc() doesn't do any overflow check. It only checks that the
min_size argument is below the max allowed alloc size.

But i agree that using it preceded by a sanity check that ensures
something like "flv->filepositions_count + 1 <= INT_MAX /
sizeof(*flv->filepositions)" would be more adequate than
av_realloc_array() with 2*size steps.

> 
> thx
> 
> [...]
> 
> 
> _______________________________________________
> 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