| Message ID | 20220218104605.105011-1-onemda@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avfilter/framepool: fix adjustment that can crash filtering | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_aarch64_jetson | success | Make finished |
| andriy/make_fate_aarch64_jetson | success | Make fate finished |
| andriy/make_armv7_RPi4 | success | Make finished |
| andriy/make_fate_armv7_RPi4 | success | Make fate finished |
Quoting Paul B Mahol (2022-02-18 11:46:05) > Fixes #9551. > > Signed-off-by: Paul B Mahol <onemda@gmail.com> > --- > libavfilter/framepool.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavfilter/framepool.c b/libavfilter/framepool.c > index 7c63807df3..aab408d355 100644 > --- a/libavfilter/framepool.c > +++ b/libavfilter/framepool.c > @@ -96,7 +96,7 @@ FFFramePool *ff_frame_pool_video_init(AVBufferRef* (*alloc)(size_t size), > if (i == 1 || i == 2) > h = AV_CEIL_RSHIFT(h, desc->log2_chroma_h); > > - pool->pools[i] = av_buffer_pool_init(pool->linesize[i] * h + 16 + 16 - 1, > + pool->pools[i] = av_buffer_pool_init(pool->linesize[i] * h + 16 + 16 - 0, > alloc); all these magic constants are extremely non-obvious, why are they there and why does removing that 1 crash anything?
On 2/20/2022 1:15 PM, Anton Khirnov wrote: > Quoting Paul B Mahol (2022-02-18 11:46:05) >> Fixes #9551. >> >> Signed-off-by: Paul B Mahol <onemda@gmail.com> >> --- >> libavfilter/framepool.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libavfilter/framepool.c b/libavfilter/framepool.c >> index 7c63807df3..aab408d355 100644 >> --- a/libavfilter/framepool.c >> +++ b/libavfilter/framepool.c >> @@ -96,7 +96,7 @@ FFFramePool *ff_frame_pool_video_init(AVBufferRef* (*alloc)(size_t size), >> if (i == 1 || i == 2) >> h = AV_CEIL_RSHIFT(h, desc->log2_chroma_h); >> >> - pool->pools[i] = av_buffer_pool_init(pool->linesize[i] * h + 16 + 16 - 1, >> + pool->pools[i] = av_buffer_pool_init(pool->linesize[i] * h + 16 + 16 - 0, >> alloc); > > all these magic constants are extremely non-obvious, why are they there > and why does removing that 1 crash anything? They are probably cargo culting from lavu's av_frame_get_buffer() and in turn from lavc's avcodec_default_get_buffer2(). In the latter, the padding is 16 + STRIDE_ALIGN - 1, where STRIDE_ALIGN is the size of the highest simd register enabled at configure time (16 for sse/neon/altivec, 32 for avx, 64 for avx512). In the former, the padding is 16 + 16 - 1, with a comment in one of those 16 that it's meant to be STRIDE_ALIGN, a lavc internal define, which means it's outdated and out of sync. What the first 16 or the -1 are, i have no idea.
diff --git a/libavfilter/framepool.c b/libavfilter/framepool.c index 7c63807df3..aab408d355 100644 --- a/libavfilter/framepool.c +++ b/libavfilter/framepool.c @@ -96,7 +96,7 @@ FFFramePool *ff_frame_pool_video_init(AVBufferRef* (*alloc)(size_t size), if (i == 1 || i == 2) h = AV_CEIL_RSHIFT(h, desc->log2_chroma_h); - pool->pools[i] = av_buffer_pool_init(pool->linesize[i] * h + 16 + 16 - 1, + pool->pools[i] = av_buffer_pool_init(pool->linesize[i] * h + 16 + 16 - 0, alloc); if (!pool->pools[i]) goto fail;
Fixes #9551. Signed-off-by: Paul B Mahol <onemda@gmail.com> --- libavfilter/framepool.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)