[FFmpeg-devel] [PATCH] Move mathops.h to libavutil.

Michael Niedermayer michaelni
Fri Nov 28 02:40:41 CET 2008


On Fri, Nov 28, 2008 at 01:03:24AM +0000, M?ns Rullg?rd wrote:
> Michael Niedermayer <michaelni at gmx.at> writes:
> 
> > On Thu, Nov 27, 2008 at 11:10:10PM +0000, M?ns Rullg?rd wrote:
> >> Michael Niedermayer <michaelni at gmx.at> writes:
> >> 
> >> > On Thu, Nov 27, 2008 at 01:18:05PM -0000, M?ns Rullg?rd wrote:
> >> >> 
> >> >> Mans Rullgard wrote:
> >> >> > ---
> >> >> >  libavcodec/lsp.c                               |    2 +-
> >> >> >  libavcodec/mpegaudiodec.c                      |    2 +-
> >> >> >  {libavcodec/armv4l => libavutil/arm}/mathops.h |    6 +++---
> >> >> >  {libavcodec => libavutil}/bfin/mathops.h       |    6 +++---
> >> >> >  {libavcodec => libavutil}/mathops.h            |   12 ++++++------
> >> >> >  {libavcodec => libavutil}/ppc/mathops.h        |    6 +++---
> >> >> >  {libavcodec/i386 => libavutil/x86}/mathops.h   |    6 +++---
> >> >> >  7 files changed, 20 insertions(+), 20 deletions(-)
> >> >> >  rename {libavcodec/armv4l => libavutil/arm}/mathops.h (96%)
> >> >> >  rename {libavcodec => libavutil}/bfin/mathops.h (95%)
> >> >> >  rename {libavcodec => libavutil}/mathops.h (91%)
> >> >> >  rename {libavcodec => libavutil}/ppc/mathops.h (92%)
> >> >> >  rename {libavcodec/i386 => libavutil/x86}/mathops.h (93%)
> >> >> 
> >> >> ping
> >> >
> >> > why should mathops be moved to libavutil?
> >> 
> >> libavutil already has a number of similar macros/functions, scattered
> >> throughout common.h and internal.h:
> >> 
> >> FASTDIV()
> >> FFABS()
> >> FFSIGN()
> >> FFMAX()
> >> COPY3_IF_LT()
> >> MASK_ABS()
> >> av_log2()
> >> av_log2_16bit()
> >> ff_sqrt()
> >> mid_pred()
> >> av_clip()
> >> av_clip_uint8()
> >> av_clip_int16()
> >> av_clipf()
> >> 
> >> Most, if not all, of these are only used in lavc.
> >
> > i know that at least mplayer uses several of them
> >
> >> 
> >> IMO, it would make sense to have all elementary maths operations
> >> defined the same place.
> >
> > it would make sense.
> > But considering the theoretic extreem, there are many math operations that
> > some might consider elementary that probably dont belong in libav* like
> > for example primality testing or factorization of large numbers.
> 
> Can we please have a serious discussion?  I am obviously only talking
> about things that would be used somewhere in FFmpeg.
> 

> >> What criteria do you use deciding where to place a function of this
> >> type?
> >
> > IMO Libavutils goal is to stay small and only contain things that are
> > really usefull for a wide range of applications or tasks.
> > some of the functions like clip or ffmax allow code to be written in a
> > more compact and readable way.
> 
> The ff* ones are, by virtue of their name, not meant to be used
> outside FFmpeg.  If you think this should change, please say so.

i think MAX/MIN should be public ...
if you want to rename them, i dont mind though that would be a big
diff ...


> 
> > COPY3_IF_LT should IMHO be removed from libavutil, its too specific and
> > obscure.
> 
> Tell me where you would like it.

close to where its used
a quick grep pointed just at motion_est*


> 
> > FASTDIV is a nice and fast way to perform divission when the operands are
> > within a limited range. A compiler couldnt just replace a a/b by it ...
> 
> Which brings back the question of ff_inverse, which is defined in
> lavc.  Surely it is wrong for a macro in lavu to depend on lavc.
> BTW, I think a name like DIV16_8 would be more descriptive of what it
> actually is.

yes, though IIRC it was correct for more than 16bit
which brings us to the question how far we should gurantee correctness
some implementations may benefit from the 16bit limit ...
iam just bringing this up as you are proposing to put the limit in the
name, i guess DIV16_8 is fine though, at least better than FASTDIV


[...]

> > Besides it really is gccs job to compile an elementary multiplication 
> > (possibly with a shift) to a reasonable sequence of instrctions. And i
> > hope these things have been reported to the gcc devels. It would be
> > silly if we where maintaining these workarounds while the gcc developers
> > where unaware of the issue.
> 
> What about the 16-bit ones?  They are used in places where a compiler
> cannot easily, if at all, determine that the input operands are
> restricted to 16 significant bits.  I certainly do not expect a
> compiler to recognise the code as an MPEG audio decoder and apply
> facts known only from the MPEG specification.

ill think about this, though in an ideal world one could use assert()
to give the compiler hints about such things ...
but in the non perfect world we live in i guess moving such code to
lavu would make sense but as said id like to think more about this

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have often repented speaking, but never of holding my tongue.
-- Xenocrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081128/f98dead9/attachment.pgp>



More information about the ffmpeg-devel mailing list