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

[Clément Bœsch]

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)

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151127/fa8aa0e2/attachment.sig>