On Sat, Nov 9, 2024 at 10:31 AM Pavel Koshevoy <pkoshe...@gmail.com> wrote:
> > > On Sat, Nov 9, 2024 at 9:53 AM James Almer <jamr...@gmail.com> wrote: > >> On 11/9/2024 12:55 PM, Pavel Koshevoy wrote: >> > On Sat, Nov 9, 2024 at 6:22 AM James Almer <jamr...@gmail.com> wrote: >> > >> >> On 11/9/2024 1:40 AM, Pavel Koshevoy wrote: >> >>> On Fri, Nov 8, 2024 at 7:29 PM James Almer <jamr...@gmail.com> wrote: >> >>> >> >>>> On 11/8/2024 10:51 PM, Pavel Koshevoy wrote: >> >>>>> I ran into segfaults in zimg when I attempted to use zscale >> >>>>> to convert a 512x538@yuv444p16le(tv) image from HLG to Bt.709 >> >>>>> with this filter chain: >> >>>>> >> >>>>> >> >>>> >> >> >> buffer=width=512:height=538:pix_fmt=yuv444p16le:time_base=1/1:sar=1/1,zscale=min=2020_ncl:rin=limited:pin=2020:tin=arib-std-b67:cin=left:t=linear,format=gbrpf32le,tonemap=gamma:desat=0,zscale=tin=linear:npl=100:p=709:m=709:r=limited:t=709,format=pix_fmts=yuv444p16le,buffersink >> >>>>> >> >>>>> I found several issues: >> >>>>> - realign_frame called av_pix_fmt_count_planes with incorrect >> >> parameter. >> >>>>> - av_frame_get_buffer did not align data pointers to specified >> >> alignment. >> >>>> >> >>>> Because it's not supposed to. The align parameter doxy states "buffer >> >>>> size alignment", which is the result of aligning the >> stride/linesizes, >> >>>> not the data pointers. >> >>>> >> >>>> You may want to use ff_default_get_video_buffer2(), which will add >> align >> >>>> amount of bytes to the allocated buffers (On top of aligning the >> >>>> linesizes to it), and then align the pointers with FFALIGN(). >> >>>> >> >>> >> >>> I am not the maintainer of vf_zscale, and I am not intimately familiar >> >> with >> >>> private ffmpeg APIs such as ff_default_get_video_buffer2, and at first >> >>> glance that function >> >>> doesn't appear to be a drop-in replacement for av_frame_get_buffer. >> >>> >> >>> ff_default_get_video_buffer2 requires AVFilterLink parameter?! -- >> >>> realign_frame doesn't >> >>> have that, it has an AVFrame which it needs to re-align to make it >> >>> compatible with libzimg API. >> >> >> >> It's trivial to make realign_frame take the necessary AVFilterLink as >> >> input argument. >> >> >> >>> >> >>> ... and why isn't av_frame_get_buffer populating AVFrame.data pointers >> >>> aligned >> >>> as specified by the align parameter? It costs at most (align - 1) more >> >>> padding bytes >> >>> to make it align the pointers, so I don't understand why it doesn't do >> >> that. >> >> >> >> Buffer alignment is set at configure time. It will be set to the >> highest >> >> needed alignment for the enabled simd (64 if avx512, else 32 if avx, >> >> else 16 if neon/sse, else 8). This is handled by av_malloc(), which is >> >> ultimately used by all alloc functions. >> >> >> > >> > Yes, I have noticed this while stepping through ffmpeg and zimg code. >> > The surprising thing I've observed is that ff_get_cpu_max_align_x86() >> > (called from av_cpu_max_align()) returned 8 ... it's surprising for an >> > ffmpeg >> > built and running on a Ryzen R9 5950x -- I would have expected 32. >> >> Yeah, av_cpu_max_align() returns a value depending on runtime >> parameters, so unless you force cpuflags using av_force_cpu_flags() to >> disable everything SSE and above (or it being disabled during >> configure), it should not return 8. >> > > I certainly did not call av_force_cpu_flags myself, and as far as I can > see > conan ffmpeg recipe does not pass any --disable-sse or --disable-avx > options > to ffmpegs configure script ... I will have to investigate this separately > when I am back at work. > > > > >> > >> > As a side note ... this ffmpeg build (and zimg build) were produced by >> > conan, >> > so perhaps the conan recipe for ffmpeg needs some changes to make >> > av_cpu_max_align() work as expected on 5950x. >> > (https://conan.io/center/recipes/ffmpeg) >> >> The conan recipe for ffmpeg adds --disable-mmx to ffmpeg configure options when building a Debug package. Unfortunately, this disabled not only MMX, but also SSE and AVX, and affected av_cpu_max_align(). I've modified my local ffmpeg conan recipe to omit the --disable-mmx option from Debug builds. Pavel. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
