Re: [FFmpeg-devel] [PATCH][RFC] avutil/reverse: make ff_reverse shared

[Michael Niedermayer]

On Thu, Nov 30, 2017 at 10:19:36PM -0300, James Almer wrote:
> Signed-off-by: James Almer 
> ---
> Pros:
> Removes duplicate arrays and ugly c file including c files
> 

> Cons:
> Makes the array in libavutil effectively part of the ABI.
> Might not be worth doing for 256 bytes

if iam not mistaken
Accessing symbols accross libs cannot avoid indirection easily.
within a lib one can point to the table directly
so depending on platform this should be slower with shared libs

(this is all of course not strictly unavoidable, it could be done
 differently but its the way it works on linux if i dont misremember)

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

The worst form of inequality is to try to make unequal things equal.
-- Aristotle


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH][RFC] avutil/reverse: make ff_reverse shared

[Hendrik Leppkes]

On Fri, Dec 1, 2017 at 9:42 AM, Nicolas George  wrote:
> James Almer (2017-11-30):
>> Signed-off-by: James Almer 
>> ---
>> Pros:
>> Removes duplicate arrays and ugly c file including c files
>>
>> Cons:
>> Makes the array in libavutil effectively part of the ABI.
>> Might not be worth doing for 256 bytes
>>
>>
>> Something like this was probably sent before, so i guess it
>> will be rejected again.
>
> I think it is worse than that: the opposite was done on purpose.
>
> I believe the rationale was that shared data object do not work
> correctly with shared libraries on windows.
>

That problem was fixed recently.

But in any case,  this creates a lot of noise for very marginal gains,
not sure its really worth it.

- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH][RFC] avutil/reverse: make ff_reverse shared

[Nicolas George]

James Almer (2017-11-30):
> Signed-off-by: James Almer 
> ---
> Pros:
> Removes duplicate arrays and ugly c file including c files
> 
> Cons:
> Makes the array in libavutil effectively part of the ABI.
> Might not be worth doing for 256 bytes
> 
> 
> Something like this was probably sent before, so i guess it
> will be rejected again.

I think it is worse than that: the opposite was done on purpose.

I believe the rationale was that shared data object do not work
correctly with shared libraries on windows.

IMHO, it is yet another argument for merging the libraries.

(And to drop windows support. Only kidding.)

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH][RFC] avutil/reverse: make ff_reverse shared

[Steven Liu]

