| Message ID | 20240323153026.1848-1-timo@rothenpieler.org |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avfilter/vf_stack: round down internal item heights | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
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.
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);