[FFmpeg-devel] [PATCH] avfilter/vf_stack: round down internal item heights

[Timo Rothenpieler]

On 23/03/2024 16:30, Timo Rothenpieler wrote:
> Following situation:
> Someone wants to stack two yuv420p frames of the size 3x3 pixel each.
> Now the various AV_CEIL_RSHIFT() calculations calculate a height of 2
> pixel for each items subsampled planes.
> However, for example in case of a vstack, the output frames will have a
> height of 6, so the subsampled planes one of 3.
> When the filter now tries to stack two items with a rounded-up height of
> 2 into a frame with a height of 3, it'll write an entire extra line past
> the end of a buffer.
> 
> This patch instead rounds down all the item heights, to avoid this
> situation. It's not ideal either, since now lines might be missed. But
> that is definitely preferable over writing past the end of the
> bufferThis patch instead rounds down all the item heights, to avoid this
> situation. It's not ideal either, since now lines might be missed. But
> that is definitely preferable over writing past the end of the buffer.
> ---
>   libavfilter/vf_stack.c | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/libavfilter/vf_stack.c b/libavfilter/vf_stack.c
> index 2bb3d9b1d2..46e7d6b7f5 100644
> --- a/libavfilter/vf_stack.c
> +++ b/libavfilter/vf_stack.c
> @@ -220,11 +220,11 @@ static int config_output(AVFilterLink *outlink)
>                   return ret;
>               }
>   
> -            item->height[1] = item->height[2] = AV_CEIL_RSHIFT(inlink->h, s->desc->log2_chroma_h);
> +            item->height[1] = item->height[2] = inlink->h / (1 << s->desc->log2_chroma_h);
>               item->height[0] = item->height[3] = inlink->h;
>   
>               if (i) {
> -                item->y[1] = item->y[2] = AV_CEIL_RSHIFT(height, s->desc->log2_chroma_h);
> +                item->y[1] = item->y[2] = height / (1 << s->desc->log2_chroma_h);
>                   item->y[0] = item->y[3] = height;
>   
>                   height += ctx->inputs[i]->h;
> @@ -244,7 +244,7 @@ static int config_output(AVFilterLink *outlink)
>                   return ret;
>               }
>   
> -            item->height[1] = item->height[2] = AV_CEIL_RSHIFT(inlink->h, s->desc->log2_chroma_h);
> +            item->height[1] = item->height[2] = inlink->h / (1 << s->desc->log2_chroma_h);
>               item->height[0] = item->height[3] = inlink->h;
>   
>               if (i) {
> @@ -278,14 +278,14 @@ static int config_output(AVFilterLink *outlink)
>                       return ret;
>                   }
>   
> -                item->height[1] = item->height[2] = AV_CEIL_RSHIFT(inlink->h, s->desc->log2_chroma_h);
> +                item->height[1] = item->height[2] = inlink->h / (1 << s->desc->log2_chroma_h);
>                   item->height[0] = item->height[3] = inlink->h;
>   
>                   if ((ret = av_image_fill_linesizes(item->x, inlink->format, inw)) < 0) {
>                       return ret;
>                   }
>   
> -                item->y[1] = item->y[2] = AV_CEIL_RSHIFT(inh, s->desc->log2_chroma_h);
> +                item->y[1] = item->y[2] = inh / (1 << s->desc->log2_chroma_h);
>                   item->y[0] = item->y[3] = inh;
>                   inw += ctx->inputs[k]->w;
>               }
> @@ -322,7 +322,7 @@ static int config_output(AVFilterLink *outlink)
>                   return ret;
>               }
>   
> -            item->height[1] = item->height[2] = AV_CEIL_RSHIFT(inlink->h, s->desc->log2_chroma_h);
> +            item->height[1] = item->height[2] = inlink->h / (1 << s->desc->log2_chroma_h);
>               item->height[0] = item->height[3] = inlink->h;
>   
>               p2 = arg;
> @@ -370,7 +370,7 @@ static int config_output(AVFilterLink *outlink)
>                   return ret;
>               }
>   
> -            item->y[1] = item->y[2] = AV_CEIL_RSHIFT(inh, s->desc->log2_chroma_h);
> +            item->y[1] = item->y[2] = inh / (1 << s->desc->log2_chroma_h);
>               item->y[0] = item->y[3] = inh;
>   
>               width  = FFMAX(width,  inlink->w + inw);

ping

I'm pretty sure this patch is not the correct solution.
But right now there is a pretty nasty heap overflow in the filter that 
should be addressed somehow.