Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().

Le nonidi 29 floréal, an CCXXV, Muhammad Faiz a écrit : > But that is what av_frame_get_buffer does (and maybe ff_get_buffer too). Maybe it does that, but I do not think it is an alignment issue, and I am not entirely convinced this is intentional. Anyway, I have wasted enough of my time on this

Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().

On Thu, May 18, 2017 at 5:30 PM, Nicolas George wrote: > Le nonidi 29 floréal, an CCXXV, Muhammad Faiz a écrit : >> I mean for rgb24 format, when alignment is 32, linesize should be >> multiple of 96 (32 is not enough). > > I think you are wrong. I do not know any alignment

Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().

Le nonidi 29 floréal, an CCXXV, Muhammad Faiz a écrit : > I mean for rgb24 format, when alignment is 32, linesize should be > multiple of 96 (32 is not enough). I think you are wrong. I do not know any alignment requirement that is not a power of 2. Regards, -- Nicolas George signature.asc

Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().

On Thu, May 18, 2017 at 5:20 PM, Nicolas George wrote: > Le nonidi 29 floréal, an CCXXV, Muhammad Faiz a écrit : >> Missing linesize check. > > No, check again. Oh, I'm sorry. My fault. > >> I tested the behaviour of av_frame_get_buffer(frame, 32) and check >> linesize[0], here

Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().

Le nonidi 29 floréal, an CCXXV, Hendrik Leppkes a écrit : > We could put the check in another central place Yes. > This is not bikeshedding, its an actual concern. Well, you win, I drop the baby, catch it or not, I do not care. But I still oppose to bad fixed being committed in areas of code I

Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().

On Thu, May 18, 2017 at 11:16 AM, Nicolas George wrote: > Le nonidi 29 floréal, an CCXXV, Hendrik Leppkes a écrit : >> I think its a saner choice to design the API to try to avoid instant >> heap corruption, instead of hoping every case sets the alignment >> properly in all

Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().

Le nonidi 29 floréal, an CCXXV, Muhammad Faiz a écrit : > Missing linesize check. No, check again. > I tested the behaviour of av_frame_get_buffer(frame, 32) and check > linesize[0], here the result of some different pixel formats: > yuv420p: 32 > rgb24: 96 > rgba: 32 > > So linesize constraint

Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().

On Thu, May 18, 2017 at 3:11 PM, Nicolas George wrote: > Signed-off-by: Nicolas George > --- > doc/APIchanges| 3 +++ > libavutil/frame.c | 17 + > libavutil/frame.h | 8 > 3 files changed, 28 insertions(+) > > > With the linesize

Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().

Le nonidi 29 floréal, an CCXXV, Hendrik Leppkes a écrit : > I think its a saner choice to design the API to try to avoid instant > heap corruption, instead of hoping every case sets the alignment > properly in all cases. > A single if condition shouldn't be too much trouble, we already flag >

Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().

On Thu, May 18, 2017 at 10:44 AM, Nicolas George wrote: > Le nonidi 29 floréal, an CCXXV, Hendrik Leppkes a écrit : >> I wonder if there should be an exception in here somewhere for >> hardware pixelformats, there is no reason to assume the hardware >> pointers passed in AVFrame

Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().

Le nonidi 29 floréal, an CCXXV, Hendrik Leppkes a écrit : > I wonder if there should be an exception in here somewhere for > hardware pixelformats, there is no reason to assume the hardware > pointers passed in AVFrame would be aligned, and aligning them would > cause all sorts of crashes. > Best

Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().

On Thu, May 18, 2017 at 10:11 AM, Nicolas George wrote: > Signed-off-by: Nicolas George > --- > doc/APIchanges| 3 +++ > libavutil/frame.c | 17 + > libavutil/frame.h | 8 > 3 files changed, 28 insertions(+) > > I wonder if there

[FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().

Signed-off-by: Nicolas George --- doc/APIchanges| 3 +++ libavutil/frame.c | 17 + libavutil/frame.h | 8 3 files changed, 28 insertions(+) With the linesize check and without the 1<<. diff --git a/doc/APIchanges b/doc/APIchanges index

Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().

On Thu, May 11, 2017 at 2:59 PM, Muhammad Faiz wrote: > On Tue, May 9, 2017 at 8:19 PM, Nicolas George wrote: >> diff --git a/libavutil/frame.c b/libavutil/frame.c >> index 24d5d5f184..e8467a1cd6 100644 >> --- a/libavutil/frame.c >> +++ b/libavutil/frame.c >>

Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().

On Sun, May 14, 2017 at 12:07:59PM +0200, Nicolas George wrote: > Le duodi 22 floréal, an CCXXV, Michael Niedermayer a écrit : > > The direct value like 32 feels more natural to me too, but iam fine > > with either. > > > > The avoidance of log() might also favor the other in some cases btw > >

Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().

Hi, On Sun, May 14, 2017 at 6:07 AM, Nicolas George wrote: > Le duodi 22 floréal, an CCXXV, Michael Niedermayer a écrit : > > The direct value like 32 feels more natural to me too, but iam fine > > with either. > > > > The avoidance of log() might also favor the other in some

Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().

Le duodi 22 floréal, an CCXXV, Michael Niedermayer a écrit : > The direct value like 32 feels more natural to me too, but iam fine > with either. > > The avoidance of log() might also favor the other in some cases btw > consider you have a 32 and need to call a fuction that needs a log2(32) So,

Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().

On Tue, May 9, 2017 at 8:19 PM, Nicolas George wrote: > diff --git a/libavutil/frame.c b/libavutil/frame.c > index 24d5d5f184..e8467a1cd6 100644 > --- a/libavutil/frame.c > +++ b/libavutil/frame.c > @@ -781,3 +781,21 @@ const char *av_frame_side_data_name(enum >

Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().

On Wed, May 10, 2017 at 11:10:48PM +0200, Nicolas George wrote: > Le primidi 21 floréal, an CCXXV, Hendrik Leppkes a écrit : > > Everywhere I found where the align value is used, its used as (1 << > > alignment). In that case, I would prefer to pass the actual alignment > > here (ie. 32 instead of

Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().

On Wed, May 10, 2017 at 11:11:58PM +0200, Hendrik Leppkes wrote: > On Wed, May 10, 2017 at 11:10 PM, Nicolas George wrote: > > Le primidi 21 floréal, an CCXXV, Hendrik Leppkes a écrit : > >> Everywhere I found where the align value is used, its used as (1 << > >> alignment). In

Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().

On Wed, May 10, 2017 at 11:10 PM, Nicolas George wrote: > Le primidi 21 floréal, an CCXXV, Hendrik Leppkes a écrit : >> Everywhere I found where the align value is used, its used as (1 << >> alignment). In that case, I would prefer to pass the actual alignment >> here (ie. 32

Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().

Le primidi 21 floréal, an CCXXV, Hendrik Leppkes a écrit : > Everywhere I found where the align value is used, its used as (1 << > alignment). In that case, I would prefer to pass the actual alignment > here (ie. 32 instead of 5), which is an easier value to understand and > matches the various

Re: [FFmpeg-devel] [PATCH 1/3] lavu/frame: add av_frame_check_align().

On Tue, May 9, 2017 at 3:19 PM, Nicolas George wrote: > diff --git a/libavutil/frame.h b/libavutil/frame.h > index 26261d7e40..196d311e29 100644 > --- a/libavutil/frame.h > +++ b/libavutil/frame.h > @@ -772,6 +772,14 @@ void av_frame_remove_side_data(AVFrame *frame, enum >