On Fri, Nov 27, 2015 at 9:43 AM, Clément Bœsch <u...@pkh.me> wrote: > On Fri, Nov 27, 2015 at 06:32:40AM -0500, Ganesh Ajjanagadde wrote: >> On Fri, Nov 27, 2015 at 4:46 AM, Clément Bœsch <u...@pkh.me> wrote: >> > On Thu, Nov 26, 2015 at 11:58:10AM -0500, Ganesh Ajjanagadde wrote: >> >> Reviewed-by: Ronald S. Bultje <rsbul...@gmail.com> >> >> Signed-off-by: Ganesh Ajjanagadde <gajjanaga...@gmail.com> >> >> --- >> >> libavutil/tablegen.h | 33 +++++++++++++++++++++++++++++++++ >> >> 1 file changed, 33 insertions(+) >> >> create mode 100644 libavutil/tablegen.h >> >> >> >> diff --git a/libavutil/tablegen.h b/libavutil/tablegen.h >> >> new file mode 100644 >> >> index 0000000..01c8eea >> >> --- /dev/null >> >> +++ b/libavutil/tablegen.h >> >> @@ -0,0 +1,33 @@ >> >> +/* >> >> + * This file is part of FFmpeg. >> >> + * >> >> + * FFmpeg is free software; you can redistribute it and/or >> >> + * modify it under the terms of the GNU Lesser General Public >> >> + * License as published by the Free Software Foundation; either >> >> + * version 2.1 of the License, or (at your option) any later version. >> >> + * >> >> + * FFmpeg is distributed in the hope that it will be useful, >> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> >> + * Lesser General Public License for more details. >> >> + * >> >> + * You should have received a copy of the GNU Lesser General Public >> >> + * License along with FFmpeg; if not, write to the Free Software >> >> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA >> >> 02110-1301 USA >> >> + */ >> >> + >> >> +/** >> >> + * @file >> >> + * Compatibility libm for table generation files >> >> + */ >> >> + >> >> +#ifndef AVUTIL_TABLEGEN_H >> >> +#define AVUTIL_TABLEGEN_H >> >> + >> >> +// we lack some functions on all host platforms, and we don't care about >> >> +// performance as it's performed at build time >> >> +#define cbrt(x) pow(x, 1.0 / 3) >> > >> >> +#define llrint(x) floor((x) + 0.5) >> >> +#define lrint(x) floor((x) + 0.5) >> >> + >> > >> > does it work for x < 0? >> >> Of course not exactly; I did recognize this. However, there are 2 >> reasons for this: >> 1. The old tablegen code did exactly a floor(x + 0.5), suggesting that >> it did not matter. Thus, this was done for simplicity, anyway these >> are imperfect hacks. The hacks can be improved, but felt that belongs >> in a separate patch - no matter how they are improved, they remain >> hacks (see number 2). >> 2. Even if one does the floor if x > 0, ceil if x < 0 trick, it has >> issues with corner cases, I recall Michael mentioning something >> regarding 0.4999 or something like that. >> > > it's pretty fine with me to use a simple hack like this, but then i'd > argue it's better if the global c namespace is not polluted with openly > broken implementations: just name the macro differently (ffrint, > simplerint, or whatever) to make sure someone doesn't end up using it in a > different context where negative values matter (and where the issue won't > get detected quickly)
Actually, how will this work? We need the same names as the standard functions so that table generation works correctly. I think the easiest thing to do since I anyway don't mind is to do a "less broken" ceil if < 0, etc so that it is more reasonable. And unless I am mistaken here, one does not need to use macros for this, inline (not av_always_inline which depends on attributes.h) functions should be fine and I prefer them. I can repost, or push directly. > > -- > Clément B. > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel