diff mbox series

[FFmpeg-devel] avutil/frame: use the maximum compile time supported alignment for strides

Message ID 20240731125523.63406-1-jamrial@gmail.com
State New
Headers show
Series [FFmpeg-devel] avutil/frame: use the maximum compile time supported alignment for strides | expand

Checks

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

Commit Message

James Almer July 31, 2024, 12:55 p.m. UTC
This puts lavu frame buffer allocator helpers in sync with lavc's decoder frame
buffer allocator's STRIDE_ALIGN define.

Remove the comment about av_cpu_max_align() while at it as using it is not
ideal when CPU flags can be changed mid process.

Should fix ticket #11116.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavutil/frame.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

James Almer Aug. 2, 2024, 7:07 p.m. UTC | #1
On 7/31/2024 9:55 AM, James Almer wrote:
> This puts lavu frame buffer allocator helpers in sync with lavc's decoder frame
> buffer allocator's STRIDE_ALIGN define.
> 
> Remove the comment about av_cpu_max_align() while at it as using it is not
> ideal when CPU flags can be changed mid process.
> 
> Should fix ticket #11116.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>   libavutil/frame.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index 673a9afb3b..5cbfc6a48b 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -166,6 +166,8 @@ void av_frame_free(AVFrame **frame)
>       av_freep(frame);
>   }
>   
> +#define ALIGN (HAVE_SIMD_ALIGN_64 ? 64 : 32)
> +
>   static int get_video_buffer(AVFrame *frame, int align)
>   {
>       const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(frame->format);
> @@ -182,7 +184,7 @@ static int get_video_buffer(AVFrame *frame, int align)
>   
>       if (!frame->linesize[0]) {
>           if (align <= 0)
> -            align = 32; /* STRIDE_ALIGN. Should be av_cpu_max_align() */
> +            align = ALIGN;
>   
>           for (int i = 1; i <= align; i += i) {
>               ret = av_image_fill_linesizes(frame->linesize, frame->format,

Will apply.
Rémi Denis-Courmont Aug. 3, 2024, 6:34 a.m. UTC | #2
Le keskiviikkona 31. heinäkuuta 2024, 15.55.23 EEST James Almer a écrit :
> This puts lavu frame buffer allocator helpers in sync with lavc's decoder
> frame buffer allocator's STRIDE_ALIGN define.

STRIDE_ALIGN can go down to 16 or even 8 bytes though. What is the reason for 
capping at 32 bytes here?

(Alternatively, cache line size is probably 64 bytes almost everywhere.)
James Almer Aug. 3, 2024, 11:47 a.m. UTC | #3
On 8/3/2024 3:34 AM, Rémi Denis-Courmont wrote:
> Le keskiviikkona 31. heinäkuuta 2024, 15.55.23 EEST James Almer a écrit :
>> This puts lavu frame buffer allocator helpers in sync with lavc's decoder
>> frame buffer allocator's STRIDE_ALIGN define.
> 
> STRIDE_ALIGN can go down to 16 or even 8 bytes though. What is the reason for
> capping at 32 bytes here?

I'd rather not reduce the alignment, but no strong feelings about it anyway.

> 
> (Alternatively, cache line size is probably 64 bytes almost everywhere.)
>
diff mbox series

Patch

diff --git a/libavutil/frame.c b/libavutil/frame.c
index 673a9afb3b..5cbfc6a48b 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -166,6 +166,8 @@  void av_frame_free(AVFrame **frame)
     av_freep(frame);
 }
 
+#define ALIGN (HAVE_SIMD_ALIGN_64 ? 64 : 32)
+
 static int get_video_buffer(AVFrame *frame, int align)
 {
     const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(frame->format);
@@ -182,7 +184,7 @@  static int get_video_buffer(AVFrame *frame, int align)
 
     if (!frame->linesize[0]) {
         if (align <= 0)
-            align = 32; /* STRIDE_ALIGN. Should be av_cpu_max_align() */
+            align = ALIGN;
 
         for (int i = 1; i <= align; i += i) {
             ret = av_image_fill_linesizes(frame->linesize, frame->format,