| Message ID | 20210219204345.14218-1-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avutil/common: Add FFINCREASE_PTR() | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
Michael Niedermayer: > Suggested-by: Andreas Rheinhardt > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > doc/APIchanges | 3 +++ > libavutil/common.h | 2 ++ > 2 files changed, 5 insertions(+) > > diff --git a/doc/APIchanges b/doc/APIchanges > index c353d2d281..e38a5cb91c 100644 > --- a/doc/APIchanges > +++ b/doc/APIchanges > @@ -15,6 +15,9 @@ libavutil: 2017-10-21 > > API changes, most recent first: > > +2021-02-xx - xxxxxxxxxx - lavu 57.xx.100 - common.h > + Add FFINCREASE_PTR() > + > 2021-02-14 - xxxxxxxxxx - lavd 58.12.100 - avdevice.h > Deprecated avdevice_capabilities_create() and > avdevice_capabilities_free(). > diff --git a/libavutil/common.h b/libavutil/common.h > index aee353d399..bf35bc8507 100644 > --- a/libavutil/common.h > +++ b/libavutil/common.h > @@ -108,6 +108,8 @@ > #define FFSWAP(type,a,b) do{type SWAP_tmp= b; b= a; a= SWAP_tmp;}while(0) > #define FF_ARRAY_ELEMS(a) (sizeof(a) / sizeof((a)[0])) > > +#define FFINCREASE_PTR(ptr, off) ((off) ? (ptr) + (off) : (ptr)) > + > /* misc math functions */ > > #ifdef HAVE_AV_CONFIG_H > If this is intended to be a public macro, it should have a proper AV prefix. - Andreas
Michael Niedermayer (12021-02-19): > Suggested-by: Andreas Rheinhardt > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > doc/APIchanges | 3 +++ > libavutil/common.h | 2 ++ > 2 files changed, 5 insertions(+) Is it only for NULL+0 or is it for all NULL+x? It is ok to hide NULL+0, but NULL+x is a different kind of bug, more serious, but the macro hides it. Maybe add an av_assert2()? Regards,
Nicolas George: > Michael Niedermayer (12021-02-19): >> Suggested-by: Andreas Rheinhardt >> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >> --- >> doc/APIchanges | 3 +++ >> libavutil/common.h | 2 ++ >> 2 files changed, 5 insertions(+) > > Is it only for NULL+0 or is it for all NULL+x? > > It is ok to hide NULL+0, but NULL+x is a different kind of bug, more > serious, but the macro hides it. Maybe add an av_assert2()? > It is only for NULL+0; NULL+x is a real bug after all. And the macro doesn't hide it; NULL+x can still be detected with all the typical tools (ubsan) to detect NULL+x. Doing it this way has the advantage of allowing the compiler to optimize the branch away (GCC does it). - Andreas
Andreas Rheinhardt (12021-02-19): > It is only for NULL+0; NULL+x is a real bug after all. > And the macro doesn't hide it; NULL+x can still be detected with all the > typical tools (ubsan) to detect NULL+x. Doing it this way has the > advantage of allowing the compiler to optimize the branch away (GCC does > it). Oh, I had read the macro the wrong way. This version is cleverer. Regards,
On Fri, Feb 19, 2021 at 10:00:39PM +0100, Andreas Rheinhardt wrote: > Michael Niedermayer: > > Suggested-by: Andreas Rheinhardt > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > doc/APIchanges | 3 +++ > > libavutil/common.h | 2 ++ > > 2 files changed, 5 insertions(+) > > > > diff --git a/doc/APIchanges b/doc/APIchanges > > index c353d2d281..e38a5cb91c 100644 > > --- a/doc/APIchanges > > +++ b/doc/APIchanges > > @@ -15,6 +15,9 @@ libavutil: 2017-10-21 > > > > API changes, most recent first: > > > > +2021-02-xx - xxxxxxxxxx - lavu 57.xx.100 - common.h > > + Add FFINCREASE_PTR() > > + > > 2021-02-14 - xxxxxxxxxx - lavd 58.12.100 - avdevice.h > > Deprecated avdevice_capabilities_create() and > > avdevice_capabilities_free(). > > diff --git a/libavutil/common.h b/libavutil/common.h > > index aee353d399..bf35bc8507 100644 > > --- a/libavutil/common.h > > +++ b/libavutil/common.h > > @@ -108,6 +108,8 @@ > > #define FFSWAP(type,a,b) do{type SWAP_tmp= b; b= a; a= SWAP_tmp;}while(0) > > #define FF_ARRAY_ELEMS(a) (sizeof(a) / sizeof((a)[0])) > > > > +#define FFINCREASE_PTR(ptr, off) ((off) ? (ptr) + (off) : (ptr)) > > + > > /* misc math functions */ > > > > #ifdef HAVE_AV_CONFIG_H > > > If this is intended to be a public macro, it should have a proper AV prefix. will apply with AV prefix thx [...]
Mar 13, 2021, 16:18 by michael@niedermayer.cc: > On Fri, Feb 19, 2021 at 10:00:39PM +0100, Andreas Rheinhardt wrote: > >> Michael Niedermayer: >> > Suggested-by: Andreas Rheinhardt >> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >> > --- >> > doc/APIchanges | 3 +++ >> > libavutil/common.h | 2 ++ >> > 2 files changed, 5 insertions(+) >> > >> > diff --git a/doc/APIchanges b/doc/APIchanges >> > index c353d2d281..e38a5cb91c 100644 >> > --- a/doc/APIchanges >> > +++ b/doc/APIchanges >> > @@ -15,6 +15,9 @@ libavutil: 2017-10-21 >> > >> > API changes, most recent first: >> > >> > +2021-02-xx - xxxxxxxxxx - lavu 57.xx.100 - common.h >> > + Add FFINCREASE_PTR() >> > + >> > 2021-02-14 - xxxxxxxxxx - lavd 58.12.100 - avdevice.h >> > Deprecated avdevice_capabilities_create() and >> > avdevice_capabilities_free(). >> > diff --git a/libavutil/common.h b/libavutil/common.h >> > index aee353d399..bf35bc8507 100644 >> > --- a/libavutil/common.h >> > +++ b/libavutil/common.h >> > @@ -108,6 +108,8 @@ >> > #define FFSWAP(type,a,b) do{type SWAP_tmp= b; b= a; a= SWAP_tmp;}while(0) >> > #define FF_ARRAY_ELEMS(a) (sizeof(a) / sizeof((a)[0])) >> > >> > +#define FFINCREASE_PTR(ptr, off) ((off) ? (ptr) + (off) : (ptr)) >> > + >> > /* misc math functions */ >> > >> > #ifdef HAVE_AV_CONFIG_H >> > >> If this is intended to be a public macro, it should have a proper AV prefix. >> > > will apply with AV prefix > Could we not make it public? Macros like FF_ARRAY_ELEMS aren't, and I think this is one of those utility functions that API users probably won't find useful. Plus the name is kinda long.
On Sat, Mar 13, 2021 at 04:41:39PM +0100, Lynne wrote: > Mar 13, 2021, 16:18 by michael@niedermayer.cc: > > > On Fri, Feb 19, 2021 at 10:00:39PM +0100, Andreas Rheinhardt wrote: > > > >> Michael Niedermayer: > >> > Suggested-by: Andreas Rheinhardt > >> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > >> > --- > >> > doc/APIchanges | 3 +++ > >> > libavutil/common.h | 2 ++ > >> > 2 files changed, 5 insertions(+) > >> > > >> > diff --git a/doc/APIchanges b/doc/APIchanges > >> > index c353d2d281..e38a5cb91c 100644 > >> > --- a/doc/APIchanges > >> > +++ b/doc/APIchanges > >> > @@ -15,6 +15,9 @@ libavutil: 2017-10-21 > >> > > >> > API changes, most recent first: > >> > > >> > +2021-02-xx - xxxxxxxxxx - lavu 57.xx.100 - common.h > >> > + Add FFINCREASE_PTR() > >> > + > >> > 2021-02-14 - xxxxxxxxxx - lavd 58.12.100 - avdevice.h > >> > Deprecated avdevice_capabilities_create() and > >> > avdevice_capabilities_free(). > >> > diff --git a/libavutil/common.h b/libavutil/common.h > >> > index aee353d399..bf35bc8507 100644 > >> > --- a/libavutil/common.h > >> > +++ b/libavutil/common.h > >> > @@ -108,6 +108,8 @@ > >> > #define FFSWAP(type,a,b) do{type SWAP_tmp= b; b= a; a= SWAP_tmp;}while(0) > >> > #define FF_ARRAY_ELEMS(a) (sizeof(a) / sizeof((a)[0])) > >> > > >> > +#define FFINCREASE_PTR(ptr, off) ((off) ? (ptr) + (off) : (ptr)) > >> > + > >> > /* misc math functions */ > >> > > >> > #ifdef HAVE_AV_CONFIG_H > >> > > >> If this is intended to be a public macro, it should have a proper AV prefix. > >> > > > > will apply with AV prefix > > > > Could we not make it public? Macros like FF_ARRAY_ELEMS aren't, > and I think this is one of those utility functions that API users probably > won't find useful. > Plus the name is kinda long. sure, what do others prefer, iam fine with either thx [...]
On Sat, Mar 13, 2021 at 04:41:39PM +0100, Lynne wrote: > Mar 13, 2021, 16:18 by michael@niedermayer.cc: > > > On Fri, Feb 19, 2021 at 10:00:39PM +0100, Andreas Rheinhardt wrote: > > > >> Michael Niedermayer: > >> > Suggested-by: Andreas Rheinhardt > >> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > >> > --- > >> > doc/APIchanges | 3 +++ > >> > libavutil/common.h | 2 ++ > >> > 2 files changed, 5 insertions(+) > >> > > >> > diff --git a/doc/APIchanges b/doc/APIchanges > >> > index c353d2d281..e38a5cb91c 100644 > >> > --- a/doc/APIchanges > >> > +++ b/doc/APIchanges > >> > @@ -15,6 +15,9 @@ libavutil: 2017-10-21 > >> > > >> > API changes, most recent first: > >> > > >> > +2021-02-xx - xxxxxxxxxx - lavu 57.xx.100 - common.h > >> > + Add FFINCREASE_PTR() > >> > + > >> > 2021-02-14 - xxxxxxxxxx - lavd 58.12.100 - avdevice.h > >> > Deprecated avdevice_capabilities_create() and > >> > avdevice_capabilities_free(). > >> > diff --git a/libavutil/common.h b/libavutil/common.h > >> > index aee353d399..bf35bc8507 100644 > >> > --- a/libavutil/common.h > >> > +++ b/libavutil/common.h > >> > @@ -108,6 +108,8 @@ > >> > #define FFSWAP(type,a,b) do{type SWAP_tmp= b; b= a; a= SWAP_tmp;}while(0) > >> > #define FF_ARRAY_ELEMS(a) (sizeof(a) / sizeof((a)[0])) > >> > > >> > +#define FFINCREASE_PTR(ptr, off) ((off) ? (ptr) + (off) : (ptr)) > >> > + > >> > /* misc math functions */ > >> > > >> > #ifdef HAVE_AV_CONFIG_H > >> > > >> If this is intended to be a public macro, it should have a proper AV prefix. > >> > > > > will apply with AV prefix > > > > Could we not make it public? Macros like FF_ARRAY_ELEMS aren't, > and I think this is one of those utility functions that API users probably > won't find useful. > Plus the name is kinda long. would you prefer FF_PTR_ADD() ? or do you have another name in mind ? thx [..]
On Sat, Mar 13, 2021 at 10:20:37PM +0100, Michael Niedermayer wrote: > On Sat, Mar 13, 2021 at 04:41:39PM +0100, Lynne wrote: > > Mar 13, 2021, 16:18 by michael@niedermayer.cc: > > > > > On Fri, Feb 19, 2021 at 10:00:39PM +0100, Andreas Rheinhardt wrote: > > > > > >> Michael Niedermayer: > > >> > Suggested-by: Andreas Rheinhardt > > >> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > >> > --- > > >> > doc/APIchanges | 3 +++ > > >> > libavutil/common.h | 2 ++ > > >> > 2 files changed, 5 insertions(+) > > >> > > > >> > diff --git a/doc/APIchanges b/doc/APIchanges > > >> > index c353d2d281..e38a5cb91c 100644 > > >> > --- a/doc/APIchanges > > >> > +++ b/doc/APIchanges > > >> > @@ -15,6 +15,9 @@ libavutil: 2017-10-21 > > >> > > > >> > API changes, most recent first: > > >> > > > >> > +2021-02-xx - xxxxxxxxxx - lavu 57.xx.100 - common.h > > >> > + Add FFINCREASE_PTR() > > >> > + > > >> > 2021-02-14 - xxxxxxxxxx - lavd 58.12.100 - avdevice.h > > >> > Deprecated avdevice_capabilities_create() and > > >> > avdevice_capabilities_free(). > > >> > diff --git a/libavutil/common.h b/libavutil/common.h > > >> > index aee353d399..bf35bc8507 100644 > > >> > --- a/libavutil/common.h > > >> > +++ b/libavutil/common.h > > >> > @@ -108,6 +108,8 @@ > > >> > #define FFSWAP(type,a,b) do{type SWAP_tmp= b; b= a; a= SWAP_tmp;}while(0) > > >> > #define FF_ARRAY_ELEMS(a) (sizeof(a) / sizeof((a)[0])) > > >> > > > >> > +#define FFINCREASE_PTR(ptr, off) ((off) ? (ptr) + (off) : (ptr)) > > >> > + > > >> > /* misc math functions */ > > >> > > > >> > #ifdef HAVE_AV_CONFIG_H > > >> > > > >> If this is intended to be a public macro, it should have a proper AV prefix. > > >> > > > > > > will apply with AV prefix > > > > > > > Could we not make it public? Macros like FF_ARRAY_ELEMS aren't, > > and I think this is one of those utility functions that API users probably > > won't find useful. > > > Plus the name is kinda long. > > would you prefer FF_PTR_ADD() ? will submit a patch with that and without adding it to APIchanges [...]
diff --git a/doc/APIchanges b/doc/APIchanges index c353d2d281..e38a5cb91c 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -15,6 +15,9 @@ libavutil: 2017-10-21 API changes, most recent first: +2021-02-xx - xxxxxxxxxx - lavu 57.xx.100 - common.h + Add FFINCREASE_PTR() + 2021-02-14 - xxxxxxxxxx - lavd 58.12.100 - avdevice.h Deprecated avdevice_capabilities_create() and avdevice_capabilities_free(). diff --git a/libavutil/common.h b/libavutil/common.h index aee353d399..bf35bc8507 100644 --- a/libavutil/common.h +++ b/libavutil/common.h @@ -108,6 +108,8 @@ #define FFSWAP(type,a,b) do{type SWAP_tmp= b; b= a; a= SWAP_tmp;}while(0) #define FF_ARRAY_ELEMS(a) (sizeof(a) / sizeof((a)[0])) +#define FFINCREASE_PTR(ptr, off) ((off) ? (ptr) + (off) : (ptr)) + /* misc math functions */ #ifdef HAVE_AV_CONFIG_H
Suggested-by: Andreas Rheinhardt Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- doc/APIchanges | 3 +++ libavutil/common.h | 2 ++ 2 files changed, 5 insertions(+)