| Message ID | 20220301142310.16367-1-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avfilter/framepool: use av_image_fill_plane_sizes() to calculate pool sizes | expand |
| Context | Check | Description |
|---|---|---|
| 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 |
On Tue, Mar 1, 2022 at 3:23 PM James Almer <jamrial@gmail.com> wrote: > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavfilter/framepool.c | 20 ++++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/libavfilter/framepool.c b/libavfilter/framepool.c > index 5b510c9af9..cf6a1d0ea0 100644 > --- a/libavfilter/framepool.c > +++ b/libavfilter/framepool.c > @@ -57,6 +57,8 @@ FFFramePool *ff_frame_pool_video_init(AVBufferRef* > (*alloc)(size_t size), > int i, ret; > FFFramePool *pool; > const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(format); > + ptrdiff_t linesizes[4]; > + size_t sizes[4]; > > if (!desc) > return NULL; > @@ -89,13 +91,19 @@ FFFramePool *ff_frame_pool_video_init(AVBufferRef* > (*alloc)(size_t size), > } > } > > - for (i = 0; i < 4 && pool->linesize[i]; i++) { > - int h = pool->height; > - if (i == 1 || i == 2) > - h = AV_CEIL_RSHIFT(h, desc->log2_chroma_h); > + for (i = 0; i < 4; i++) > + linesizes[i] = pool->linesize[i]; > > - pool->pools[i] = av_buffer_pool_init(pool->linesize[i] * h + > align, > - alloc); > + if (av_image_fill_plane_sizes(sizes, pool->format, > + FFALIGN(pool->height, align), > This is not needed for height. Original reporter never posted valgrind report of bug. > + linesizes) < 0) { > + goto fail; > + } > + > + for (i = 0; i < 4 && sizes[i]; i++) { > + if (sizes[i] > SIZE_MAX - align) > + goto fail; > + pool->pools[i] = av_buffer_pool_init(sizes[i] + align, alloc); > if (!pool->pools[i]) > goto fail; > } > -- > 2.35.1 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
On 3/1/2022 11:31 AM, Paul B Mahol wrote: > On Tue, Mar 1, 2022 at 3:23 PM James Almer <jamrial@gmail.com> wrote: > >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavfilter/framepool.c | 20 ++++++++++++++------ >> 1 file changed, 14 insertions(+), 6 deletions(-) >> >> diff --git a/libavfilter/framepool.c b/libavfilter/framepool.c >> index 5b510c9af9..cf6a1d0ea0 100644 >> --- a/libavfilter/framepool.c >> +++ b/libavfilter/framepool.c >> @@ -57,6 +57,8 @@ FFFramePool *ff_frame_pool_video_init(AVBufferRef* >> (*alloc)(size_t size), >> int i, ret; >> FFFramePool *pool; >> const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(format); >> + ptrdiff_t linesizes[4]; >> + size_t sizes[4]; >> >> if (!desc) >> return NULL; >> @@ -89,13 +91,19 @@ FFFramePool *ff_frame_pool_video_init(AVBufferRef* >> (*alloc)(size_t size), >> } >> } >> >> - for (i = 0; i < 4 && pool->linesize[i]; i++) { >> - int h = pool->height; >> - if (i == 1 || i == 2) >> - h = AV_CEIL_RSHIFT(h, desc->log2_chroma_h); >> + for (i = 0; i < 4; i++) >> + linesizes[i] = pool->linesize[i]; >> >> - pool->pools[i] = av_buffer_pool_init(pool->linesize[i] * h + >> align, >> - alloc); >> + if (av_image_fill_plane_sizes(sizes, pool->format, >> + FFALIGN(pool->height, align), >> > > This is not needed for height. av_frame_get_buffer() does it, and the lavc pool uses avcodec_align_dimensions2() to align height and achieve the same effect (Look at what it does for yuv422p). It's the only change in 17a59a634c39b00a680c6ebbaea58db95594d13d that could have generated the issue Haihao reported. > > Original reporter never posted valgrind report of bug. > > >> + linesizes) < 0) { >> + goto fail; >> + } >> + >> + for (i = 0; i < 4 && sizes[i]; i++) { >> + if (sizes[i] > SIZE_MAX - align) >> + goto fail; >> + pool->pools[i] = av_buffer_pool_init(sizes[i] + align, alloc); >> if (!pool->pools[i]) >> goto fail; >> } >> -- >> 2.35.1 >> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >> > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Tue, Mar 1, 2022 at 3:36 PM James Almer <jamrial@gmail.com> wrote: > On 3/1/2022 11:31 AM, Paul B Mahol wrote: > > On Tue, Mar 1, 2022 at 3:23 PM James Almer <jamrial@gmail.com> wrote: > > > >> Signed-off-by: James Almer <jamrial@gmail.com> > >> --- > >> libavfilter/framepool.c | 20 ++++++++++++++------ > >> 1 file changed, 14 insertions(+), 6 deletions(-) > >> > >> diff --git a/libavfilter/framepool.c b/libavfilter/framepool.c > >> index 5b510c9af9..cf6a1d0ea0 100644 > >> --- a/libavfilter/framepool.c > >> +++ b/libavfilter/framepool.c > >> @@ -57,6 +57,8 @@ FFFramePool *ff_frame_pool_video_init(AVBufferRef* > >> (*alloc)(size_t size), > >> int i, ret; > >> FFFramePool *pool; > >> const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(format); > >> + ptrdiff_t linesizes[4]; > >> + size_t sizes[4]; > >> > >> if (!desc) > >> return NULL; > >> @@ -89,13 +91,19 @@ FFFramePool *ff_frame_pool_video_init(AVBufferRef* > >> (*alloc)(size_t size), > >> } > >> } > >> > >> - for (i = 0; i < 4 && pool->linesize[i]; i++) { > >> - int h = pool->height; > >> - if (i == 1 || i == 2) > >> - h = AV_CEIL_RSHIFT(h, desc->log2_chroma_h); > >> + for (i = 0; i < 4; i++) > >> + linesizes[i] = pool->linesize[i]; > >> > >> - pool->pools[i] = av_buffer_pool_init(pool->linesize[i] * h + > >> align, > >> - alloc); > >> + if (av_image_fill_plane_sizes(sizes, pool->format, > >> + FFALIGN(pool->height, align), > >> > > > > This is not needed for height. > > av_frame_get_buffer() does it, and the lavc pool uses > avcodec_align_dimensions2() to align height and achieve the same effect > (Look at what it does for yuv422p). It's the only change in > 17a59a634c39b00a680c6ebbaea58db95594d13d that could have generated the > issue Haihao reported. > I see only victims of cargo cultism here. Filter frames are unrelated to avcodec frames. Why I can not reproduce the issue, I'm on similar intel cpu. > > > > > Original reporter never posted valgrind report of bug. > > > > > >> + linesizes) < 0) { > >> + goto fail; > >> + } > >> + > >> + for (i = 0; i < 4 && sizes[i]; i++) { > >> + if (sizes[i] > SIZE_MAX - align) > >> + goto fail; > >> + pool->pools[i] = av_buffer_pool_init(sizes[i] + align, alloc); > >> if (!pool->pools[i]) > >> goto fail; > >> } > >> -- > >> 2.35.1 > >> > >> _______________________________________________ > >> ffmpeg-devel mailing list > >> ffmpeg-devel@ffmpeg.org > >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >> > >> To unsubscribe, visit link above, or email > >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > >> > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
On 3/1/2022 11:53 AM, Paul B Mahol wrote: > On Tue, Mar 1, 2022 at 3:36 PM James Almer <jamrial@gmail.com> wrote: > >> On 3/1/2022 11:31 AM, Paul B Mahol wrote: >>> On Tue, Mar 1, 2022 at 3:23 PM James Almer <jamrial@gmail.com> wrote: >>> >>>> Signed-off-by: James Almer <jamrial@gmail.com> >>>> --- >>>> libavfilter/framepool.c | 20 ++++++++++++++------ >>>> 1 file changed, 14 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/libavfilter/framepool.c b/libavfilter/framepool.c >>>> index 5b510c9af9..cf6a1d0ea0 100644 >>>> --- a/libavfilter/framepool.c >>>> +++ b/libavfilter/framepool.c >>>> @@ -57,6 +57,8 @@ FFFramePool *ff_frame_pool_video_init(AVBufferRef* >>>> (*alloc)(size_t size), >>>> int i, ret; >>>> FFFramePool *pool; >>>> const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(format); >>>> + ptrdiff_t linesizes[4]; >>>> + size_t sizes[4]; >>>> >>>> if (!desc) >>>> return NULL; >>>> @@ -89,13 +91,19 @@ FFFramePool *ff_frame_pool_video_init(AVBufferRef* >>>> (*alloc)(size_t size), >>>> } >>>> } >>>> >>>> - for (i = 0; i < 4 && pool->linesize[i]; i++) { >>>> - int h = pool->height; >>>> - if (i == 1 || i == 2) >>>> - h = AV_CEIL_RSHIFT(h, desc->log2_chroma_h); >>>> + for (i = 0; i < 4; i++) >>>> + linesizes[i] = pool->linesize[i]; >>>> >>>> - pool->pools[i] = av_buffer_pool_init(pool->linesize[i] * h + >>>> align, >>>> - alloc); >>>> + if (av_image_fill_plane_sizes(sizes, pool->format, >>>> + FFALIGN(pool->height, align), >>>> >>> >>> This is not needed for height. >> >> av_frame_get_buffer() does it, and the lavc pool uses >> avcodec_align_dimensions2() to align height and achieve the same effect >> (Look at what it does for yuv422p). It's the only change in >> 17a59a634c39b00a680c6ebbaea58db95594d13d that could have generated the >> issue Haihao reported. >> > > I see only victims of cargo cultism here. Filter frames are unrelated to > avcodec frames. > > Why I can not reproduce the issue, I'm on similar intel cpu. What CPU do you have? He has an Ice-Lake, so av_cpu_max_align() returns 64. Unless you have one of those, or an Alder-Lake with the E-cores disabled, you'll get 32 out of it. Hardcode align to 64 in ff_default_get_video_buffer() to reproduce the failure. Also, i was wrong and aligning height makes no difference, it will still fail, so the problem is elsewhere. > > >> >>> >>> Original reporter never posted valgrind report of bug. >>> >>> >>>> + linesizes) < 0) { >>>> + goto fail; >>>> + } >>>> + >>>> + for (i = 0; i < 4 && sizes[i]; i++) { >>>> + if (sizes[i] > SIZE_MAX - align) >>>> + goto fail; >>>> + pool->pools[i] = av_buffer_pool_init(sizes[i] + align, alloc); >>>> if (!pool->pools[i]) >>>> goto fail; >>>> } >>>> -- >>>> 2.35.1 >>>> >>>> _______________________________________________ >>>> ffmpeg-devel mailing list >>>> ffmpeg-devel@ffmpeg.org >>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>>> >>>> To unsubscribe, visit link above, or email >>>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >>>> >>> _______________________________________________ >>> ffmpeg-devel mailing list >>> ffmpeg-devel@ffmpeg.org >>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>> >>> To unsubscribe, visit link above, or email >>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >> > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Tue, 2022-03-01 at 12:03 -0300, James Almer wrote: > > On 3/1/2022 11:53 AM, Paul B Mahol wrote: > > On Tue, Mar 1, 2022 at 3:36 PM James Almer <jamrial@gmail.com> wrote: > > > > > On 3/1/2022 11:31 AM, Paul B Mahol wrote: > > > > On Tue, Mar 1, 2022 at 3:23 PM James Almer <jamrial@gmail.com> wrote: > > > > > > > > > Signed-off-by: James Almer <jamrial@gmail.com> > > > > > --- > > > > > libavfilter/framepool.c | 20 ++++++++++++++------ > > > > > 1 file changed, 14 insertions(+), 6 deletions(-) > > > > > > > > > > diff --git a/libavfilter/framepool.c b/libavfilter/framepool.c > > > > > index 5b510c9af9..cf6a1d0ea0 100644 > > > > > --- a/libavfilter/framepool.c > > > > > +++ b/libavfilter/framepool.c > > > > > @@ -57,6 +57,8 @@ FFFramePool *ff_frame_pool_video_init(AVBufferRef* > > > > > (*alloc)(size_t size), > > > > > int i, ret; > > > > > FFFramePool *pool; > > > > > const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(format); > > > > > + ptrdiff_t linesizes[4]; > > > > > + size_t sizes[4]; > > > > > > > > > > if (!desc) > > > > > return NULL; > > > > > @@ -89,13 +91,19 @@ FFFramePool *ff_frame_pool_video_init(AVBufferRef* > > > > > (*alloc)(size_t size), > > > > > } > > > > > } > > > > > > > > > > - for (i = 0; i < 4 && pool->linesize[i]; i++) { > > > > > - int h = pool->height; > > > > > - if (i == 1 || i == 2) > > > > > - h = AV_CEIL_RSHIFT(h, desc->log2_chroma_h); > > > > > + for (i = 0; i < 4; i++) > > > > > + linesizes[i] = pool->linesize[i]; > > > > > > > > > > - pool->pools[i] = av_buffer_pool_init(pool->linesize[i] * h + > > > > > align, > > > > > - alloc); > > > > > + if (av_image_fill_plane_sizes(sizes, pool->format, > > > > > + FFALIGN(pool->height, align), > > > > > > > > > > > > > This is not needed for height. > > > > > > av_frame_get_buffer() does it, and the lavc pool uses > > > avcodec_align_dimensions2() to align height and achieve the same effect > > > (Look at what it does for yuv422p). It's the only change in > > > 17a59a634c39b00a680c6ebbaea58db95594d13d that could have generated the > > > issue Haihao reported. > > > > > > > I see only victims of cargo cultism here. Filter frames are unrelated to > > avcodec frames. > > > > Why I can not reproduce the issue, I'm on similar intel cpu. > > What CPU do you have? He has an Ice-Lake, so av_cpu_max_align() returns > 64. Unless you have one of those, or an Alder-Lake with the E-cores > disabled, you'll get 32 out of it. Yes, I experienced this issue with Ice Lake, and I can't reproduce this issue with another CPU in which av_cpu_max_align() returns 32. > > Hardcode align to 64 in ff_default_get_video_buffer() to reproduce the > failure. Also, i was wrong and aligning height makes no difference, it > will still fail, so the problem is elsewhere. > This patch doesn't work for me too. Thanks Haihao > > > > > > > > > > > > > > > Original reporter never posted valgrind report of bug. > > > > > > > > > > > > > + linesizes) < 0) { > > > > > + goto fail; > > > > > + } > > > > > + > > > > > + for (i = 0; i < 4 && sizes[i]; i++) { > > > > > + if (sizes[i] > SIZE_MAX - align) > > > > > + goto fail; > > > > > + pool->pools[i] = av_buffer_pool_init(sizes[i] + align, > > > > > alloc); > > > > > if (!pool->pools[i]) > > > > > goto fail; > > > > > } > > > > > -- > > > > > 2.35.1 > > > > > > > > > > _______________________________________________ > > > > > ffmpeg-devel mailing list > > > > > ffmpeg-devel@ffmpeg.org > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > > > > > > > > _______________________________________________ > > > > ffmpeg-devel mailing list > > > > ffmpeg-devel@ffmpeg.org > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > To unsubscribe, visit link above, or email > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > _______________________________________________ > > > ffmpeg-devel mailing list > > > ffmpeg-devel@ffmpeg.org > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > To unsubscribe, visit link above, or email > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On 3/1/2022 11:23 AM, James Almer wrote: > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavfilter/framepool.c | 20 ++++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/libavfilter/framepool.c b/libavfilter/framepool.c > index 5b510c9af9..cf6a1d0ea0 100644 > --- a/libavfilter/framepool.c > +++ b/libavfilter/framepool.c > @@ -57,6 +57,8 @@ FFFramePool *ff_frame_pool_video_init(AVBufferRef* (*alloc)(size_t size), > int i, ret; > FFFramePool *pool; > const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(format); > + ptrdiff_t linesizes[4]; > + size_t sizes[4]; > > if (!desc) > return NULL; > @@ -89,13 +91,19 @@ FFFramePool *ff_frame_pool_video_init(AVBufferRef* (*alloc)(size_t size), > } > } > > - for (i = 0; i < 4 && pool->linesize[i]; i++) { > - int h = pool->height; > - if (i == 1 || i == 2) > - h = AV_CEIL_RSHIFT(h, desc->log2_chroma_h); > + for (i = 0; i < 4; i++) > + linesizes[i] = pool->linesize[i]; > > - pool->pools[i] = av_buffer_pool_init(pool->linesize[i] * h + align, > - alloc); > + if (av_image_fill_plane_sizes(sizes, pool->format, > + FFALIGN(pool->height, align), > + linesizes) < 0) { > + goto fail; > + } > + > + for (i = 0; i < 4 && sizes[i]; i++) { > + if (sizes[i] > SIZE_MAX - align) > + goto fail; > + pool->pools[i] = av_buffer_pool_init(sizes[i] + align, alloc); > if (!pool->pools[i]) > goto fail; > } Ping. I can also remove the height padding if preferred.
On 3/3/22, James Almer <jamrial@gmail.com> wrote: > > > On 3/1/2022 11:23 AM, James Almer wrote: >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavfilter/framepool.c | 20 ++++++++++++++------ >> 1 file changed, 14 insertions(+), 6 deletions(-) >> >> diff --git a/libavfilter/framepool.c b/libavfilter/framepool.c >> index 5b510c9af9..cf6a1d0ea0 100644 >> --- a/libavfilter/framepool.c >> +++ b/libavfilter/framepool.c >> @@ -57,6 +57,8 @@ FFFramePool *ff_frame_pool_video_init(AVBufferRef* >> (*alloc)(size_t size), >> int i, ret; >> FFFramePool *pool; >> const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(format); >> + ptrdiff_t linesizes[4]; >> + size_t sizes[4]; >> >> if (!desc) >> return NULL; >> @@ -89,13 +91,19 @@ FFFramePool *ff_frame_pool_video_init(AVBufferRef* >> (*alloc)(size_t size), >> } >> } >> >> - for (i = 0; i < 4 && pool->linesize[i]; i++) { >> - int h = pool->height; >> - if (i == 1 || i == 2) >> - h = AV_CEIL_RSHIFT(h, desc->log2_chroma_h); >> + for (i = 0; i < 4; i++) >> + linesizes[i] = pool->linesize[i]; >> >> - pool->pools[i] = av_buffer_pool_init(pool->linesize[i] * h + >> align, >> - alloc); >> + if (av_image_fill_plane_sizes(sizes, pool->format, >> + FFALIGN(pool->height, align), >> + linesizes) < 0) { >> + goto fail; >> + } >> + >> + for (i = 0; i < 4 && sizes[i]; i++) { >> + if (sizes[i] > SIZE_MAX - align) >> + goto fail; >> + pool->pools[i] = av_buffer_pool_init(sizes[i] + align, alloc); >> if (!pool->pools[i]) >> goto fail; >> } > > Ping. I can also remove the height padding if preferred. It does not work. No? > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
On 3/3/2022 10:56 AM, Paul B Mahol wrote: > On 3/3/22, James Almer <jamrial@gmail.com> wrote: >> >> >> On 3/1/2022 11:23 AM, James Almer wrote: >>> Signed-off-by: James Almer <jamrial@gmail.com> >>> --- >>> libavfilter/framepool.c | 20 ++++++++++++++------ >>> 1 file changed, 14 insertions(+), 6 deletions(-) >>> >>> diff --git a/libavfilter/framepool.c b/libavfilter/framepool.c >>> index 5b510c9af9..cf6a1d0ea0 100644 >>> --- a/libavfilter/framepool.c >>> +++ b/libavfilter/framepool.c >>> @@ -57,6 +57,8 @@ FFFramePool *ff_frame_pool_video_init(AVBufferRef* >>> (*alloc)(size_t size), >>> int i, ret; >>> FFFramePool *pool; >>> const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(format); >>> + ptrdiff_t linesizes[4]; >>> + size_t sizes[4]; >>> >>> if (!desc) >>> return NULL; >>> @@ -89,13 +91,19 @@ FFFramePool *ff_frame_pool_video_init(AVBufferRef* >>> (*alloc)(size_t size), >>> } >>> } >>> >>> - for (i = 0; i < 4 && pool->linesize[i]; i++) { >>> - int h = pool->height; >>> - if (i == 1 || i == 2) >>> - h = AV_CEIL_RSHIFT(h, desc->log2_chroma_h); >>> + for (i = 0; i < 4; i++) >>> + linesizes[i] = pool->linesize[i]; >>> >>> - pool->pools[i] = av_buffer_pool_init(pool->linesize[i] * h + >>> align, >>> - alloc); >>> + if (av_image_fill_plane_sizes(sizes, pool->format, >>> + FFALIGN(pool->height, align), >>> + linesizes) < 0) { >>> + goto fail; >>> + } >>> + >>> + for (i = 0; i < 4 && sizes[i]; i++) { >>> + if (sizes[i] > SIZE_MAX - align) >>> + goto fail; >>> + pool->pools[i] = av_buffer_pool_init(sizes[i] + align, alloc); >>> if (!pool->pools[i]) >>> goto fail; >>> } >> >> Ping. I can also remove the height padding if preferred. > > It does not work. No? It doesn't fix the mpeg encoder issue with yuv422p streams and 64 stride alignment, no, but it's not strictly about that either. It's a simplification using existing helpers. > >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >> > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On 3/3/22, James Almer <jamrial@gmail.com> wrote: > > > On 3/3/2022 10:56 AM, Paul B Mahol wrote: >> On 3/3/22, James Almer <jamrial@gmail.com> wrote: >>> >>> >>> On 3/1/2022 11:23 AM, James Almer wrote: >>>> Signed-off-by: James Almer <jamrial@gmail.com> >>>> --- >>>> libavfilter/framepool.c | 20 ++++++++++++++------ >>>> 1 file changed, 14 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/libavfilter/framepool.c b/libavfilter/framepool.c >>>> index 5b510c9af9..cf6a1d0ea0 100644 >>>> --- a/libavfilter/framepool.c >>>> +++ b/libavfilter/framepool.c >>>> @@ -57,6 +57,8 @@ FFFramePool *ff_frame_pool_video_init(AVBufferRef* >>>> (*alloc)(size_t size), >>>> int i, ret; >>>> FFFramePool *pool; >>>> const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(format); >>>> + ptrdiff_t linesizes[4]; >>>> + size_t sizes[4]; >>>> >>>> if (!desc) >>>> return NULL; >>>> @@ -89,13 +91,19 @@ FFFramePool *ff_frame_pool_video_init(AVBufferRef* >>>> (*alloc)(size_t size), >>>> } >>>> } >>>> >>>> - for (i = 0; i < 4 && pool->linesize[i]; i++) { >>>> - int h = pool->height; >>>> - if (i == 1 || i == 2) >>>> - h = AV_CEIL_RSHIFT(h, desc->log2_chroma_h); >>>> + for (i = 0; i < 4; i++) >>>> + linesizes[i] = pool->linesize[i]; >>>> >>>> - pool->pools[i] = av_buffer_pool_init(pool->linesize[i] * h + >>>> align, >>>> - alloc); >>>> + if (av_image_fill_plane_sizes(sizes, pool->format, >>>> + FFALIGN(pool->height, align), >>>> + linesizes) < 0) { >>>> + goto fail; >>>> + } >>>> + >>>> + for (i = 0; i < 4 && sizes[i]; i++) { >>>> + if (sizes[i] > SIZE_MAX - align) >>>> + goto fail; >>>> + pool->pools[i] = av_buffer_pool_init(sizes[i] + align, alloc); >>>> if (!pool->pools[i]) >>>> goto fail; >>>> } >>> >>> Ping. I can also remove the height padding if preferred. >> >> It does not work. No? > > It doesn't fix the mpeg encoder issue with yuv422p streams and 64 stride > alignment, no, but it's not strictly about that either. It's a > simplification using existing helpers. Ok, if you need it. > >> >>> _______________________________________________ >>> ffmpeg-devel mailing list >>> ffmpeg-devel@ffmpeg.org >>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>> >>> To unsubscribe, visit link above, or email >>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >>> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
On 3/3/2022 1:46 PM, Paul B Mahol wrote: > On 3/3/22, James Almer <jamrial@gmail.com> wrote: >> >> >> On 3/3/2022 10:56 AM, Paul B Mahol wrote: >>> On 3/3/22, James Almer <jamrial@gmail.com> wrote: >>>> >>>> >>>> On 3/1/2022 11:23 AM, James Almer wrote: >>>>> Signed-off-by: James Almer <jamrial@gmail.com> >>>>> --- >>>>> libavfilter/framepool.c | 20 ++++++++++++++------ >>>>> 1 file changed, 14 insertions(+), 6 deletions(-) >>>>> >>>>> diff --git a/libavfilter/framepool.c b/libavfilter/framepool.c >>>>> index 5b510c9af9..cf6a1d0ea0 100644 >>>>> --- a/libavfilter/framepool.c >>>>> +++ b/libavfilter/framepool.c >>>>> @@ -57,6 +57,8 @@ FFFramePool *ff_frame_pool_video_init(AVBufferRef* >>>>> (*alloc)(size_t size), >>>>> int i, ret; >>>>> FFFramePool *pool; >>>>> const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(format); >>>>> + ptrdiff_t linesizes[4]; >>>>> + size_t sizes[4]; >>>>> >>>>> if (!desc) >>>>> return NULL; >>>>> @@ -89,13 +91,19 @@ FFFramePool *ff_frame_pool_video_init(AVBufferRef* >>>>> (*alloc)(size_t size), >>>>> } >>>>> } >>>>> >>>>> - for (i = 0; i < 4 && pool->linesize[i]; i++) { >>>>> - int h = pool->height; >>>>> - if (i == 1 || i == 2) >>>>> - h = AV_CEIL_RSHIFT(h, desc->log2_chroma_h); >>>>> + for (i = 0; i < 4; i++) >>>>> + linesizes[i] = pool->linesize[i]; >>>>> >>>>> - pool->pools[i] = av_buffer_pool_init(pool->linesize[i] * h + >>>>> align, >>>>> - alloc); >>>>> + if (av_image_fill_plane_sizes(sizes, pool->format, >>>>> + FFALIGN(pool->height, align), >>>>> + linesizes) < 0) { >>>>> + goto fail; >>>>> + } >>>>> + >>>>> + for (i = 0; i < 4 && sizes[i]; i++) { >>>>> + if (sizes[i] > SIZE_MAX - align) >>>>> + goto fail; >>>>> + pool->pools[i] = av_buffer_pool_init(sizes[i] + align, alloc); >>>>> if (!pool->pools[i]) >>>>> goto fail; >>>>> } >>>> >>>> Ping. I can also remove the height padding if preferred. >>> >>> It does not work. No? >> >> It doesn't fix the mpeg encoder issue with yuv422p streams and 64 stride >> alignment, no, but it's not strictly about that either. It's a >> simplification using existing helpers. > > Ok, if you need it. Applied without the vertical padding, to not change the behavior your introduced in 17a59a634c.
diff --git a/libavfilter/framepool.c b/libavfilter/framepool.c index 5b510c9af9..cf6a1d0ea0 100644 --- a/libavfilter/framepool.c +++ b/libavfilter/framepool.c @@ -57,6 +57,8 @@ FFFramePool *ff_frame_pool_video_init(AVBufferRef* (*alloc)(size_t size), int i, ret; FFFramePool *pool; const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(format); + ptrdiff_t linesizes[4]; + size_t sizes[4]; if (!desc) return NULL; @@ -89,13 +91,19 @@ FFFramePool *ff_frame_pool_video_init(AVBufferRef* (*alloc)(size_t size), } } - for (i = 0; i < 4 && pool->linesize[i]; i++) { - int h = pool->height; - if (i == 1 || i == 2) - h = AV_CEIL_RSHIFT(h, desc->log2_chroma_h); + for (i = 0; i < 4; i++) + linesizes[i] = pool->linesize[i]; - pool->pools[i] = av_buffer_pool_init(pool->linesize[i] * h + align, - alloc); + if (av_image_fill_plane_sizes(sizes, pool->format, + FFALIGN(pool->height, align), + linesizes) < 0) { + goto fail; + } + + for (i = 0; i < 4 && sizes[i]; i++) { + if (sizes[i] > SIZE_MAX - align) + goto fail; + pool->pools[i] = av_buffer_pool_init(sizes[i] + align, alloc); if (!pool->pools[i]) goto fail; }
Signed-off-by: James Almer <jamrial@gmail.com> --- libavfilter/framepool.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)