On 9/4/2018 5:09 PM, Michael Niedermayer wrote: > 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_z...@gentoo.org> >>>> >>>> Merged-by: James Almer <jamr...@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.
Do you have a suggestion on how to choose how many lines of padding to add? And how would it be done? Just passing (h + padding_lines) to av_buffer_alloc() pre merge, and to av_image_fill_pointers() post merge? It would also be faster if you could commit that change instead. > > > [...] > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel