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

Submitted by Michael Niedermayer on Sept. 10, 2018, 10:57 p.m.

Details

Message ID 20180910225711.13391-1-michael@niedermayer.cc
State New
Headers show

Commit Message

Michael Niedermayer Sept. 10, 2018, 10:57 p.m.
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(-)

Comments

Michael Niedermayer Sept. 10, 2018, 11:03 p.m.
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


[...]
James Almer Sept. 11, 2018, 1:12 a.m.
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;
>

Patch hide | download patch | download mbox

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++) {
+        if (frame->data[i])
+            frame->data[i] += i*plane_padding;
     }
 
     frame->extended_data = frame->data;