2017-12-01 9:19 GMT+08:00 James Almer :
> Signed-off-by: James Almer 
> ---
> Pros:
> Removes duplicate arrays and ugly c file including c files
>
> Cons:
> Makes the array in libavutil effectively part of the ABI.
> Might not be worth doing for 256 bytes
>
>
> Something like this was probably sent before, so i guess it
> will be rejected again.
>
>  libavcodec/Makefile  |  2 +-
>  libavcodec/asvdec.c  |  4 ++--
>  libavcodec/asvenc.c  |  4 ++--
>  libavcodec/dcaenc.c  |  2 +-
>  libavcodec/dsd.c |  4 ++--
>  libavcodec/dsddec.c  |  2 +-
>  libavcodec/dstdec.c  |  2 +-
>  libavcodec/indeo2.c  |  2 +-
>  libavcodec/ivi.c |  6 +++---
>  libavcodec/mathops.h |  8 
>  libavcodec/mpeg12dec.c   |  4 ++--
>  libavcodec/pcm.c |  8 
>  libavcodec/reverse.c |  1 -
>  libavcodec/s302m.c   | 36 ++--
>  libavcodec/s302menc.c| 36 ++--
>  libavcodec/tiff.c| 10 +-
>  libavcodec/webp.c|  4 ++--
>  libavcodec/wnv1.c|  4 ++--
>  libavcodec/xbmdec.c  |  4 ++--
>  libavcodec/xbmenc.c  |  2 +-
>  libavdevice/Makefile |  1 -
>  libavdevice/decklink_dec.cpp |  2 +-
>  libavdevice/reverse.c|  1 -
>  libavutil/eval.c |  2 +-
>  libavutil/reverse.c  |  3 ++-
>  libavutil/reverse.h  |  3 ++-
>  26 files changed, 78 insertions(+), 79 deletions(-)
>  delete mode 100644 libavcodec/reverse.c
>  delete mode 100644 libavdevice/reverse.c
>
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index ab7893f560..6942b2c926 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -129,7 +129,7 @@ OBJS-$(CONFIG_QSVENC)  += qsvenc.o
>  OBJS-$(CONFIG_RANGECODER)  += rangecoder.o
>  OBJS-$(CONFIG_RDFT)+= rdft.o
>  OBJS-$(CONFIG_RV34DSP) += rv34dsp.o
> -OBJS-$(CONFIG_SHARED)  += log2_tab.o reverse.o
> +OBJS-$(CONFIG_SHARED)  += log2_tab.o
>  OBJS-$(CONFIG_SINEWIN) += sinewin.o sinewin_fixed.o
>  OBJS-$(CONFIG_SNAPPY)  += snappy.o
>  OBJS-$(CONFIG_STARTCODE)   += startcode.o
> diff --git a/libavcodec/asvdec.c b/libavcodec/asvdec.c
> index 9a11446f52..11d1564db3 100644
> --- a/libavcodec/asvdec.c
> +++ b/libavcodec/asvdec.c
> @@ -71,7 +71,7 @@ static av_cold void init_vlcs(ASV1Context *a)
>  // FIXME write a reversed bitstream reader to avoid the double reverse
>  static inline int asv2_get_bits(GetBitContext *gb, int n)
>  {
> -return ff_reverse[get_bits(gb, n) << (8 - n)];
> +return avpriv_reverse[get_bits(gb, n) << (8 - n)];
>  }
>
>  static inline int asv1_get_level(GetBitContext *gb)
> @@ -229,7 +229,7 @@ static int decode_frame(AVCodecContext *avctx, void 
> *data, int *got_frame,
>  } else {
>  int i;
>  for (i = 0; i < buf_size; i++)
> -a->bitstream_buffer[i] = ff_reverse[buf[i]];
> +a->bitstream_buffer[i] = avpriv_reverse[buf[i]];
>  }
>
>  init_get_bits(>gb, a->bitstream_buffer, buf_size * 8);
> diff --git a/libavcodec/asvenc.c b/libavcodec/asvenc.c
> index c4eca2a13d..7ed932c934 100644
> --- a/libavcodec/asvenc.c
> +++ b/libavcodec/asvenc.c
> @@ -37,7 +37,7 @@
>
>  static inline void asv2_put_bits(PutBitContext *pb, int n, int v)
>  {
> -put_bits(pb, n, ff_reverse[v << (8 - n)]);
> +put_bits(pb, n, avpriv_reverse[v << (8 - n)]);
>  }
>
>  static inline void asv1_put_level(PutBitContext *pb, int level)
> @@ -306,7 +306,7 @@ static int encode_frame(AVCodecContext *avctx, AVPacket 
> *pkt,
>  } else {
>  int i;
>  for (i = 0; i < 4 * size; i++)
> -pkt->data[i] = ff_reverse[pkt->data[i]];
> +pkt->data[i] = avpriv_reverse[pkt->data[i]];
>  }
>
>  pkt->size   = size * 4;
> diff --git a/libavcodec/dcaenc.c b/libavcodec/dcaenc.c
> index dd601ffae0..b111fe98ba 100644
> --- a/libavcodec/dcaenc.c
> +++ b/libavcodec/dcaenc.c
> @@ -446,7 +446,7 @@ static void fft(const int32_t in[2 * 256], cplx32 
> out[256])
>  }
>  /* post-rotation */
>  for (i = 0; i < 256; i++) {
> -int b = ff_reverse[i];
> +int b = avpriv_reverse[i];
>  rout[i].re = mul32(buf[b].re, cos_t(4 * i))
> - mul32(buf[b].im, sin_t(4 * i));
>  rout[i].im = mul32(buf[b].im, cos_t(4 * i))
> diff --git a/libavcodec/dsd.c b/libavcodec/dsd.c
> index 9104f38476..2233adf349 100644
> --- a/libavcodec/dsd.c
> +++ b/libavcodec/dsd.c
> @@ -63,11 +63,11 @@ void ff_dsd2pcm_translate(DSDContext* s, size_t samples, 
> int lsbf,
>  pos = s->pos;
>
>  while (samples-- > 0) {
> -s->buf[pos] = lsbf ? ff_reverse[*src] : *src;
> +s->buf[pos] = lsbf ? avpriv_reverse[*src] : *src;
>  src += src_stride;

[FFmpeg-devel] [PATCH][RFC] avutil/reverse: make ff_reverse shared

[James Almer]

Signed-off-by: James Almer 
---
Pros:
Removes duplicate arrays and ugly c file including c files

Cons:
Makes the array in libavutil effectively part of the ABI.
Might not be worth doing for 256 bytes


Something like this was probably sent before, so i guess it
will be rejected again.

 libavcodec/Makefile  |  2 +-
 libavcodec/asvdec.c  |  4 ++--
 libavcodec/asvenc.c  |  4 ++--
 libavcodec/dcaenc.c  |  2 +-
 libavcodec/dsd.c |  4 ++--
 libavcodec/dsddec.c  |  2 +-
 libavcodec/dstdec.c  |  2 +-
 libavcodec/indeo2.c  |  2 +-
 libavcodec/ivi.c |  6 +++---
 libavcodec/mathops.h |  8 
 libavcodec/mpeg12dec.c   |  4 ++--
 libavcodec/pcm.c |  8 
 libavcodec/reverse.c |  1 -
 libavcodec/s302m.c   | 36 ++--
 libavcodec/s302menc.c| 36 ++--
 libavcodec/tiff.c| 10 +-
 libavcodec/webp.c|  4 ++--
 libavcodec/wnv1.c|  4 ++--
 libavcodec/xbmdec.c  |  4 ++--
 libavcodec/xbmenc.c  |  2 +-
 libavdevice/Makefile |  1 -
 libavdevice/decklink_dec.cpp |  2 +-
 libavdevice/reverse.c|  1 -
 libavutil/eval.c |  2 +-
 libavutil/reverse.c  |  3 ++-
 libavutil/reverse.h  |  3 ++-
 26 files changed, 78 insertions(+), 79 deletions(-)
 delete mode 100644 libavcodec/reverse.c
 delete mode 100644 libavdevice/reverse.c

diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index ab7893f560..6942b2c926 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -129,7 +129,7 @@ OBJS-$(CONFIG_QSVENC)  += qsvenc.o
 OBJS-$(CONFIG_RANGECODER)  += rangecoder.o
 OBJS-$(CONFIG_RDFT)+= rdft.o
 OBJS-$(CONFIG_RV34DSP) += rv34dsp.o
-OBJS-$(CONFIG_SHARED)  += log2_tab.o reverse.o
+OBJS-$(CONFIG_SHARED)  += log2_tab.o
 OBJS-$(CONFIG_SINEWIN) += sinewin.o sinewin_fixed.o
 OBJS-$(CONFIG_SNAPPY)  += snappy.o
 OBJS-$(CONFIG_STARTCODE)   += startcode.o
diff --git a/libavcodec/asvdec.c b/libavcodec/asvdec.c
index 9a11446f52..11d1564db3 100644
--- a/libavcodec/asvdec.c
+++ b/libavcodec/asvdec.c
@@ -71,7 +71,7 @@ static av_cold void init_vlcs(ASV1Context *a)
 // FIXME write a reversed bitstream reader to avoid the double reverse
 static inline int asv2_get_bits(GetBitContext *gb, int n)
 {
-return ff_reverse[get_bits(gb, n) << (8 - n)];
+return avpriv_reverse[get_bits(gb, n) << (8 - n)];
 }
 
 static inline int asv1_get_level(GetBitContext *gb)
@@ -229,7 +229,7 @@ static int decode_frame(AVCodecContext *avctx, void *data, 
int *got_frame,
 } else {
 int i;
 for (i = 0; i < buf_size; i++)
-a->bitstream_buffer[i] = ff_reverse[buf[i]];
+a->bitstream_buffer[i] = avpriv_reverse[buf[i]];
 }
 
 init_get_bits(>gb, a->bitstream_buffer, buf_size * 8);
diff --git a/libavcodec/asvenc.c b/libavcodec/asvenc.c
index c4eca2a13d..7ed932c934 100644
--- a/libavcodec/asvenc.c
+++ b/libavcodec/asvenc.c
@@ -37,7 +37,7 @@
 
 static inline void asv2_put_bits(PutBitContext *pb, int n, int v)
 {
-put_bits(pb, n, ff_reverse[v << (8 - n)]);
+put_bits(pb, n, avpriv_reverse[v << (8 - n)]);
 }
 
 static inline void asv1_put_level(PutBitContext *pb, int level)
@@ -306,7 +306,7 @@ static int encode_frame(AVCodecContext *avctx, AVPacket 
*pkt,
 } else {
 int i;
 for (i = 0; i < 4 * size; i++)
-pkt->data[i] = ff_reverse[pkt->data[i]];
+pkt->data[i] = avpriv_reverse[pkt->data[i]];
 }
 
 pkt->size   = size * 4;
diff --git a/libavcodec/dcaenc.c b/libavcodec/dcaenc.c
index dd601ffae0..b111fe98ba 100644
--- a/libavcodec/dcaenc.c
+++ b/libavcodec/dcaenc.c
@@ -446,7 +446,7 @@ static void fft(const int32_t in[2 * 256], cplx32 out[256])
 }
 /* post-rotation */
 for (i = 0; i < 256; i++) {
-int b = ff_reverse[i];
+int b = avpriv_reverse[i];
 rout[i].re = mul32(buf[b].re, cos_t(4 * i))
- mul32(buf[b].im, sin_t(4 * i));
 rout[i].im = mul32(buf[b].im, cos_t(4 * i))
diff --git a/libavcodec/dsd.c b/libavcodec/dsd.c
index 9104f38476..2233adf349 100644
--- a/libavcodec/dsd.c
+++ b/libavcodec/dsd.c
@@ -63,11 +63,11 @@ void ff_dsd2pcm_translate(DSDContext* s, size_t samples, 
int lsbf,
 pos = s->pos;
 
 while (samples-- > 0) {
-s->buf[pos] = lsbf ? ff_reverse[*src] : *src;
+s->buf[pos] = lsbf ? avpriv_reverse[*src] : *src;
 src += src_stride;
 
 p = s->buf + ((pos - CTABLES) & FIFOMASK);
-*p = ff_reverse[*p];
+*p = avpriv_reverse[*p];
 
 sum = 0.0;
 for (i = 0; i < CTABLES; i++) {
diff --git a/libavcodec/dsddec.c b/libavcodec/dsddec.c
index 2c5c357acc..3974bdeeb2 100644
--- a/libavcodec/dsddec.c