[FFmpeg-devel] [PATCH] avfilter/framepool: fix alignment requirements for audio and video filters

James Almer jamrial at gmail.com
Mon Feb 21 14:08:54 EET 2022



On 2/21/2022 8:33 AM, Paul B Mahol wrote:
> On Mon, Feb 21, 2022 at 2:54 AM James Almer <jamrial at gmail.com> wrote:
> 
>>
>>
>> On 2/20/2022 7:02 PM, Paul B Mahol wrote:
>>> Signed-off-by: Paul B Mahol <onemda at gmail.com>
>>> ---
>>>    libavfilter/audio.c     | 11 +++++------
>>>    libavfilter/framepool.c | 16 ++++++----------
>>>    libavfilter/video.c     | 11 +++++------
>>>    3 files changed, 16 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/libavfilter/audio.c b/libavfilter/audio.c
>>> index cebc9709dd..a0408226a3 100644
>>> --- a/libavfilter/audio.c
>>> +++ b/libavfilter/audio.c
>>> @@ -22,15 +22,13 @@
>>>    #include "libavutil/avassert.h"
>>>    #include "libavutil/channel_layout.h"
>>>    #include "libavutil/common.h"
>>> +#include "libavutil/cpu.h"
>>>
>>>    #include "audio.h"
>>>    #include "avfilter.h"
>>>    #include "framepool.h"
>>>    #include "internal.h"
>>>
>>> -#define BUFFER_ALIGN 0
>>> -
>>> -
>>>    AVFrame *ff_null_get_audio_buffer(AVFilterLink *link, int nb_samples)
>>>    {
>>>        return ff_get_audio_buffer(link->dst->outputs[0], nb_samples);
>>> @@ -41,12 +39,13 @@ AVFrame *ff_default_get_audio_buffer(AVFilterLink
>> *link, int nb_samples)
>>>        AVFrame *frame = NULL;
>>>        int channels = link->channels;
>>>        int channel_layout_nb_channels =
>> av_get_channel_layout_nb_channels(link->channel_layout);
>>> +    int align = av_cpu_max_align();
>>>
>>>        av_assert0(channels == channel_layout_nb_channels ||
>> !channel_layout_nb_channels);
>>>
>>>        if (!link->frame_pool) {
>>>            link->frame_pool = ff_frame_pool_audio_init(av_buffer_allocz,
>> channels,
>>> -                                                    nb_samples,
>> link->format, BUFFER_ALIGN);
>>> +                                                    nb_samples,
>> link->format, align);
>>>            if (!link->frame_pool)
>>>                return NULL;
>>>        } else {
>>> @@ -62,11 +61,11 @@ AVFrame *ff_default_get_audio_buffer(AVFilterLink
>> *link, int nb_samples)
>>>            }
>>>
>>>            if (pool_channels != channels || pool_nb_samples < nb_samples
>> ||
>>> -            pool_format != link->format || pool_align != BUFFER_ALIGN) {
>>> +            pool_format != link->format || pool_align != align) {
>>>
>>>                ff_frame_pool_uninit((FFFramePool **)&link->frame_pool);
>>>                link->frame_pool =
>> ff_frame_pool_audio_init(av_buffer_allocz, channels,
>>> -                                                        nb_samples,
>> link->format, BUFFER_ALIGN);
>>> +                                                        nb_samples,
>> link->format, align);
>>>                if (!link->frame_pool)
>>>                    return NULL;
>>>            }
>>> diff --git a/libavfilter/framepool.c b/libavfilter/framepool.c
>>> index 7c63807df3..ebfd5dc0e0 100644
>>> --- a/libavfilter/framepool.c
>>> +++ b/libavfilter/framepool.c
>>> @@ -76,14 +76,10 @@ FFFramePool *ff_frame_pool_video_init(AVBufferRef*
>> (*alloc)(size_t size),
>>>        }
>>>
>>>        if (!pool->linesize[0]) {
>>> -        for(i = 1; i <= align; i += i) {
>>> -            ret = av_image_fill_linesizes(pool->linesize, pool->format,
>>> -                                          FFALIGN(pool->width, i));
>>> -            if (ret < 0) {
>>> -                goto fail;
>>> -            }
>>> -            if (!(pool->linesize[0] & (pool->align - 1)))
>>> -                break;
>>> +        ret = av_image_fill_linesizes(pool->linesize, pool->format,
>>> +                                      FFALIGN(pool->width, align));
>>> +        if (ret < 0) {
>>> +            goto fail;
>>>            }
>>>
>>>            for (i = 0; i < 4 && pool->linesize[i]; i++) {
>>> @@ -92,11 +88,11 @@ FFFramePool *ff_frame_pool_video_init(AVBufferRef*
>> (*alloc)(size_t size),
>>>        }
>>>
>>>        for (i = 0; i < 4 && pool->linesize[i]; i++) {
>>> -        int h = FFALIGN(pool->height, 32);
>>> +        int h = pool->height;
>>>            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,
>>
>> Why are you removing the padding on each plane? Just add align padding
>> bytes and it should be good.
>>
> 
> linesize[i]  is already aligned, why would there be need for additional
> align at end of buffer?

FFALIGN(pool->width, align) can be pool->width. If so, then there would 
be no padding at all.

> 
> 
>>>                                                 alloc);
>>>            if (!pool->pools[i])
>>>                goto fail;
>>> diff --git a/libavfilter/video.c b/libavfilter/video.c
>>> index 7ef04144e4..fa3d588044 100644
>>> --- a/libavfilter/video.c
>>> +++ b/libavfilter/video.c
>>> @@ -24,6 +24,7 @@
>>>    #include <stdio.h>
>>>
>>>    #include "libavutil/buffer.h"
>>> +#include "libavutil/cpu.h"
>>>    #include "libavutil/hwcontext.h"
>>>    #include "libavutil/imgutils.h"
>>>
>>> @@ -32,9 +33,6 @@
>>>    #include "internal.h"
>>>    #include "video.h"
>>>
>>> -#define BUFFER_ALIGN 32
>>> -
>>> -
>>>    AVFrame *ff_null_get_video_buffer(AVFilterLink *link, int w, int h)
>>>    {
>>>        return ff_get_video_buffer(link->dst->outputs[0], w, h);
>>> @@ -46,6 +44,7 @@ AVFrame *ff_default_get_video_buffer(AVFilterLink
>> *link, int w, int h)
>>>        int pool_width = 0;
>>>        int pool_height = 0;
>>>        int pool_align = 0;
>>> +    int align = av_cpu_max_align();
>>>        enum AVPixelFormat pool_format = AV_PIX_FMT_NONE;
>>>
>>>        if (link->hw_frames_ctx &&
>>> @@ -65,7 +64,7 @@ AVFrame *ff_default_get_video_buffer(AVFilterLink
>> *link, int w, int h)
>>>
>>>        if (!link->frame_pool) {
>>>            link->frame_pool = ff_frame_pool_video_init(av_buffer_allocz,
>> w, h,
>>> -                                                    link->format,
>> BUFFER_ALIGN);
>>> +                                                    link->format,
>> align);
>>>            if (!link->frame_pool)
>>>                return NULL;
>>>        } else {
>>> @@ -76,11 +75,11 @@ AVFrame *ff_default_get_video_buffer(AVFilterLink
>> *link, int w, int h)
>>>            }
>>>
>>>            if (pool_width != w || pool_height != h ||
>>> -            pool_format != link->format || pool_align != BUFFER_ALIGN) {
>>> +            pool_format != link->format || pool_align != align) {
>>>
>>>                ff_frame_pool_uninit((FFFramePool **)&link->frame_pool);
>>>                link->frame_pool =
>> ff_frame_pool_video_init(av_buffer_allocz, w, h,
>>> -                                                        link->format,
>> BUFFER_ALIGN);
>>> +                                                        link->format,
>> align);
>>>                if (!link->frame_pool)
>>>                    return NULL;
>>>            }
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".


More information about the ffmpeg-devel mailing list