[FFmpeg-devel] [PATCH] lavu/video_enc_params: make sure blocks are properly aligned

Andreas Rheinhardt andreas.rheinhardt at outlook.com
Thu Apr 8 13:27:03 EEST 2021


Anton Khirnov:
> Quoting Andreas Rheinhardt (2021-04-08 11:41:41)
>> Anton Khirnov:
>>> ---
>>>  libavutil/video_enc_params.c | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/libavutil/video_enc_params.c b/libavutil/video_enc_params.c
>>> index 635176ab91..1779a8799a 100644
>>> --- a/libavutil/video_enc_params.c
>>> +++ b/libavutil/video_enc_params.c
>>> @@ -30,9 +30,9 @@ AVVideoEncParams *av_video_enc_params_alloc(enum AVVideoEncParamsType type,
>>>                                              unsigned int nb_blocks, size_t *out_size)
>>>  {
>>>      AVVideoEncParams *par;
>>> -    size_t size;
>>> +    size_t blocks_offset = FFALIGN(sizeof(*par), 8);
>>> +    size_t size          = blocks_offset;
>>>  
>>> -    size = sizeof(*par);
>>>      if (nb_blocks > (SIZE_MAX - size) / sizeof(AVVideoBlockParams))
>>>          return NULL;
>>>      size += sizeof(AVVideoBlockParams) * nb_blocks;
>>> @@ -44,7 +44,7 @@ AVVideoEncParams *av_video_enc_params_alloc(enum AVVideoEncParamsType type,
>>>      par->type          = type;
>>>      par->nb_blocks     = nb_blocks;
>>>      par->block_size    = sizeof(AVVideoBlockParams);
>>> -    par->blocks_offset = sizeof(*par);
>>> +    par->blocks_offset = blocks_offset;
>>>  
>>>      if (out_size)
>>>          *out_size = size;
>>>
>> Wouldn't it be safer to just define a struct { AVVideoEncParams header;
>> AVVideoBlockParams blocks[1]; } and use the offset of blocks in that?
> 
> I don't see how it would be safer and it prevents you from adding new
> fields to AVVideoEncParams
> 
Safer: You are making assumptions about the alignment of
AVVideoBlockParams; these assumptions may or may not be fulfilled in
practice.
And I really don't get your point about this preventing the addition of
new fields to AVVideoEncParams. If something is added to
AVVideoEncParams, sizeof(header) and offsetof(blocks) will grow
automatically as required.

- Andreas


More information about the ffmpeg-devel mailing list