| Message ID | 20210727144813.452917-2-george@nsup.org |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,01/10] lavu/internal: add hex to int functions. | 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 |
On Tue, 27 Jul 2021, Nicolas George wrote: > Signed-off-by: Nicolas George <george@nsup.org> > --- > libavutil/internal.h | 34 ++++++++++++++++++++++++++++++++++ > 1 file changed, 34 insertions(+) > > diff --git a/libavutil/internal.h b/libavutil/internal.h > index a33e8700c3..ba221438ed 100644 > --- a/libavutil/internal.h > +++ b/libavutil/internal.h > @@ -291,4 +291,38 @@ void ff_check_pixfmt_descriptors(void); > */ > int avpriv_dict_set_timestamp(AVDictionary **dict, const char *key, int64_t timestamp); > > +/** > + * Test if val is between min and max, inclusive. > + */ > +static inline int ff_between(unsigned min, unsigned max, unsigned val) > +{ > + return val - min <= max - min; > +} This seems a bit too general. What if somebody needs int and not unsigned? I just think it is better to not make this ff_*. > + > +/** > + * Convert a hex digit to its value, -1 if not hex. > + */ > +static inline int ff_hexchar2int(char c) > +{ > + if (ff_between('0', '9', c)) av_isdigit(c) > + return c - '0'; > + c &= ~('a' - 'A'); c = av_toupper(c); > + if (ff_between('A', 'F', c)) > + return c - 'A' + 10; > + return -1; > +} > + > +/** > + * Convert a hex digit to its value, -1 if not hex. > + */ > +static inline int ff_hexpair2int(const char *s) > +{ > + int a, b; > + > + if ((a = ff_hexchar2int(s[0])) < 0 || > + (b = ff_hexchar2int(s[1])) < 0) > + return -1; > + return (a << 4) | b; > +} > + > #endif /* AVUTIL_INTERNAL_H */ Thanks, Marton
Marton Balint (12021-07-27): > > +static inline int ff_between(unsigned min, unsigned max, unsigned val) > > +{ > > + return val - min <= max - min; > > +} > This seems a bit too general. What if somebody needs int and not unsigned? I Any type smaller than unsigned will be promoted to unsigned, which is exactly what needs to happen. It will not work for larger types, and I think it is ok; the C standard does the same: abs() does not work for int64_t. > just think it is better to not make this ff_*. We cannot hide a static inline function. If you really insist, I can make it a macro and #undef it later, but <PUKE EMOJI>. > > + if (ff_between('0', '9', c)) > > av_isdigit(c) > > > + return c - '0'; > > + c &= ~('a' - 'A'); > > c = av_toupper(c); That would require including avstring.h from internal.h. I do not think these simple functions are worth it. Regards,
On Wed, 28 Jul 2021, Nicolas George wrote: > Marton Balint (12021-07-27): >>> +static inline int ff_between(unsigned min, unsigned max, unsigned val) >>> +{ >>> + return val - min <= max - min; >>> +} >> This seems a bit too general. What if somebody needs int and not unsigned? I > > Any type smaller than unsigned will be promoted to unsigned, which is > exactly what needs to happen. It will not work for larger types, and I > think it is ok; the C standard does the same: abs() does not work for > int64_t. > >> just think it is better to not make this ff_*. > > We cannot hide a static inline function. If you really insist, I can > make it a macro and #undef it later, but <PUKE EMOJI>. No, fine as is, can be changed later if needed. >> >> av_isdigit(c) >> >>> + return c - '0'; >>> + c &= ~('a' - 'A'); >> >> c = av_toupper(c); > > That would require including avstring.h from internal.h. I do not think > these simple functions are worth it. Then maybe add the function to avstring.h? c &= ~('a' - 'A') is anything but readable, at least add a comment if you don't like avstring.h. Thanks, Marton
Marton Balint (12021-07-28): > Then maybe add the function to avstring.h? You mean make it public? I am not against it, but I would wait for a third opinion. > c &= ~('a' - 'A') is anything but > readable, at least add a comment if you don't like avstring.h. I can add a comment, no problem. Good idea. Regards,
Nicolas George (12021-07-28): > You mean make it public? I am not against it, but I would wait for a > third opinion. If there are no third opinion, I will stick with the conservative stance and keep it private. Regards,
diff --git a/libavutil/internal.h b/libavutil/internal.h index a33e8700c3..ba221438ed 100644 --- a/libavutil/internal.h +++ b/libavutil/internal.h @@ -291,4 +291,38 @@ void ff_check_pixfmt_descriptors(void); */ int avpriv_dict_set_timestamp(AVDictionary **dict, const char *key, int64_t timestamp); +/** + * Test if val is between min and max, inclusive. + */ +static inline int ff_between(unsigned min, unsigned max, unsigned val) +{ + return val - min <= max - min; +} + +/** + * Convert a hex digit to its value, -1 if not hex. + */ +static inline int ff_hexchar2int(char c) +{ + if (ff_between('0', '9', c)) + return c - '0'; + c &= ~('a' - 'A'); + if (ff_between('A', 'F', c)) + return c - 'A' + 10; + return -1; +} + +/** + * Convert a hex digit to its value, -1 if not hex. + */ +static inline int ff_hexpair2int(const char *s) +{ + int a, b; + + if ((a = ff_hexchar2int(s[0])) < 0 || + (b = ff_hexchar2int(s[1])) < 0) + return -1; + return (a << 4) | b; +} + #endif /* AVUTIL_INTERNAL_H */
Signed-off-by: Nicolas George <george@nsup.org> --- libavutil/internal.h | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)