[FFmpeg-devel] [PATCHv3 1/3] avutil/tablegen: add tablegen compatibility hacks

[Ganesh Ajjanagadde]

On Fri, Nov 27, 2015 at 9:43 AM, Clément Bœsch <u at 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 at pkh.me> wrote:
>> > On Thu, Nov 26, 2015 at 11:58:10AM -0500, Ganesh Ajjanagadde wrote:
>> >> Reviewed-by: Ronald S. Bultje <rsbultje at gmail.com>
>> >> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at 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)

ok, will change.

>
> --
> Clément B.
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>