Re: [libav-devel] [PATCH 1/4] Move avutil tables only used in libavcodec to libavcodec.

2012-10-10 Thread Måns Rullgård
Diego Biurrun  writes:

> On Wed, Oct 10, 2012 at 11:55:57AM +0100, Måns Rullgård wrote:
>> Diego Biurrun  writes:
>> > On Wed, Oct 10, 2012 at 11:31:40AM +0100, Måns Rullgård wrote:
>> >> Diego Biurrun  writes:
>> >> > diff --git a/libavutil/inverse.c b/libavcodec/mathops.c
>> >> > similarity index 79%
>> >> > rename from libavutil/inverse.c
>> >> > rename to libavcodec/mathops.c
>> >> > index 5a5c490..45d06eb 100644
>> >> > --- a/libavutil/inverse.c
>> >> > +++ b/libavcodec/mathops.c
>> >> > @@ -1,7 +1,4 @@
>> >> >  /*
>> >> > - * Inverse table
>> >> > - * Copyright (c) 2002-2004 Michael Niedermayer 
>> >> > - *
>> >> >   * This file is part of Libav.
>> >> >   *
>> >> >   * Libav is free software; you can redistribute it and/or
>> >> > @@ -58,3 +55,14 @@ const uint32_t ff_inverse[257]={
>> >> > +
>> >> > +const uint8_t ff_sqrt_tab[256]={
>> >> > +  0, 16, 23, 28, 32, 36, 40, 43, 46, 48, 51, 54, 56, 58, 60, 62, 64, 
>> >> > 66, 68, 70, 72, 74, 76, 77, 79, 80, 82, 84, 85, 87, 88, 90,
>> >> > + 91, 92, 94, 95, 96, 98, 
>> >> > 99,100,102,103,104,105,107,108,109,110,111,112,114,115,116,117,118,119,120,121,122,123,124,125,126,127,
>> >> > +128,129,130,131,132,133,134,135,136,137,138,139,140,141,142,143,144,144,145,146,147,148,149,150,151,151,152,153,154,155,156,156,
>> >> > +157,158,159,160,160,161,162,163,164,164,165,166,167,168,168,169,170,171,171,172,173,174,174,175,176,176,177,178,179,179,180,181,
>> >> > +182,182,183,184,184,185,186,186,187,188,188,189,190,190,191,192,192,193,194,194,195,196,196,197,198,198,199,200,200,201,202,202,
>> >> > +203,204,204,205,205,206,207,207,208,208,209,210,210,211,212,212,213,213,214,215,215,216,216,217,218,218,219,219,220,220,221,222,
>> >> > +222,223,223,224,224,225,226,226,227,227,228,228,229,230,230,231,231,232,232,233,233,234,235,235,236,236,237,237,238,238,239,239,
>> >> > +240,240,241,242,242,243,243,244,244,245,245,246,246,247,247,248,248,249,249,250,250,251,251,252,252,253,253,254,254,255,255,255
>> >> > +};
>> >> 
>> >> The inverse table _MUST_ be in separate file from anything else.
>> >> Otherwise linking will fail when it clashes with the one from libavutil.
>> >
>> > You seem to have misread the patch.  It does not duplicate a table,
>> > but move it.  There is no duplicate to clash with.
>> 
>> I did not misread anything.  It adds a table to a file containing a
>> duplicated table.  The ff_inverse table must remain in a file of its
>> own.
>
> No.  The file libavcodec/mathops.c ends up containing ff_sqrt_tab and
> ff_inverse, but none of the two tables are duplicated.

Hmm, I missed that you removed the libavutil copy.

Anyway, mathops.c is a badly chosen filename.  The file does not contain
any "ops", only tables.

-- 
Måns Rullgård
m...@mansr.com
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 1/4] Move avutil tables only used in libavcodec to libavcodec.

2012-10-10 Thread Diego Biurrun
On Wed, Oct 10, 2012 at 11:55:57AM +0100, Måns Rullgård wrote:
> Diego Biurrun  writes:
> > On Wed, Oct 10, 2012 at 11:31:40AM +0100, Måns Rullgård wrote:
> >> Diego Biurrun  writes:
> >> > diff --git a/libavutil/inverse.c b/libavcodec/mathops.c
> >> > similarity index 79%
> >> > rename from libavutil/inverse.c
> >> > rename to libavcodec/mathops.c
> >> > index 5a5c490..45d06eb 100644
> >> > --- a/libavutil/inverse.c
> >> > +++ b/libavcodec/mathops.c
> >> > @@ -1,7 +1,4 @@
> >> >  /*
> >> > - * Inverse table
> >> > - * Copyright (c) 2002-2004 Michael Niedermayer 
> >> > - *
> >> >   * This file is part of Libav.
> >> >   *
> >> >   * Libav is free software; you can redistribute it and/or
> >> > @@ -58,3 +55,14 @@ const uint32_t ff_inverse[257]={
> >> > +
> >> > +const uint8_t ff_sqrt_tab[256]={
> >> > +  0, 16, 23, 28, 32, 36, 40, 43, 46, 48, 51, 54, 56, 58, 60, 62, 64, 
> >> > 66, 68, 70, 72, 74, 76, 77, 79, 80, 82, 84, 85, 87, 88, 90,
> >> > + 91, 92, 94, 95, 96, 98, 
> >> > 99,100,102,103,104,105,107,108,109,110,111,112,114,115,116,117,118,119,120,121,122,123,124,125,126,127,
> >> > +128,129,130,131,132,133,134,135,136,137,138,139,140,141,142,143,144,144,145,146,147,148,149,150,151,151,152,153,154,155,156,156,
> >> > +157,158,159,160,160,161,162,163,164,164,165,166,167,168,168,169,170,171,171,172,173,174,174,175,176,176,177,178,179,179,180,181,
> >> > +182,182,183,184,184,185,186,186,187,188,188,189,190,190,191,192,192,193,194,194,195,196,196,197,198,198,199,200,200,201,202,202,
> >> > +203,204,204,205,205,206,207,207,208,208,209,210,210,211,212,212,213,213,214,215,215,216,216,217,218,218,219,219,220,220,221,222,
> >> > +222,223,223,224,224,225,226,226,227,227,228,228,229,230,230,231,231,232,232,233,233,234,235,235,236,236,237,237,238,238,239,239,
> >> > +240,240,241,242,242,243,243,244,244,245,245,246,246,247,247,248,248,249,249,250,250,251,251,252,252,253,253,254,254,255,255,255
> >> > +};
> >> 
> >> The inverse table _MUST_ be in separate file from anything else.
> >> Otherwise linking will fail when it clashes with the one from libavutil.
> >
> > You seem to have misread the patch.  It does not duplicate a table,
> > but move it.  There is no duplicate to clash with.
> 
> I did not misread anything.  It adds a table to a file containing a
> duplicated table.  The ff_inverse table must remain in a file of its
> own.

No.  The file libavcodec/mathops.c ends up containing ff_sqrt_tab and
ff_inverse, but none of the two tables are duplicated.

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 1/4] Move avutil tables only used in libavcodec to libavcodec.

2012-10-10 Thread Måns Rullgård
Diego Biurrun  writes:

> On Wed, Oct 10, 2012 at 11:31:40AM +0100, Måns Rullgård wrote:
>> Diego Biurrun  writes:
>> > diff --git a/libavutil/inverse.c b/libavcodec/mathops.c
>> > similarity index 79%
>> > rename from libavutil/inverse.c
>> > rename to libavcodec/mathops.c
>> > index 5a5c490..45d06eb 100644
>> > --- a/libavutil/inverse.c
>> > +++ b/libavcodec/mathops.c
>> > @@ -1,7 +1,4 @@
>> >  /*
>> > - * Inverse table
>> > - * Copyright (c) 2002-2004 Michael Niedermayer 
>> > - *
>> >   * This file is part of Libav.
>> >   *
>> >   * Libav is free software; you can redistribute it and/or
>> > @@ -58,3 +55,14 @@ const uint32_t ff_inverse[257]={
>> > +
>> > +const uint8_t ff_sqrt_tab[256]={
>> > +  0, 16, 23, 28, 32, 36, 40, 43, 46, 48, 51, 54, 56, 58, 60, 62, 64, 66, 
>> > 68, 70, 72, 74, 76, 77, 79, 80, 82, 84, 85, 87, 88, 90,
>> > + 91, 92, 94, 95, 96, 98, 
>> > 99,100,102,103,104,105,107,108,109,110,111,112,114,115,116,117,118,119,120,121,122,123,124,125,126,127,
>> > +128,129,130,131,132,133,134,135,136,137,138,139,140,141,142,143,144,144,145,146,147,148,149,150,151,151,152,153,154,155,156,156,
>> > +157,158,159,160,160,161,162,163,164,164,165,166,167,168,168,169,170,171,171,172,173,174,174,175,176,176,177,178,179,179,180,181,
>> > +182,182,183,184,184,185,186,186,187,188,188,189,190,190,191,192,192,193,194,194,195,196,196,197,198,198,199,200,200,201,202,202,
>> > +203,204,204,205,205,206,207,207,208,208,209,210,210,211,212,212,213,213,214,215,215,216,216,217,218,218,219,219,220,220,221,222,
>> > +222,223,223,224,224,225,226,226,227,227,228,228,229,230,230,231,231,232,232,233,233,234,235,235,236,236,237,237,238,238,239,239,
>> > +240,240,241,242,242,243,243,244,244,245,245,246,246,247,247,248,248,249,249,250,250,251,251,252,252,253,253,254,254,255,255,255
>> > +};
>> 
>> The inverse table _MUST_ be in separate file from anything else.
>> Otherwise linking will fail when it clashes with the one from libavutil.
>
> You seem to have misread the patch.  It does not duplicate a table,
> but move it.  There is no duplicate to clash with.

I did not misread anything.  It adds a table to a file containing a
duplicated table.  The ff_inverse table must remain in a file of its
own.

-- 
Måns Rullgård
m...@mansr.com
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 1/4] Move avutil tables only used in libavcodec to libavcodec.

2012-10-10 Thread Diego Biurrun
On Wed, Oct 10, 2012 at 11:31:40AM +0100, Måns Rullgård wrote:
> Diego Biurrun  writes:
> > diff --git a/libavutil/inverse.c b/libavcodec/mathops.c
> > similarity index 79%
> > rename from libavutil/inverse.c
> > rename to libavcodec/mathops.c
> > index 5a5c490..45d06eb 100644
> > --- a/libavutil/inverse.c
> > +++ b/libavcodec/mathops.c
> > @@ -1,7 +1,4 @@
> >  /*
> > - * Inverse table
> > - * Copyright (c) 2002-2004 Michael Niedermayer 
> > - *
> >   * This file is part of Libav.
> >   *
> >   * Libav is free software; you can redistribute it and/or
> > @@ -58,3 +55,14 @@ const uint32_t ff_inverse[257]={
> > +
> > +const uint8_t ff_sqrt_tab[256]={
> > +  0, 16, 23, 28, 32, 36, 40, 43, 46, 48, 51, 54, 56, 58, 60, 62, 64, 66, 
> > 68, 70, 72, 74, 76, 77, 79, 80, 82, 84, 85, 87, 88, 90,
> > + 91, 92, 94, 95, 96, 98, 
> > 99,100,102,103,104,105,107,108,109,110,111,112,114,115,116,117,118,119,120,121,122,123,124,125,126,127,
> > +128,129,130,131,132,133,134,135,136,137,138,139,140,141,142,143,144,144,145,146,147,148,149,150,151,151,152,153,154,155,156,156,
> > +157,158,159,160,160,161,162,163,164,164,165,166,167,168,168,169,170,171,171,172,173,174,174,175,176,176,177,178,179,179,180,181,
> > +182,182,183,184,184,185,186,186,187,188,188,189,190,190,191,192,192,193,194,194,195,196,196,197,198,198,199,200,200,201,202,202,
> > +203,204,204,205,205,206,207,207,208,208,209,210,210,211,212,212,213,213,214,215,215,216,216,217,218,218,219,219,220,220,221,222,
> > +222,223,223,224,224,225,226,226,227,227,228,228,229,230,230,231,231,232,232,233,233,234,235,235,236,236,237,237,238,238,239,239,
> > +240,240,241,242,242,243,243,244,244,245,245,246,246,247,247,248,248,249,249,250,250,251,251,252,252,253,253,254,254,255,255,255
> > +};
> 
> The inverse table _MUST_ be in separate file from anything else.
> Otherwise linking will fail when it clashes with the one from libavutil.

You seem to have misread the patch.  It does not duplicate a table,
but move it.  There is no duplicate to clash with.

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 1/4] Move avutil tables only used in libavcodec to libavcodec.

2012-10-10 Thread Måns Rullgård
Diego Biurrun  writes:

> diff --git a/libavutil/inverse.c b/libavcodec/mathops.c
> similarity index 79%
> rename from libavutil/inverse.c
> rename to libavcodec/mathops.c
> index 5a5c490..45d06eb 100644
> --- a/libavutil/inverse.c
> +++ b/libavcodec/mathops.c
> @@ -1,7 +1,4 @@
>  /*
> - * Inverse table
> - * Copyright (c) 2002-2004 Michael Niedermayer 
> - *
>   * This file is part of Libav.
>   *
>   * Libav is free software; you can redistribute it and/or
> @@ -58,3 +55,14 @@ const uint32_t ff_inverse[257]={
>17318417,   17248865,   17179870,   17111424,   17043522,   16976156,   
> 16909321,   16843010,
>16777216
>  };
> +
> +const uint8_t ff_sqrt_tab[256]={
> +  0, 16, 23, 28, 32, 36, 40, 43, 46, 48, 51, 54, 56, 58, 60, 62, 64, 66, 68, 
> 70, 72, 74, 76, 77, 79, 80, 82, 84, 85, 87, 88, 90,
> + 91, 92, 94, 95, 96, 98, 
> 99,100,102,103,104,105,107,108,109,110,111,112,114,115,116,117,118,119,120,121,122,123,124,125,126,127,
> +128,129,130,131,132,133,134,135,136,137,138,139,140,141,142,143,144,144,145,146,147,148,149,150,151,151,152,153,154,155,156,156,
> +157,158,159,160,160,161,162,163,164,164,165,166,167,168,168,169,170,171,171,172,173,174,174,175,176,176,177,178,179,179,180,181,
> +182,182,183,184,184,185,186,186,187,188,188,189,190,190,191,192,192,193,194,194,195,196,196,197,198,198,199,200,200,201,202,202,
> +203,204,204,205,205,206,207,207,208,208,209,210,210,211,212,212,213,213,214,215,215,216,216,217,218,218,219,219,220,220,221,222,
> +222,223,223,224,224,225,226,226,227,227,228,228,229,230,230,231,231,232,232,233,233,234,235,235,236,236,237,237,238,238,239,239,
> +240,240,241,242,242,243,243,244,244,245,245,246,246,247,247,248,248,249,249,250,250,251,251,252,252,253,253,254,254,255,255,255
> +};

The inverse table _MUST_ be in separate file from anything else.
Otherwise linking will fail when it clashes with the one from libavutil.

-- 
Måns Rullgård
m...@mansr.com
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 1/4] Move avutil tables only used in libavcodec to libavcodec.

2012-10-04 Thread Diego Biurrun
On Thu, Oct 04, 2012 at 06:43:16PM +0100, Måns Rullgård wrote:
> Diego Biurrun  writes:
> 
> > ---
> >  libavcodec/Makefile |7 +
> >  libavcodec/arm/mathops.h|   24 ++
> >  libavcodec/inverse.c|1 -
> >  libavutil/inverse.c => libavcodec/mathops.c |   14 --
> >  libavcodec/mathops.h|   29 ++
> >  libavcodec/motion_est.c |2 +-
> >  libavcodec/mpegvideo.c  |2 +-
> >  libavcodec/mpegvideo_enc.c  |1 +
> >  libavcodec/ra144.c  |2 +-
> >  libavcodec/roqaudioenc.c|2 +-
> >  libavutil/Makefile  |1 -
> >  libavutil/arm/intmath.h |   24 --
> >  libavutil/intmath.h |   35 
> > ---
> >  libavutil/mathematics.c |   11 
> >  14 files changed, 70 insertions(+), 85 deletions(-)
> >  delete mode 100644 libavcodec/inverse.c
> >  rename libavutil/inverse.c => libavcodec/mathops.c (79%)
> 
> I'd prefer to see this split in one patch per table/macro.

ff_sqrt depends on ff_sqrt_tab and FASTDIV, which depends on ff_inverse.
So I don't see how this could be split.

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 1/4] Move avutil tables only used in libavcodec to libavcodec.

2012-10-04 Thread Måns Rullgård
Diego Biurrun  writes:

> ---
>  libavcodec/Makefile |7 +
>  libavcodec/arm/mathops.h|   24 ++
>  libavcodec/inverse.c|1 -
>  libavutil/inverse.c => libavcodec/mathops.c |   14 --
>  libavcodec/mathops.h|   29 ++
>  libavcodec/motion_est.c |2 +-
>  libavcodec/mpegvideo.c  |2 +-
>  libavcodec/mpegvideo_enc.c  |1 +
>  libavcodec/ra144.c  |2 +-
>  libavcodec/roqaudioenc.c|2 +-
>  libavutil/Makefile  |1 -
>  libavutil/arm/intmath.h |   24 --
>  libavutil/intmath.h |   35 
> ---
>  libavutil/mathematics.c |   11 
>  14 files changed, 70 insertions(+), 85 deletions(-)
>  delete mode 100644 libavcodec/inverse.c
>  rename libavutil/inverse.c => libavcodec/mathops.c (79%)

I'd prefer to see this split in one patch per table/macro.

-- 
Måns Rullgård
m...@mansr.com
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


[libav-devel] [PATCH 1/4] Move avutil tables only used in libavcodec to libavcodec.

2012-10-04 Thread Diego Biurrun
---
 libavcodec/Makefile |7 +
 libavcodec/arm/mathops.h|   24 ++
 libavcodec/inverse.c|1 -
 libavutil/inverse.c => libavcodec/mathops.c |   14 --
 libavcodec/mathops.h|   29 ++
 libavcodec/motion_est.c |2 +-
 libavcodec/mpegvideo.c  |2 +-
 libavcodec/mpegvideo_enc.c  |1 +
 libavcodec/ra144.c  |2 +-
 libavcodec/roqaudioenc.c|2 +-
 libavutil/Makefile  |1 -
 libavutil/arm/intmath.h |   24 --
 libavutil/intmath.h |   35 ---
 libavutil/mathematics.c |   11 
 14 files changed, 70 insertions(+), 85 deletions(-)
 delete mode 100644 libavcodec/inverse.c
 rename libavutil/inverse.c => libavcodec/mathops.c (79%)

diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index bd6567e..409f3b1 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -22,6 +22,7 @@ OBJS = allcodecs.o
  \
fmtconvert.o \
imgconvert.o \
jrevdct.o\
+   mathops.o\
options.o\
parser.o \
raw.o\
@@ -655,12 +656,6 @@ OBJS-$(CONFIG_TEXT2MOVSUB_BSF)+= movsub_bsf.o
 OBJS-$(HAVE_PTHREADS)  += pthread.o
 OBJS-$(HAVE_W32THREADS)+= pthread.o
 
-# inverse.o contains the ff_inverse table definition, which is used by
-# the FASTDIV macro (from libavutil); since referencing the external
-# table has a negative effect on performance, copy it in libavcodec as
-# well.
-OBJS-$(!CONFIG_SMALL)  += inverse.o
-
 SKIPHEADERS+= %_tablegen.h  \
   %_tables.h\
   aac_tablegen_decl.h   \
diff --git a/libavcodec/arm/mathops.h b/libavcodec/arm/mathops.h
index 3803fcd..17a687d 100644
--- a/libavcodec/arm/mathops.h
+++ b/libavcodec/arm/mathops.h
@@ -36,6 +36,30 @@ static inline av_const int MULH(int a, int b)
 __asm__ ("smmul %0, %1, %2" : "=r"(r) : "r"(a), "r"(b));
 return r;
 }
+
+#define FASTDIV FASTDIV
+static av_always_inline av_const int FASTDIV(int a, int b)
+{
+int r;
+__asm__ ("cmp %2, #2   \n\t"
+ "ldr %0, [%3, %2, lsl #2] \n\t"
+ "ite le   \n\t"
+ "lsrle   %0, %1, #1   \n\t"
+ "smmulgt %0, %0, %1   \n\t"
+ : "=&r"(r) : "r"(a), "r"(b), "r"(ff_inverse) : "cc");
+return r;
+}
+
+#else /* HAVE_ARMV6 */
+
+#define FASTDIV FASTDIV
+static av_always_inline av_const int FASTDIV(int a, int b)
+{
+int r, t;
+__asm__ ("umull %1, %0, %2, %3"
+ : "=&r"(r), "=&r"(t) : "r"(a), "r"(ff_inverse[b]));
+return r;
+}
 #endif
 
 #define MLS64(d, a, b) MAC64(d, -(a), b)
