| Message ID | 20180903003423.6656-1-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Sun, Sep 02, 2018 at 09:34:23PM -0300, James Almer wrote: > From: Luca Barbato <lu_zero@gentoo.org> > > Merged-by: James Almer <jamrial@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" thanks [...]
On Mon, Sep 3, 2018 at 10:17 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > On Sun, Sep 02, 2018 at 09:34:23PM -0300, James Almer wrote: > > From: Luca Barbato <lu_zero@gentoo.org> > > > > Merged-by: James Almer <jamrial@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. > thanks > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > I have often repented speaking, but never of holding my tongue. > -- Xenocrates > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel thank you - James, Michael. regards Maxym
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@gentoo.org> >> >> Merged-by: James Almer <jamrial@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. > > Also if this changes how buffers are structured or sized it requires a more > detailed commit message than "frame: Simplify the video allocation" Suggestion welcome, but it would be added to the merge commit message. > > thanks > > [...] > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On 2018-09-03 15:29, James Almer wrote:
> pass 32 - 1 to both av_image_fill_pointers() calls directly?
Please do not add a magic number where nobody will find it. Use one of
the 3 already existing methods for knowing the alignment necessary for
assembly.
If this is unrelated, my apologies.
On 9/3/2018 11:56 AM, James Darnley wrote: > On 2018-09-03 15:29, James Almer wrote: >> pass 32 - 1 to both av_image_fill_pointers() calls directly? > > Please do not add a magic number where nobody will find it. Use one of > the 3 already existing methods for knowing the alignment necessary for > assembly. That magic number is STRIDE_ALIGN, and it's mentioned as such before and after the patch. Except for the -1, which i don't know where it came from. Michael (who i think added it) probably knows. > > If this is unrelated, my apologies. > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
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@gentoo.org> > >> > >> Merged-by: James Almer <jamrial@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. [...]
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_zero@gentoo.org> >>>> >>>> Merged-by: James Almer <jamrial@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 >
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_zero@gentoo.org> > >>>> > >>>> Merged-by: James Almer <jamrial@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 > 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 thx [...]
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_zero@gentoo.org> >>>>>> >>>>>> Merged-by: James Almer <jamrial@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? > > >> 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. 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. > > > thx > > [...] > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
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_zero@gentoo.org> > >>>>>> > >>>>>> Merged-by: James Almer <jamrial@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 [...]
On 9/3/2018 6:03 AM, Maxym Dmytrychenko wrote: > On Mon, Sep 3, 2018 at 10:17 AM Michael Niedermayer <michael@niedermayer.cc> > wrote: > >> On Sun, Sep 02, 2018 at 09:34:23PM -0300, James Almer wrote: >>> From: Luca Barbato <lu_zero@gentoo.org> >>> >>> Merged-by: James Almer <jamrial@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?
On Tue, Sep 11, 2018 at 6:43 PM James Almer <jamrial@gmail.com> wrote: > On 9/3/2018 6:03 AM, Maxym Dmytrychenko wrote: > > On Mon, Sep 3, 2018 at 10:17 AM Michael Niedermayer > <michael@niedermayer.cc> > > wrote: > > > >> On Sun, Sep 02, 2018 at 09:34:23PM -0300, James Almer wrote: > >>> From: Luca Barbato <lu_zero@gentoo.org> > >>> > >>> Merged-by: James Almer <jamrial@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? > just finished my test cases and it seems to be just fine, fixes the original, non-deterministic problem. distance between frame->data[1] and frame->data[0] is not always non-continuous(and causes the problem) where now av_frame_get_buffer() fixes such corner case. thanks, James regards Max
diff --git a/libavutil/frame.c b/libavutil/frame.c index deb9b6f334..f072baa916 100644 --- a/libavutil/frame.c +++ b/libavutil/frame.c @@ -211,7 +211,7 @@ void av_frame_free(AVFrame **frame) static int get_video_buffer(AVFrame *frame, int align) { const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(frame->format); - int ret, i; + int ret, i, padded_height; if (!desc) return AVERROR(EINVAL); @@ -236,24 +236,18 @@ static int get_video_buffer(AVFrame *frame, int align) frame->linesize[i] = FFALIGN(frame->linesize[i], align); } - for (i = 0; i < 4 && frame->linesize[i]; i++) { - int h = FFALIGN(frame->height, 32); - if (i == 1 || i == 2) - h = AV_CEIL_RSHIFT(h, desc->log2_chroma_h); + padded_height = FFALIGN(frame->height, 32) + 32 /* STRIDE_ALIGN */ - 1; + if ((ret = av_image_fill_pointers(frame->data, frame->format, padded_height, + NULL, frame->linesize)) < 0) + return ret; - frame->buf[i] = av_buffer_alloc(frame->linesize[i] * h + 16 + 16/*STRIDE_ALIGN*/ - 1); - if (!frame->buf[i]) - goto fail; + frame->buf[0] = av_buffer_alloc(ret); + if (!frame->buf[0]) + goto fail; - frame->data[i] = frame->buf[i]->data; - } - if (desc->flags & AV_PIX_FMT_FLAG_PAL || desc->flags & FF_PSEUDOPAL) { - av_buffer_unref(&frame->buf[1]); - frame->buf[1] = av_buffer_alloc(AVPALETTE_SIZE); - if (!frame->buf[1]) - goto fail; - frame->data[1] = frame->buf[1]->data; - } + if (av_image_fill_pointers(frame->data, frame->format, padded_height, + frame->buf[0]->data, frame->linesize) < 0) + goto fail; frame->extended_data = frame->data;
From: Luca Barbato <lu_zero@gentoo.org> Merged-by: James Almer <jamrial@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. libavutil/frame.c | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-)