[FFmpeg-devel] avutil/frame: Simplify the video allocation

From: Luca Barbato <lu_zero@gentoo.org>

Merged-by: James Almer <jamrial@gmail.com>
Padding-Remixed-by: Michael Niedermayer <michael@niedermayer.cc>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavutil/frame.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

On Tue, Sep 11, 2018 at 12:57:11AM +0200, Michael Niedermayer wrote:
> From: Luca Barbato <lu_zero@gentoo.org>
> 
> Merged-by: James Almer <jamrial@gmail.com>
> Padding-Remixed-by: Michael Niedermayer <michael@niedermayer.cc>
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavutil/frame.c | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index deb9b6f334..6147c61259 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -211,7 +211,8 @@ void av_frame_free(AVFrame **frame)
>  static int get_video_buffer(AVFrame *frame, int align)
>  {
>      const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(frame->format);
> -    int ret, i;
> +    int ret, i, padded_height;
> +    int plane_padding = FFMAX(16 + 16/*STRIDE_ALIGN*/, align);
>  
>      if (!desc)
>          return AVERROR(EINVAL);
> @@ -236,23 +237,22 @@ static int get_video_buffer(AVFrame *frame, int align)
>              frame->linesize[i] = FFALIGN(frame->linesize[i], align);
>      }
>  
> -    for (i = 0; i < 4 && frame->linesize[i]; i++) {
> -        int h = FFALIGN(frame->height, 32);
> -        if (i == 1 || i == 2)
> -            h = AV_CEIL_RSHIFT(h, desc->log2_chroma_h);
> +    padded_height = FFALIGN(frame->height, 32);
> +    if ((ret = av_image_fill_pointers(frame->data, frame->format, padded_height,
> +                                      NULL, frame->linesize)) < 0)
> +        return ret;
>  
> -        frame->buf[i] = av_buffer_alloc(frame->linesize[i] * h + 16 + 16/*STRIDE_ALIGN*/ - 1);
> -        if (!frame->buf[i])
> -            goto fail;
> +    frame->buf[0] = av_buffer_alloc(ret + 4*plane_padding);
> +    if (!frame->buf[0])
> +        goto fail;
>  
> -        frame->data[i] = frame->buf[i]->data;
> -    }
> -    if (desc->flags & AV_PIX_FMT_FLAG_PAL || desc->flags & FF_PSEUDOPAL) {
> -        av_buffer_unref(&frame->buf[1]);
> -        frame->buf[1] = av_buffer_alloc(AVPALETTE_SIZE);
> -        if (!frame->buf[1])
> -            goto fail;
> -        frame->data[1] = frame->buf[1]->data;
> +    if (av_image_fill_pointers(frame->data, frame->format, padded_height,
> +                               frame->buf[0]->data, frame->linesize) < 0)
> +        goto fail;
> +

> +    for (i = 0; i < 4; i++) {
                ^
this is more ideal if its 1, saw it a moment after sending


[...]

On 9/10/2018 7:57 PM, Michael Niedermayer wrote:
> From: Luca Barbato <lu_zero@gentoo.org>
> 
> Merged-by: James Almer <jamrial@gmail.com>
> Padding-Remixed-by: Michael Niedermayer <michael@niedermayer.cc>
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavutil/frame.c | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index deb9b6f334..6147c61259 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -211,7 +211,8 @@ void av_frame_free(AVFrame **frame)
>  static int get_video_buffer(AVFrame *frame, int align)
>  {
>      const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(frame->format);
> -    int ret, i;
> +    int ret, i, padded_height;
> +    int plane_padding = FFMAX(16 + 16/*STRIDE_ALIGN*/, align);

STRIDE_ALIGN can be 64 right now, depending on configuration time
options and environment. But there's no avx512 code in the tree just
yet, so i guess it's not important for now.

>  
>      if (!desc)
>          return AVERROR(EINVAL);
> @@ -236,23 +237,22 @@ static int get_video_buffer(AVFrame *frame, int align)
>              frame->linesize[i] = FFALIGN(frame->linesize[i], align);
>      }
>  
> -    for (i = 0; i < 4 && frame->linesize[i]; i++) {
> -        int h = FFALIGN(frame->height, 32);
> -        if (i == 1 || i == 2)
> -            h = AV_CEIL_RSHIFT(h, desc->log2_chroma_h);
> +    padded_height = FFALIGN(frame->height, 32);
> +    if ((ret = av_image_fill_pointers(frame->data, frame->format, padded_height,
> +                                      NULL, frame->linesize)) < 0)
> +        return ret;
>  
> -        frame->buf[i] = av_buffer_alloc(frame->linesize[i] * h + 16 + 16/*STRIDE_ALIGN*/ - 1);
> -        if (!frame->buf[i])
> -            goto fail;
> +    frame->buf[0] = av_buffer_alloc(ret + 4*plane_padding);
> +    if (!frame->buf[0])
> +        goto fail;
>  
> -        frame->data[i] = frame->buf[i]->data;
> -    }
> -    if (desc->flags & AV_PIX_FMT_FLAG_PAL || desc->flags & FF_PSEUDOPAL) {
> -        av_buffer_unref(&frame->buf[1]);
> -        frame->buf[1] = av_buffer_alloc(AVPALETTE_SIZE);
> -        if (!frame->buf[1])
> -            goto fail;
> -        frame->data[1] = frame->buf[1]->data;
> +    if (av_image_fill_pointers(frame->data, frame->format, padded_height,
> +                               frame->buf[0]->data, frame->linesize) < 0)
> +        goto fail;
> +
> +    for (i = 0; i < 4; i++) {
> +        if (frame->data[i])
> +            frame->data[i] += i*plane_padding;

I see now what you meant regarding the pointers post merge.

Thanks a lot. Will merge later with the suggested change in your reply
to this patch.

>      }
>  
>      frame->extended_data = frame->data;
>