[FFmpeg-devel] [PATCH] opt: check image size when setting it

[Hendrik Leppkes]

On Sat, Dec 10, 2016 at 4:26 PM, wm4 <nfxjfg at googlemail.com> wrote:
> On Sat, 10 Dec 2016 16:11:15 +0100
> Andreas Cadhalpun <andreas.cadhalpun at googlemail.com> wrote:
>
>> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
>> ---
>>  libavutil/opt.c | 10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavutil/opt.c b/libavutil/opt.c
>> index f855ccb..f713d3f 100644
>> --- a/libavutil/opt.c
>> +++ b/libavutil/opt.c
>> @@ -32,6 +32,7 @@
>>  #include "common.h"
>>  #include "dict.h"
>>  #include "eval.h"
>> +#include "imgutils.h"
>>  #include "log.h"
>>  #include "parseutils.h"
>>  #include "pixdesc.h"
>> @@ -325,8 +326,15 @@ static int set_string_image_size(void *obj, const AVOption *o, const char *val,
>>          return 0;
>>      }
>>      ret = av_parse_video_size(dst, dst + 1, val);
>> -    if (ret < 0)
>> +    if (ret < 0) {
>>          av_log(obj, AV_LOG_ERROR, "Unable to parse option value \"%s\" as image size\n", val);
>> +        return ret;
>> +    }
>> +    ret = av_image_check_size(*dst, *(dst + 1), 0, obj);
>> +    if (ret < 0) {
>> +        *dst = 0;
>> +        *(dst + 1) = 0;
>> +    }
>>      return ret;
>>  }
>>
>
> I'd argue that rather than doing this, image allocation functions etc.
> should error out if the dimensions are too large.
>
> This way the allowed dimensions could be enlarged (e.g. by taking the
> pixel format into account) without changing everything from int to
> size_t.

I agree, av_image_check_size has some arbitrary technical limits in it
right now that may not necessarily apply everywhere you want to set an
image size.
Allocations would need to be checked anyway with or without this check
because most options accessible through AVOptions are also accessible
through other means, so I would rather leave the decision of the
maximum size up to the code that uses these values.

- Hendrik