diff --git a/libavcodec/inverse.c b/libavcodec/inverse.c
deleted file mode 100644
index 04681d2..000
--- a/libavcodec/inverse.c
+++ /dev/null
@@ -1 +0,0 @@
-#include "libavutil/inverse.c"
diff --git a/libavutil/inverse.c b/libavcodec/mathops.c
similarity index 79%
rename from libavutil/inverse.c
rename to libavcodec/mathops.c
index 5a5c490..45d06eb 100644
--- a/libavutil/inverse.c
+++ b/libavcodec/mathops.c
@@ -1,7 +1,4 @@
 /*
- * Inverse table
- * Copyright (c) 2002-2004 Michael Niedermayer 
- *
  * This file is part of Libav.
  *
  * Libav is free software; you can redistribute it and/or
@@ -58,3 +55,14 @@ const uint32_t ff_inverse[257]={
   17318417,   17248865,   17179870,   17111424,   17043522,   16976156,   
16909321,   16843010,
   16777216
 };
+
+const uint8_t ff_sqrt_tab[256]={
+  0, 16, 23, 28, 32, 36, 40, 43, 46, 48, 51, 54, 56, 58, 60, 62, 64, 66, 68, 
70, 72, 74, 76, 77, 79, 80, 82, 84, 85, 87, 88, 90,
+ 91, 92, 94, 95, 96, 98, 
99,100,102,103,104,105,107,108,109,110,111,112,114,115,116,117,118,119,120,121,122,123,124,125,126,127,
+128,129,130,131,132,133,134,135,136,137,138,139,140,141,142,143,144,144,145,146,147,148,149,150,151,151,152,153,154,155,156,156,
+157,158,159,160,160,161,162,163,164,164,165,166,167,168,168,169,170,171,171,172,173,174,174,175,176,176,177,178,179,179,180,181,
+182,182,183,184,184,185,186,186,187,188,188,189,190,190,191,192,192,193,194,194,195,196,196,197,198,198,199,200,200,201,202,202,
+203,204,204,205,205,206,207,207,208,