diff mbox series

[FFmpeg-devel,2/9] avcodec/opustab: Avoid indirection to access ff_celt_window

Message ID GV1P250MB073784A833E61A9A74617D148F5B9@GV1P250MB0737.EURP250.PROD.OUTLOOK.COM
State Accepted
Commit e10e27a2ead8848648b29a1b397cc240206e9c3d
Headers show
Series [FFmpeg-devel,1/9] avutil/attributes_internal: Add visibility pragma | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Andreas Rheinhardt Oct. 3, 2022, 10:44 p.m. UTC
Currently, it is accessed via a pointer (ff_celt_window)
exported from opustab.h which points inside a static array
(ff_celt_window_padded) in opustab.h. Instead export
ff_celt_window_padded directly and make opustab.h
a static const pointer pointing inside ff_celt_window_padded.
Also mark all the declarations in opustab.h as hidden,
so that the compiler knows that ff_celt_window has a fixed
offset from the code even when compiling position-independent
code.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/opusenc.c | 4 ++--
 libavcodec/opustab.c | 4 +---
 libavcodec/opustab.h | 8 +++++++-
 3 files changed, 10 insertions(+), 6 deletions(-)

Comments

Michael Niedermayer Oct. 4, 2022, 12:14 p.m. UTC | #1
On Tue, Oct 04, 2022 at 12:44:58AM +0200, Andreas Rheinhardt wrote:
> Currently, it is accessed via a pointer (ff_celt_window)
> exported from opustab.h which points inside a static array
> (ff_celt_window_padded) in opustab.h. Instead export
> ff_celt_window_padded directly and make opustab.h
> a static const pointer pointing inside ff_celt_window_padded.
> Also mark all the declarations in opustab.h as hidden,
> so that the compiler knows that ff_celt_window has a fixed
> offset from the code even when compiling position-independent
> code.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavcodec/opusenc.c | 4 ++--
>  libavcodec/opustab.c | 4 +---
>  libavcodec/opustab.h | 8 +++++++-
>  3 files changed, 10 insertions(+), 6 deletions(-)
[...]
> diff --git a/libavcodec/opustab.h b/libavcodec/opustab.h
> index 16011db758..9c9f1b9d98 100644
> --- a/libavcodec/opustab.h
> +++ b/libavcodec/opustab.h
> @@ -25,6 +25,9 @@
>  
>  #include <stdint.h>
>  
> +#include "libavutil/attributes_internal.h"
> +
> +FF_VISIBILITY_PUSH_HIDDEN
>  extern const uint8_t  ff_celt_band_end[];
>  
>  extern const uint8_t  ff_opus_default_coupled_streams[];

didnt investigate but this breaks mingw64 here

CC	libavcodec/opus.o
In file included from src/libavcodec/opus.c:34:0:
src/libavcodec/opustab.h:31:1: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘extern’
 extern const uint8_t  ff_celt_band_end[];
 ^~~~~~
In file included from src/libavcodec/opus.c:35:0:
src/libavcodec/internal.h:52:1: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘typedef’
 typedef struct AVCodecInternal {
 ^~~~~~~
src/libavcodec/opus.c: In function ‘ff_opus_parse_extradata’:
src/libavcodec/opus.c:333:24: error: ‘struct AVCodecInternal’ has no member named ‘skip_samples’
         avctx->internal->skip_samples = avctx->delay;
                        ^~
src/ffbuild/common.mak:81: recipe for target 'libavcodec/opus.o' failed
make: *** [libavcodec/opus.o] Error 1



[...]
Andreas Rheinhardt Oct. 4, 2022, 12:18 p.m. UTC | #2
Michael Niedermayer:
> On Tue, Oct 04, 2022 at 12:44:58AM +0200, Andreas Rheinhardt wrote:
>> Currently, it is accessed via a pointer (ff_celt_window)
>> exported from opustab.h which points inside a static array
>> (ff_celt_window_padded) in opustab.h. Instead export
>> ff_celt_window_padded directly and make opustab.h
>> a static const pointer pointing inside ff_celt_window_padded.
>> Also mark all the declarations in opustab.h as hidden,
>> so that the compiler knows that ff_celt_window has a fixed
>> offset from the code even when compiling position-independent
>> code.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>>  libavcodec/opusenc.c | 4 ++--
>>  libavcodec/opustab.c | 4 +---
>>  libavcodec/opustab.h | 8 +++++++-
>>  3 files changed, 10 insertions(+), 6 deletions(-)
> [...]
>> diff --git a/libavcodec/opustab.h b/libavcodec/opustab.h
>> index 16011db758..9c9f1b9d98 100644
>> --- a/libavcodec/opustab.h
>> +++ b/libavcodec/opustab.h
>> @@ -25,6 +25,9 @@
>>  
>>  #include <stdint.h>
>>  
>> +#include "libavutil/attributes_internal.h"
>> +
>> +FF_VISIBILITY_PUSH_HIDDEN
>>  extern const uint8_t  ff_celt_band_end[];
>>  
>>  extern const uint8_t  ff_opus_default_coupled_streams[];
> 
> didnt investigate but this breaks mingw64 here
> 
> CC	libavcodec/opus.o
> In file included from src/libavcodec/opus.c:34:0:
> src/libavcodec/opustab.h:31:1: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘extern’
>  extern const uint8_t  ff_celt_band_end[];
>  ^~~~~~
> In file included from src/libavcodec/opus.c:35:0:
> src/libavcodec/internal.h:52:1: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘typedef’
>  typedef struct AVCodecInternal {
>  ^~~~~~~
> src/libavcodec/opus.c: In function ‘ff_opus_parse_extradata’:
> src/libavcodec/opus.c:333:24: error: ‘struct AVCodecInternal’ has no member named ‘skip_samples’
>          avctx->internal->skip_samples = avctx->delay;
>                         ^~
> src/ffbuild/common.mak:81: recipe for target 'libavcodec/opus.o' failed
> make: *** [libavcodec/opus.o] Error 1
> 
> 

I already see the problem: An earlier version used
FF_VISIBILITY_START/END_HIDDEN, then I decided to switch to
FF_VISIBILITY_PUSH/POP_HIDDEN, but I only modified the part where hidden
is supported; in the other case, FF_VISIBILITY_START/END_HIDDEN is still
defined.
Thanks for testing.

- Andreas
diff mbox series

Patch

diff --git a/libavcodec/opusenc.c b/libavcodec/opusenc.c
index 58e3f5fa38..06f71876eb 100644
--- a/libavcodec/opusenc.c
+++ b/libavcodec/opusenc.c
@@ -203,7 +203,7 @@  static void celt_frame_mdct(OpusEncContext *s, CeltFrame *f)
                 float *src2 = &b->samples[CELT_OVERLAP*t];
                 s->dsp->vector_fmul(win, src1, ff_celt_window, 128);
                 s->dsp->vector_fmul_reverse(&win[CELT_OVERLAP], src2,
-                                            ff_celt_window - 8, 128);
+                                            ff_celt_window_padded, 128);
                 src1 = src2;
                 s->tx_fn[0](s->tx[0], b->coeffs + t, win, sizeof(float)*f->blocks);
             }
