Re: [FFmpeg-devel] [PATCH] avfilter/framepool: use av_image_fill_plane_sizes() to calculate pool sizes

[James Almer <jamrial@gmail.com>]

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".
_______________________________________________
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".