[FFmpeg-devel] [PATCH v3 1/4] libavutil/imgutils: add utility to get plane sizes

[James Almer]

On 7/13/2020 2:09 PM, Brian Kim wrote:
> This utility helps avoid undefined behavior when doing things like
> checking how much memory we need to allocate for an image before we have
> allocated a buffer.
> 
> Signed-off-by: Brian Kim <bkkim at google.com>
> ---
>  doc/APIchanges       |  3 ++
>  libavutil/imgutils.c | 98 +++++++++++++++++++++++++++++++++-----------
>  libavutil/imgutils.h | 11 +++++
>  libavutil/version.h  |  2 +-
>  4 files changed, 90 insertions(+), 24 deletions(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 1d6cc36b8c..44defd9ca8 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -15,6 +15,9 @@ libavutil:     2017-10-21
>  
>  API changes, most recent first:
>  
> +2020-07-xx - xxxxxxxxxx - lavu 56.56.100 - imgutils.h
> +  Add av_image_fill_plane_sizes().
> +
>  2020-06-12 - b09fb030c1 - lavu 56.55.100 - pixdesc.h
>    Add AV_PIX_FMT_X2RGB10.
>  
> diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
> index 7f9c1b632c..345b7fa94c 100644
> --- a/libavutil/imgutils.c
> +++ b/libavutil/imgutils.c
> @@ -108,45 +108,69 @@ int av_image_fill_linesizes(int linesizes[4], enum AVPixelFormat pix_fmt, int wi
>      return 0;
>  }
>  
> -int av_image_fill_pointers(uint8_t *data[4], enum AVPixelFormat pix_fmt, int height,
> -                           uint8_t *ptr, const int linesizes[4])
> +int av_image_fill_plane_sizes(size_t sizes[4], enum AVPixelFormat pix_fmt,
> +                              int height, const ptrdiff_t linesizes[4])
>  {
> -    int i, total_size, size[4] = { 0 }, has_plane[4] = { 0 };
> +    int i, has_plane[4] = { 0 };
>  
>      const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pix_fmt);
> -    memset(data     , 0, sizeof(data[0])*4);
> +    memset(sizes    , 0, sizeof(sizes[0])*4);
>  
>      if (!desc || desc->flags & AV_PIX_FMT_FLAG_HWACCEL)
>          return AVERROR(EINVAL);
>  
> -    data[0] = ptr;
> -    if (linesizes[0] > (INT_MAX - 1024) / height)
> +    if (linesizes[0] > SIZE_MAX / height)
>          return AVERROR(EINVAL);
> -    size[0] = linesizes[0] * height;
> +    sizes[0] = linesizes[0] * height;

You would need to cast height to size_t for this, i think, but seeing
av_image_check_size() currently rejects line sizes and plane sizes
bigger than INT_MAX, maybe we should just keep INT_MAX in the above
check instead (No need to send a new revision for this, i can amend it
before pushing with either solution. I just want your opinion).

>  
>      if (desc->flags & AV_PIX_FMT_FLAG_PAL ||
>          desc->flags & FF_PSEUDOPAL) {
> -        data[1] = ptr + size[0]; /* palette is stored here as 256 32 bits words */
> -        return size[0] + 256 * 4;
> +        sizes[1] = 256 * 4; /* palette is stored here as 256 32 bits words */
> +        return 0;
>      }
>  
>      for (i = 0; i < 4; i++)
>          has_plane[desc->comp[i].plane] = 1;
>  
> -    total_size = size[0];
>      for (i = 1; i < 4 && has_plane[i]; i++) {
>          int h, s = (i == 1 || i == 2) ? desc->log2_chroma_h : 0;
> -        data[i] = data[i-1] + size[i-1];
>          h = (height + (1 << s) - 1) >> s;
> -        if (linesizes[i] > INT_MAX / h)
> +        if (linesizes[i] > SIZE_MAX / h)
>              return AVERROR(EINVAL);
> -        size[i] = h * linesizes[i];
> -        if (total_size > INT_MAX - size[i])
> +        sizes[i] = h * linesizes[i];

Same here.

Rest LGTM.