[FFmpeg-devel] [PATCH] Move cbrt tables to a separate cbrt_data(_fixed).c files.

Hendrik Leppkes h.leppkes at gmail.com
Sun Mar 13 17:50:17 CET 2016


On Sun, Mar 13, 2016 at 5:24 PM, Ganesh Ajjanagadde <gajjanag at gmail.com> wrote:
> On Sat, Mar 12, 2016 at 1:24 PM, Reimar Döffinger
> <Reimar.Doeffinger at gmx.de> wrote:
>> Allows sharing and reusing the data between different files.
>>
>> Signed-off-by: Reimar Döffinger <Reimar.Doeffinger at gmx.de>
>> ---
>>  libavcodec/Makefile                 | 10 +++++-----
>>  libavcodec/aacdec.c                 |  2 +-
>>  libavcodec/aacdec_fixed.c           |  6 +++---
>>  libavcodec/aacdec_template.c        |  4 ++--
>>  libavcodec/cbrt_data.c              | 28 +++++++++++++++++++++++++++
>>  libavcodec/cbrt_data.h              | 38 +++++++++++++++++++++++++++++++++++++
>>  libavcodec/cbrt_data_fixed.c        | 29 ++++++++++++++++++++++++++++
>>  libavcodec/cbrt_tablegen.h          | 18 ++++--------------
>>  libavcodec/cbrt_tablegen_template.c |  8 ++++++--
>>  9 files changed, 116 insertions(+), 27 deletions(-)
>>  create mode 100644 libavcodec/cbrt_data.c
>>  create mode 100644 libavcodec/cbrt_data.h
>>  create mode 100644 libavcodec/cbrt_data_fixed.c
>>
>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>> index 1cd9572..6bb1af1 100644
>> --- a/libavcodec/Makefile
>> +++ b/libavcodec/Makefile
>> @@ -137,17 +137,17 @@ OBJS-$(CONFIG_A64MULTI_ENCODER)        += a64multienc.o elbg.o
>>  OBJS-$(CONFIG_A64MULTI5_ENCODER)       += a64multienc.o elbg.o
>>  OBJS-$(CONFIG_AAC_DECODER)             += aacdec.o aactab.o aacsbr.o aacps_float.o \
>>                                            aacadtsdec.o mpeg4audio.o kbdwin.o \
>> -                                          sbrdsp.o aacpsdsp_float.o
>> +                                          sbrdsp.o aacpsdsp_float.o cbrt_data.o
>>  OBJS-$(CONFIG_AAC_FIXED_DECODER)       += aacdec_fixed.o aactab.o aacsbr_fixed.o aacps_fixed.o \
>>                                            aacadtsdec.o mpeg4audio.o kbdwin.o \
>> -                                          sbrdsp_fixed.o aacpsdsp_fixed.o
>> +                                          sbrdsp_fixed.o aacpsdsp_fixed.o cbrt_data_fixed.o
>>  OBJS-$(CONFIG_AAC_ENCODER)             += aacenc.o aaccoder.o aacenctab.o    \
>>                                            aacpsy.o aactab.o      \
>>                                            aacenc_is.o \
>>                                            aacenc_tns.o \
>>                                            aacenc_ltp.o \
>>                                            aacenc_pred.o \
>> -                                          psymodel.o mpeg4audio.o kbdwin.o
>> +                                          psymodel.o mpeg4audio.o kbdwin.o cbrt_data.o
>>  OBJS-$(CONFIG_AASC_DECODER)            += aasc.o msrledec.o
>>  OBJS-$(CONFIG_AC3_DECODER)             += ac3dec_float.o ac3dec_data.o ac3.o kbdwin.o
>>  OBJS-$(CONFIG_AC3_FIXED_DECODER)       += ac3dec_fixed.o ac3dec_data.o ac3.o kbdwin.o
>> @@ -1017,8 +1017,8 @@ $(GEN_HEADERS): $(SUBDIR)%_tables.h: $(SUBDIR)%_tablegen$(HOSTEXESUF)
>>         $(M)./$< > $@
>>
>>  ifdef CONFIG_HARDCODED_TABLES
>> -$(SUBDIR)aacdec.o: $(SUBDIR)cbrt_tables.h
>> -$(SUBDIR)aacdec_fixed.o: $(SUBDIR)cbrt_fixed_tables.h
>> +$(SUBDIR)cbrt_data.o: $(SUBDIR)cbrt_tables.h
>> +$(SUBDIR)cbrt_data_fixed.o: $(SUBDIR)cbrt_fixed_tables.h
>>  $(SUBDIR)aacps_float.o: $(SUBDIR)aacps_tables.h
>>  $(SUBDIR)aacps_fixed.o: $(SUBDIR)aacps_fixed_tables.h
>>  $(SUBDIR)aactab_fixed.o: $(SUBDIR)aac_fixed_tables.h
>> diff --git a/libavcodec/aacdec.c b/libavcodec/aacdec.c
>> index 26bdea1..ee9b4eb 100644
>> --- a/libavcodec/aacdec.c
>> +++ b/libavcodec/aacdec.c
>> @@ -50,7 +50,7 @@
>>  #include "aac.h"
>>  #include "aactab.h"
>>  #include "aacdectab.h"
>> -#include "cbrt_tablegen.h"
>> +#include "cbrt_data.h"
>>  #include "sbr.h"
>>  #include "aacsbr.h"
>>  #include "mpeg4audio.h"
>> diff --git a/libavcodec/aacdec_fixed.c b/libavcodec/aacdec_fixed.c
>> index 396a874..acb8178 100644
>> --- a/libavcodec/aacdec_fixed.c
>> +++ b/libavcodec/aacdec_fixed.c
>> @@ -75,7 +75,7 @@
>>  #include "aac.h"
>>  #include "aactab.h"
>>  #include "aacdectab.h"
>> -#include "cbrt_tablegen.h"
>> +#include "cbrt_data.h"
>>  #include "sbr.h"
>>  #include "aacsbr.h"
>>  #include "mpeg4audio.h"
>> @@ -155,9 +155,9 @@ static void vector_pow43(int *coefs, int len)
>>      for (i=0; i<len; i++) {
>>          coef = coefs[i];
>>          if (coef < 0)
>> -            coef = -(int)cbrt_tab[-coef];
>> +            coef = -(int)ff_cbrt_tab_fixed[-coef];
>>          else
>> -            coef = (int)cbrt_tab[coef];
>> +            coef = (int)ff_cbrt_tab_fixed[coef];
>>          coefs[i] = coef;
>>      }
>>  }
>> diff --git a/libavcodec/aacdec_template.c b/libavcodec/aacdec_template.c
>> index 6bc94c8..883ed52 100644
>> --- a/libavcodec/aacdec_template.c
>> +++ b/libavcodec/aacdec_template.c
>> @@ -1104,7 +1104,7 @@ static av_cold void aac_static_table_init(void)
>>      AAC_RENAME(ff_init_ff_sine_windows)( 9);
>>      AAC_RENAME(ff_init_ff_sine_windows)( 7);
>>
>> -    AAC_RENAME(cbrt_tableinit)();
>> +    AAC_RENAME(ff_cbrt_tableinit)();
>>  }
>>
>>  static AVOnce aac_table_init = AV_ONCE_INIT;
>> @@ -1795,7 +1795,7 @@ static int decode_spectrum_and_dequant(AACContext *ac, INTFLOAT coef[1024],
>>                                          v = -v;
>>                                      *icf++ = v;
>>  #else
>> -                                    *icf++ = cbrt_tab[n] | (bits & 1U<<31);
>> +                                    *icf++ = ff_cbrt_tab[n] | (bits & 1U<<31);
>>  #endif /* USE_FIXED */
>>                                      bits <<= 1;
>>                                  } else {
>> diff --git a/libavcodec/cbrt_data.c b/libavcodec/cbrt_data.c
>> new file mode 100644
>> index 0000000..f5d9778
>> --- /dev/null
>> +++ b/libavcodec/cbrt_data.c
>> @@ -0,0 +1,28 @@
>> +/*
>> + * Copyright (c) 2016 Reimar Döffinger <Reimar.Doeffinger at gmx.de>
>> + *
>> + * 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
>> + */
>> +
>> +#include "config.h"
>> +#include "cbrt_data.h"
>> +
>> +#if CONFIG_HARDCODED_TABLES
>> +#include "libavcodec/cbrt_tables.h"
>> +#else
>> +#include "cbrt_tablegen.h"
>> +#endif
>> diff --git a/libavcodec/cbrt_data.h b/libavcodec/cbrt_data.h
>> new file mode 100644
>> index 0000000..232f74f
>> --- /dev/null
>> +++ b/libavcodec/cbrt_data.h
>> @@ -0,0 +1,38 @@
>> +/*
>> + * Copyright (c) 2016 Reimar Döffinger <Reimar.Doeffinger at gmx.de>
>> + *
>> + * 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
>> + */
>> +
>> +#ifndef AVCODEC_CBRT_DATA_H
>> +#define AVCODEC_CBRT_DATA_H
>> +
>> +#include <stdint.h>
>> +
>> +#if CONFIG_HARDCODED_TABLES
>> +#define ff_cbrt_tableinit_fixed()
>> +#define ff_cbrt_tableinit()
>> +extern const uint32_t ff_cbrt_tab[1 << 13];
>> +extern const uint32_t ff_cbrt_tab_fixed[1 << 13];
>> +#else
>> +void ff_cbrt_tableinit(void);
>> +void ff_cbrt_tableinit_fixed(void);
>> +extern uint32_t ff_cbrt_tab[1 << 13];
>> +extern uint32_t ff_cbrt_tab_fixed[1 << 13];
>> +#endif
>> +
>> +#endif
>> diff --git a/libavcodec/cbrt_data_fixed.c b/libavcodec/cbrt_data_fixed.c
>> new file mode 100644
>> index 0000000..d661b25
>> --- /dev/null
>> +++ b/libavcodec/cbrt_data_fixed.c
>> @@ -0,0 +1,29 @@
>> +/*
>> + * Copyright (c) 2016 Reimar Döffinger <Reimar.Doeffinger at gmx.de>
>> + *
>> + * 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
>> + */
>> +
>> +#include "config.h"
>> +#include "cbrt_data.h"
>> +
>> +#if CONFIG_HARDCODED_TABLES
>> +#include "libavcodec/cbrt_fixed_tables.h"
>> +#else
>> +#define USE_FIXED 1
>> +#include "cbrt_tablegen.h"
>> +#endif
>> diff --git a/libavcodec/cbrt_tablegen.h b/libavcodec/cbrt_tablegen.h
>> index 21e4b9a..9af18d8 100644
>> --- a/libavcodec/cbrt_tablegen.h
>> +++ b/libavcodec/cbrt_tablegen.h
>> @@ -35,21 +35,12 @@
>>  #define CBRT(x) av_float2int((float)(x))
>>  #endif
>>
>> -#if CONFIG_HARDCODED_TABLES
>> -#if USE_FIXED
>> -#define cbrt_tableinit_fixed()
>> -#include "libavcodec/cbrt_fixed_tables.h"
>> -#else
>> -#define cbrt_tableinit()
>> -#include "libavcodec/cbrt_tables.h"
>> -#endif
>> -#else
>> -static uint32_t cbrt_tab[1 << 13];
>> +uint32_t AAC_RENAME(ff_cbrt_tab)[1 << 13];
>>
>> -static av_cold void AAC_RENAME(cbrt_tableinit)(void)
>> +av_cold void AAC_RENAME(ff_cbrt_tableinit)(void)
>>  {
>>      static double cbrt_tab_dbl[1 << 13];
>> -    if (!cbrt_tab[(1<<13) - 1]) {
>> +    if (!AAC_RENAME(ff_cbrt_tab)[(1<<13) - 1]) {
>>          int i, j, k;
>>          double cbrt_val;
>>
>> @@ -75,9 +66,8 @@ static av_cold void AAC_RENAME(cbrt_tableinit)(void)
>>          }
>>
>>          for (i = 0; i < 1<<13; i++)
>> -            cbrt_tab[i] = CBRT(cbrt_tab_dbl[i]);
>> +            AAC_RENAME(ff_cbrt_tab)[i] = CBRT(cbrt_tab_dbl[i]);
>>      }
>>  }
>
> Note that cbrt_tab_dbl is really intended to be shared by both the
> fixed/floating table inits. This was another thing my patch achieved:
> only doing the more expensive double table init once across
> float/fixed, and then doing the cheap conversion to uint32_t via
> av_float2int or lrint(x*8192). Please change; it could go into a
> separate patch if you prefer.
>

Having both float and fixed decoders used at the same time seems like
a rather unlikely use-case, so if such an optimization takes rather
high complexity, its probably not worth going, IMHO.

- Hendrik


More information about the ffmpeg-devel mailing list