@@ -224,7 +224,7 @@  static void celt_frame_mdct(OpusEncContext *s, CeltFrame *f)
 
             /* Samples, windowed */
             s->dsp->vector_fmul_reverse(temp, b->samples + rwin,
-                                        ff_celt_window - 8, 128);
+                                        ff_celt_window_padded, 128);
             memcpy(win + lap_dst + blk_len, temp, CELT_OVERLAP*sizeof(float));
 
             s->tx_fn[f->size](s->tx[f->size], b->coeffs, win, sizeof(float));
diff --git a/libavcodec/opustab.c b/libavcodec/opustab.c
index b23ca4feac..2a57511177 100644
--- a/libavcodec/opustab.c
+++ b/libavcodec/opustab.c
@@ -1101,7 +1101,7 @@  const float ff_celt_postfilter_taps[3][3] = {
     { 0.7998046875f, 0.1000976562f, 0.0           }
 };
 
-DECLARE_ALIGNED(32, static const float, ff_celt_window_padded)[136] = {
+DECLARE_ALIGNED(32, const float, ff_celt_window_padded)[136] = {
     0.00000000f, 0.00000000f, 0.00000000f, 0.00000000f,
     0.00000000f, 0.00000000f, 0.00000000f, 0.00000000f,
     6.7286966e-05f, 0.00060551348f, 0.0016815970f, 0.0032947962f, 0.0054439943f,
@@ -1132,8 +1132,6 @@  DECLARE_ALIGNED(32, static const float, ff_celt_window_padded)[136] = {
     1.00000000f, 1.00000000f, 1.00000000f,
 };
 
-const float *const ff_celt_window = &ff_celt_window_padded[8];
-
 /* square of the window, used for the postfilter */
 const float ff_celt_window2[120] = {
     4.5275357e-09f, 3.66647e-07f, 2.82777e-06f, 1.08557e-05f, 2.96371e-05f, 6.60594e-05f,
diff --git a/libavcodec/opustab.h b/libavcodec/opustab.h
index 16011db758..9c9f1b9d98 100644
--- a/libavcodec/opustab.h
+++ b/libavcodec/opustab.h
@@ -25,6 +25,9 @@ 
 
 #include <stdint.h>
 
+#include "libavutil/attributes_internal.h"
+
+FF_VISIBILITY_PUSH_HIDDEN
 extern const uint8_t  ff_celt_band_end[];
 
 extern const uint8_t  ff_opus_default_coupled_streams[];
@@ -154,8 +157,11 @@  extern const uint16_t ff_celt_qn_exp2[];
 extern const float    ff_celt_postfilter_taps[3][3];
 
 extern const float    ff_celt_window2[120];
-extern const float *const ff_celt_window;
+
+extern const float    ff_celt_window_padded[];
+static const float *const ff_celt_window = &ff_celt_window_padded[8];
 
 extern const uint32_t * const ff_celt_pvq_u_row[15];
+FF_VISIBILITY_POP_HIDDEN
 
 #endif /* AVCODEC_OPUSTAB_H */