[FFmpeg-devel] [PATCH 3/3] avutil/frame: also align data pointers in av_frame_get_buffer()
James Almer
jamrial at gmail.com
Fri Nov 22 05:21:22 EET 2024
On 11/22/2024 12:17 AM, Pavel Koshevoy wrote:
> On Sat, Nov 16, 2024 at 10:02 AM James Almer <jamrial at gmail.com> wrote:
>
>> From: Pavel Koshevoy <pkoshevoy at gmail.com>
>>
>> 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.
>>
>> Co-authored-by: James Almer <jamrial at gmail.com>
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>> doc/APIchanges | 4 ++++
>> libavutil/frame.c | 21 ++++++++++++++++-----
>> libavutil/frame.h | 7 ++++---
>> libavutil/version.h | 2 +-
>> 4 files changed, 25 insertions(+), 9 deletions(-)
>>
>> diff --git a/doc/APIchanges b/doc/APIchanges
>> index 15606cafac..d477904856 100644
>> --- a/doc/APIchanges
>> +++ b/doc/APIchanges
>> @@ -2,6 +2,10 @@ The last version increases of all libraries were on
>> 2024-03-07
>>
>> API changes, most recent first:
>>
>> +2024-11-16 - xxxxxxxxxx - lavu 59.47.101 - frame.h
>> + av_frame_get_buffer() now also aligns the data pointers according to
>> + the requested alignment.
>> +
>> 2024-11-13 - xxxxxxxxxx - lavu 59.47.100 - channel_layout.h
>> Add AV_CHAN_BINAURAL_LEFT, AV_CHAN_BINAURAL_RIGHT
>> Add AV_CH_BINAURAL_LEFT, AV_CH_BINAURAL_RIGHT
>> diff --git a/libavutil/frame.c b/libavutil/frame.c
>> index 093853b173..5fb47dbaa6 100644
>> --- a/libavutil/frame.c
>> +++ b/libavutil/frame.c
>> @@ -210,7 +210,7 @@ static int get_video_buffer(AVFrame *frame, int align)
>> padded_height, linesizes)) < 0)
>> return ret;
>>
>> - total_size = 4*plane_padding;
>> + total_size = 4 * plane_padding + 4 * align;
>> for (int i = 0; i < 4; i++) {
>> if (sizes[i] > SIZE_MAX - total_size)
>> return AVERROR(EINVAL);
>> @@ -230,6 +230,7 @@ static int get_video_buffer(AVFrame *frame, int align)
>> for (int i = 1; i < 4; i++) {
>> if (frame->data[i])
>> frame->data[i] += i * plane_padding;
>> + frame->data[i] = (uint8_t *)FFALIGN((uintptr_t)frame->data[i],
>> align);
>> }
>>
>> frame->extended_data = frame->data;
>> @@ -244,6 +245,7 @@ static int get_audio_buffer(AVFrame *frame, int align)
>> {
>> int planar = av_sample_fmt_is_planar(frame->format);
>> int channels, planes;
>> + size_t size;
>> int ret;
>>
>> channels = frame->ch_layout.nb_channels;
>> @@ -256,6 +258,9 @@ static int get_audio_buffer(AVFrame *frame, int align)
>> return ret;
>> }
>>
>> + if (align <= 0)
>> + align = ALIGN;
>> +
>> if (planes > AV_NUM_DATA_POINTERS) {
>> frame->extended_data = av_calloc(planes,
>> sizeof(*frame->extended_data));
>> @@ -270,21 +275,27 @@ static int get_audio_buffer(AVFrame *frame, int
>> align)
>> } else
>> frame->extended_data = frame->data;
>>
>> + if (frame->linesize[0] > SIZE_MAX - align)
>> + return AVERROR(EINVAL);
>> + size = frame->linesize[0] + (size_t)align;
>> +
>> for (int i = 0; i < FFMIN(planes, AV_NUM_DATA_POINTERS); i++) {
>> - frame->buf[i] = av_buffer_alloc(frame->linesize[0]);
>> + frame->buf[i] = av_buffer_alloc(size);
>> if (!frame->buf[i]) {
>> av_frame_unref(frame);
>> return AVERROR(ENOMEM);
>> }
>> - frame->extended_data[i] = frame->data[i] = frame->buf[i]->data;
>> + frame->extended_data[i] = frame->data[i] =
>> + (uint8_t *)FFALIGN((uintptr_t)frame->buf[i]->data, align);
>> }
>> for (int i = 0; i < planes - AV_NUM_DATA_POINTERS; i++) {
>> - frame->extended_buf[i] = av_buffer_alloc(frame->linesize[0]);
>> + frame->extended_buf[i] = av_buffer_alloc(size);
>> if (!frame->extended_buf[i]) {
>> av_frame_unref(frame);
>> return AVERROR(ENOMEM);
>> }
>> - frame->extended_data[i + AV_NUM_DATA_POINTERS] =
>> frame->extended_buf[i]->data;
>> + frame->extended_data[i + AV_NUM_DATA_POINTERS] =
>> + (uint8_t *)FFALIGN((uintptr_t)frame->extended_buf[i]->data,
>> align);
>> }
>> return 0;
>>
>> diff --git a/libavutil/frame.h b/libavutil/frame.h
>> index f7806566d5..c107f43bc0 100644
>> --- a/libavutil/frame.h
>> +++ b/libavutil/frame.h
>> @@ -887,9 +887,10 @@ void av_frame_move_ref(AVFrame *dst, AVFrame *src);
>> * cases.
>> *
>> * @param frame frame in which to store the new buffers.
>> - * @param align Required buffer size alignment. If equal to 0, alignment
>> will be
>> - * chosen automatically for the current CPU. It is highly
>> - * recommended to pass 0 here unless you know what you are
>> doing.
>> + * @param align Required buffer size and data pointer alignment. If equal
>> to 0,
>> + * alignment will be chosen automatically for the current
>> CPU.
>> + * It is highly recommended to pass 0 here unless you know
>> what
>> + * you are doing.
>> *
>> * @return 0 on success, a negative AVERROR on error.
>> */
>> diff --git a/libavutil/version.h b/libavutil/version.h
>> index c1878a49ad..6a4abcf7f5 100644
>> --- a/libavutil/version.h
>> +++ b/libavutil/version.h
>> @@ -80,7 +80,7 @@
>>
>> #define LIBAVUTIL_VERSION_MAJOR 59
>> #define LIBAVUTIL_VERSION_MINOR 47
>> -#define LIBAVUTIL_VERSION_MICRO 100
>> +#define LIBAVUTIL_VERSION_MICRO 101
>>
>> #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
>> LIBAVUTIL_VERSION_MINOR, \
>> --
>> 2.47.0
>>
>>
>
> is something blocking this patch?
Not really, i was just waiting for someone else to look at it, but if
nobody comments I'll push it.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 495 bytes
Desc: OpenPGP digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20241122/8d9a600c/attachment.sig>
More information about the ffmpeg-devel
mailing list