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

[Michael Niedermayer]

On Mon, Sep 03, 2018 at 10:29:13AM -0300, James Almer wrote:
> On 9/3/2018 5:17 AM, Michael Niedermayer 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
> 
> The only way to keep this padding we currently have in the tree applied
> to the buffer allocation for each plane like it was before the change
> (Except it'll now be one continuous buffer instead of one per plane) is
> by passing it alongside the height parameter to
> av_image_fill_pointers(). The result is essentially the same.
> 
> Do you want me to change the name of the variable, or remove it and pass
> 32 - 1 to both av_image_fill_pointers() calls directly? Removing the
> padding will probably just make whatever overreads prompted its addition
> to resurface.
> Alternatively, i can just no-op this merge and move on.

allocating one plane instead of 3 is better obviously so i dont think this
should be no-oped unless someone implements this differently

i dont think the padding can be removed saftely but i might be missing something
also i do not remember this 100%

what i see and i may have misunderstood your reply but the code before places
a few bytes between planes, the new code places a few lines, that is alot more
space. Its not even the best that can be done with the current API. For example
the number of extra lines would generally be 1 to provide sufficient padding
at most reaslistic resolutions.

also there is the independant question on the API, do we want/need to make 
adding padding between planes easier?

actually i think that if we change from 31 bytes to X lines padding then this
should be a commit seperate of the 3->1 change. This would make bisect much
more meaningfull and its rather trivial to split this.


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Does the universe only have a finite lifespan? No, its going to go on
forever, its just that you wont like living in it. -- Hiranya Peiri
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180904/9398eb12/attachment.sig>