On Thu, Sep 06, 2018 at 08:04:02PM -0300, James Almer wrote: > On 9/6/2018 7:26 PM, Michael Niedermayer wrote: > > On Thu, Sep 06, 2018 at 01:10:31PM -0300, James Almer wrote: > >> 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? > > > > something like (with rounding up) > > bytes * horizontal_chroma_subsampling / width * vertical_chroma_subsampling > > Isn't a calculation like this already being done?
not sure i understand what you refer to > > > > > > >> 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? > > > > possible > > > > > >> > >> It would also be faster if you could commit that change instead. > > > > thinking of this, its maybe simpler to adjust data[*] by these to get > > exactly teh same effect as before > > Is this before or after the merge? Because after the merge it's > av_image_fill_pointers() who does all the work, and get_video_buffer() > has no control over the pointers. i meant after but maybe i miss something why the caller couldnt adjust the pointers > > Nothing about this is obvious to me, so i ask again if you could > implement this instead. Otherwise I'll just no-op the merge and add it > to the list of skipped changes in case someone else wants to give it a > try at some other time. ill take a look tomorrow, ping me in case i forget [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Let us carefully observe those good qualities wherein our enemies excel us and endeavor to excel them, by avoiding what is faulty, and imitating what is excellent in them. -- Plutarch
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
