[FFmpeg-devel] [PATCH] frame: Simplify the video allocation

[James Almer]

On 9/3/2018 6:03 AM, Maxym Dmytrychenko wrote:
> On Mon, Sep 3, 2018 at 10:17 AM Michael Niedermayer <michael at niedermayer.cc>
> wrote:
> 
>> On Sun, Sep 02, 2018 at 09:34:23PM -0300, James Almer wrote:
>>> From: Luca Barbato <lu_zero at gentoo.org>
>>>
>>> Merged-by: James Almer <jamrial at gmail.com>
>>> ---
>>> This is the next merge in the queue. It's a critical part of the AVFrame
>> API,
>>> so even if FATE passes I'd rather have others look at it and test in case
>>> something breaks.
>>>
>>> The only difference compared to the libav commit is the "32 - 1" padding
>> per
>>> plane when allocating the buffer, which was only in our tree.
>>
>> why is the STRIDE_ALIGN (which is a thing in units of bytes along the
>> horizontal axis) added to padded_height which is vertical axis ?
>> This is not done prior to the change
>>
>> Also if this changes how buffers are structured or sized it requires a more
>> detailed commit message than "frame: Simplify the video allocation"
>>
>>
> If can help:
> Use of av_image_fill_pointers() helps to allocate planes continuously
> instead of separate allocation for each plane,
> which can end up in very different start locations of the allocated memory.
> 
> Continuous allocation can be better for performance and/or functional sides
> of the operations,
> example of Intel's HW - QSV,
> which is assuming Y/UV planes to be continuously allocated.

I just merged this commit and the next, "qsv: enforcing continuous
memory layout" of your authoring, where one line checks the distance
between frame->data[1] and frame->data[0] to ensure the buffer is
continuous. This check, with the padding in our av_frame_get_buffer()
implementation, will probably always fail, but i can't test it.

Can you look into it, if that's indeed the case?