[FFmpeg-devel] [PATCH] lavu/get_video_buffer: also align data pointers (v2)

[Pavel Koshevoy]

On Sat, Nov 16, 2024 at 9:25 AM Pavel Koshevoy <pkoshevoy at gmail.com> wrote:

>
>
> On Sat, Nov 16, 2024 at 9:00 AM James Almer <jamrial at gmail.com> wrote:
>
>> On 11/16/2024 12:57 PM, Pavel Koshevoy wrote:
>> > On Fri, Nov 15, 2024 at 9:25 PM Anton Khirnov <anton at khirnov.net>
>> wrote:
>> >
>> >> Quoting Pavel Koshevoy (2024-11-15 20:32:21)
>> >>> This avoids unpleasant surprises to av_frame_get_buffer callers
>> >>> that explicitly specified 64-byte alignment and didn't get
>> >>> AVFrame.data pointers that are 64-byte aligned.
>> >>>
>> >>> For example, see https://github.com/sekrit-twc/zimg/issues/212
>> >>>
>> >>> Although the zscale issue has already been resolved by other means
>> >>> it would still be prudent to improve the behavior of
>> av_frame_get_buffer
>> >>> to fix any unknown and future instances of similar issues.
>> >>> ---
>> >>>   libavutil/frame.c | 4 +++-
>> >>>   libavutil/frame.h | 7 ++++---
>> >>>   2 files changed, 7 insertions(+), 4 deletions(-)
>> >>
>> >> I think this warrants a micro bump and an APIchanges entry, so external
>> >> callers can reliably depend on this.
>> >>
>> >> Otherwise looks good.
>> >>
>> >> --
>> >> Anton Khirnov
>> >>
>> >
>> >
>> > Thank you, I've submitted a v3 of the patch with doc/APIchanges entry
>> added
>> > and LIBAVUTIL_VERSION_MICRO bumped to 101.
>> >
>> > BTW, I did not make any changes to get_audio_buffer which also accepts
>> > an explicit alignment parameter.  I think it probably needs the same
>> > changes.
>>
>> I'll amend the patch to include that change and push it.
>>
>
> Thank you.
>
>
>>
>> What's the reason you add align - 1 bytes to the total size instead of
>> simply align, for that matter?
>>
>>
> align - 1 is the most a pointer can be misaligned by,
> so that's the most padding we need to correct it.
>


... but I haven't considered that align can be 0, so that may be a problem.

Pavel.