Re: [FFmpeg-devel] [PATCH] avcodec/ac3: always use hardcoded tables
On Mon, Nov 30, 2015 at 9:35 AM, Ganesh Ajjanagadde wrote:
> On Mon, Nov 30, 2015 at 6:39 AM, wm4 wrote:
>> On Sun, 29 Nov 2015 20:40:11 -0500
>> Ganesh Ajjanagadde wrote:
>>
>>> The table in question is a 253 byte one. In fact, it turns out that
>>> dynamic generation of the table results in an increased binary size.
>>>
>>> Code compiled with GCC 5.2.0, x86-64 (size in bytes), before and after
>>> patch:
>>> old: 62321064 libavcodec/libavcodec.so.57
>>> new: 62320536 libavcodec/libavcodec.so.57
>>>
>>> Thus, it always make sense to statically allocate this.
>>>
>>> Signed-off-by: Ganesh Ajjanagadde
>>> ---
>>> libavcodec/ac3.c| 24
>>> libavcodec/ac3dec.c | 1 -
>>> libavcodec/ac3enc.c | 2 --
>>> libavcodec/ac3tab.h | 8 +---
>>> 4 files changed, 1 insertion(+), 34 deletions(-)
>>>
>>> diff --git a/libavcodec/ac3.c b/libavcodec/ac3.c
>>> index b54315d..1d4eaa5 100644
>>> --- a/libavcodec/ac3.c
>>> +++ b/libavcodec/ac3.c
>>> @@ -39,8 +39,6 @@ const uint8_t ff_ac3_band_start_tab[AC3_CRITICAL_BANDS+1]
>>> = {
>>> 79, 85, 97, 109, 121, 133, 157, 181, 205, 229, 253
>>> };
>>>
>>> -#if CONFIG_HARDCODED_TABLES
>>> -
>>> /**
>>> * Map each frequency coefficient bin to the critical band that contains
>>> it.
>>> */
>>> @@ -69,10 +67,6 @@ const uint8_t ff_ac3_bin_to_band_tab[253] = {
>>> 49, 49, 49, 49, 49, 49, 49, 49, 49, 49, 49, 49
>>> };
>>>
>>> -#else /* CONFIG_HARDCODED_TABLES */
>>> -uint8_t ff_ac3_bin_to_band_tab[253];
>>> -#endif
>>> -
>>> static inline int calc_lowcomp1(int a, int b0, int b1, int c)
>>> {
>>> if ((b0 + 256) == b1) {
>>> @@ -214,21 +208,3 @@ int ff_ac3_bit_alloc_calc_mask(AC3BitAllocParameters
>>> *s, int16_t *band_psd,
>>> }
>>> return 0;
>>> }
>>> -
>>> -/**
>>> - * Initialize some tables.
>>> - * note: This function must remain thread safe because it is called by the
>>> - * AVParser init code.
>>> - */
>>> -av_cold void ff_ac3_common_init(void)
>>> -{
>>> -#if !CONFIG_HARDCODED_TABLES
>>> -/* compute ff_ac3_bin_to_band_tab from ff_ac3_band_start_tab */
>>> -int bin = 0, band;
>>> -for (band = 0; band < AC3_CRITICAL_BANDS; band++) {
>>> -int band_end = ff_ac3_band_start_tab[band+1];
>>> -while (bin < band_end)
>>> -ff_ac3_bin_to_band_tab[bin++] = band;
>>> -}
>>> -#endif /* !CONFIG_HARDCODED_TABLES */
>>> -}
>>> diff --git a/libavcodec/ac3dec.c b/libavcodec/ac3dec.c
>>> index ad91405..efc58e5 100644
>>> --- a/libavcodec/ac3dec.c
>>> +++ b/libavcodec/ac3dec.c
>>> @@ -185,7 +185,6 @@ static av_cold int ac3_decode_init(AVCodecContext
>>> *avctx)
>>>
>>> s->avctx = avctx;
>>>
>>> -ff_ac3_common_init();
>>> ac3_tables_init();
>>> ff_mdct_init(&s->imdct_256, 8, 1, 1.0);
>>> ff_mdct_init(&s->imdct_512, 9, 1, 1.0);
>>> diff --git a/libavcodec/ac3enc.c b/libavcodec/ac3enc.c
>>> index c8a0caa..636ca72 100644
>>> --- a/libavcodec/ac3enc.c
>>> +++ b/libavcodec/ac3enc.c
>>> @@ -2431,8 +2431,6 @@ av_cold int ff_ac3_encode_init(AVCodecContext *avctx)
>>>
>>> s->eac3 = avctx->codec_id == AV_CODEC_ID_EAC3;
>>>
>>> -ff_ac3_common_init();
>>> -
>>> ret = validate_options(s);
>>> if (ret)
>>> return ret;
>>> diff --git a/libavcodec/ac3tab.h b/libavcodec/ac3tab.h
>>> index 74cbd9e..f529fc8 100644
>>> --- a/libavcodec/ac3tab.h
>>> +++ b/libavcodec/ac3tab.h
>>> @@ -27,12 +27,6 @@
>>> #include "libavutil/internal.h"
>>> #include "ac3.h"
>>>
>>> -#if CONFIG_HARDCODED_TABLES
>>> -# define HCONST const
>>> -#else
>>> -# define HCONST
>>> -#endif
>>> -
>>> extern const uint16_t ff_ac3_frame_size_tab[38][3];
>>> extern const uint8_t ff_ac3_channels_tab[8];
>>> extern av_export const uint16_t avpriv_ac3_channel_layout_tab[8];
>>> @@ -54,7 +48,7 @@ extern const int16_t ff_ac3_floor_tab[8];
>>> extern const uint16_t ff_ac3_fast_gain_tab[8];
>>> extern const uint16_t ff_eac3_default_chmap[8];
>>> extern const uint8_t ff_ac3_band_start_tab[AC3_CRITICAL_BANDS+1];
>>> -extern HCONST uint8_t ff_ac3_bin_to_band_tab[253];
>>> +extern const uint8_t ff_ac3_bin_to_band_tab[253];
>>>
>>> /** Custom channel map locations bitmask
>>> * Other channels described in documentation:
>>
>> Seems fine if it passes FATE. (Note that I'm not AC3 maintainer.)
>
> Maintainer seems to be Justin Ruggles, who has been inactive since
> last year, and moreover works for Libav. Thanks for the review; will
> push later.
pushed
>
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/ac3: always use hardcoded tables
On Mon, Nov 30, 2015 at 6:39 AM, wm4 wrote:
> On Sun, 29 Nov 2015 20:40:11 -0500
> Ganesh Ajjanagadde wrote:
>
>> The table in question is a 253 byte one. In fact, it turns out that
>> dynamic generation of the table results in an increased binary size.
>>
>> Code compiled with GCC 5.2.0, x86-64 (size in bytes), before and after
>> patch:
>> old: 62321064 libavcodec/libavcodec.so.57
>> new: 62320536 libavcodec/libavcodec.so.57
>>
>> Thus, it always make sense to statically allocate this.
>>
>> Signed-off-by: Ganesh Ajjanagadde
>> ---
>> libavcodec/ac3.c| 24
>> libavcodec/ac3dec.c | 1 -
>> libavcodec/ac3enc.c | 2 --
>> libavcodec/ac3tab.h | 8 +---
>> 4 files changed, 1 insertion(+), 34 deletions(-)
>>
>> diff --git a/libavcodec/ac3.c b/libavcodec/ac3.c
>> index b54315d..1d4eaa5 100644
>> --- a/libavcodec/ac3.c
>> +++ b/libavcodec/ac3.c
>> @@ -39,8 +39,6 @@ const uint8_t ff_ac3_band_start_tab[AC3_CRITICAL_BANDS+1]
>> = {
>> 79, 85, 97, 109, 121, 133, 157, 181, 205, 229, 253
>> };
>>
>> -#if CONFIG_HARDCODED_TABLES
>> -
>> /**
>> * Map each frequency coefficient bin to the critical band that contains it.
>> */
>> @@ -69,10 +67,6 @@ const uint8_t ff_ac3_bin_to_band_tab[253] = {
>> 49, 49, 49, 49, 49, 49, 49, 49, 49, 49, 49, 49
>> };
>>
>> -#else /* CONFIG_HARDCODED_TABLES */
>> -uint8_t ff_ac3_bin_to_band_tab[253];
>> -#endif
>> -
>> static inline int calc_lowcomp1(int a, int b0, int b1, int c)
>> {
>> if ((b0 + 256) == b1) {
>> @@ -214,21 +208,3 @@ int ff_ac3_bit_alloc_calc_mask(AC3BitAllocParameters
>> *s, int16_t *band_psd,
>> }
>> return 0;
>> }
>> -
>> -/**
>> - * Initialize some tables.
>> - * note: This function must remain thread safe because it is called by the
>> - * AVParser init code.
>> - */
>> -av_cold void ff_ac3_common_init(void)
>> -{
>> -#if !CONFIG_HARDCODED_TABLES
>> -/* compute ff_ac3_bin_to_band_tab from ff_ac3_band_start_tab */
>> -int bin = 0, band;
>> -for (band = 0; band < AC3_CRITICAL_BANDS; band++) {
>> -int band_end = ff_ac3_band_start_tab[band+1];
>> -while (bin < band_end)
>> -ff_ac3_bin_to_band_tab[bin++] = band;
>> -}
>> -#endif /* !CONFIG_HARDCODED_TABLES */
>> -}
>> diff --git a/libavcodec/ac3dec.c b/libavcodec/ac3dec.c
>> index ad91405..efc58e5 100644
>> --- a/libavcodec/ac3dec.c
>> +++ b/libavcodec/ac3dec.c
>> @@ -185,7 +185,6 @@ static av_cold int ac3_decode_init(AVCodecContext *avctx)
>>
>> s->avctx = avctx;
>>
>> -ff_ac3_common_init();
>> ac3_tables_init();
>> ff_mdct_init(&s->imdct_256, 8, 1, 1.0);
>> ff_mdct_init(&s->imdct_512, 9, 1, 1.0);
>> diff --git a/libavcodec/ac3enc.c b/libavcodec/ac3enc.c
>> index c8a0caa..636ca72 100644
>> --- a/libavcodec/ac3enc.c
>> +++ b/libavcodec/ac3enc.c
>> @@ -2431,8 +2431,6 @@ av_cold int ff_ac3_encode_init(AVCodecContext *avctx)
>>
>> s->eac3 = avctx->codec_id == AV_CODEC_ID_EAC3;
>>
>> -ff_ac3_common_init();
>> -
>> ret = validate_options(s);
>> if (ret)
>> return ret;
>> diff --git a/libavcodec/ac3tab.h b/libavcodec/ac3tab.h
>> index 74cbd9e..f529fc8 100644
>> --- a/libavcodec/ac3tab.h
>> +++ b/libavcodec/ac3tab.h
>> @@ -27,12 +27,6 @@
>> #include "libavutil/internal.h"
>> #include "ac3.h"
>>
>> -#if CONFIG_HARDCODED_TABLES
>> -# define HCONST const
>> -#else
>> -# define HCONST
>> -#endif
>> -
>> extern const uint16_t ff_ac3_frame_size_tab[38][3];
>> extern const uint8_t ff_ac3_channels_tab[8];
>> extern av_export const uint16_t avpriv_ac3_channel_layout_tab[8];
>> @@ -54,7 +48,7 @@ extern const int16_t ff_ac3_floor_tab[8];
>> extern const uint16_t ff_ac3_fast_gain_tab[8];
>> extern const uint16_t ff_eac3_default_chmap[8];
>> extern const uint8_t ff_ac3_band_start_tab[AC3_CRITICAL_BANDS+1];
>> -extern HCONST uint8_t ff_ac3_bin_to_band_tab[253];
>> +extern const uint8_t ff_ac3_bin_to_band_tab[253];
>>
>> /** Custom channel map locations bitmask
>> * Other channels described in documentation:
>
> Seems fine if it passes FATE. (Note that I'm not AC3 maintainer.)
Maintainer seems to be Justin Ruggles, who has been inactive since
last year, and moreover works for Libav. Thanks for the review; will
push later.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/ac3: always use hardcoded tables
On Sun, 29 Nov 2015 20:40:11 -0500
Ganesh Ajjanagadde wrote:
> The table in question is a 253 byte one. In fact, it turns out that
> dynamic generation of the table results in an increased binary size.
>
> Code compiled with GCC 5.2.0, x86-64 (size in bytes), before and after
> patch:
> old: 62321064 libavcodec/libavcodec.so.57
> new: 62320536 libavcodec/libavcodec.so.57
>
> Thus, it always make sense to statically allocate this.
>
> Signed-off-by: Ganesh Ajjanagadde
> ---
> libavcodec/ac3.c| 24
> libavcodec/ac3dec.c | 1 -
> libavcodec/ac3enc.c | 2 --
> libavcodec/ac3tab.h | 8 +---
> 4 files changed, 1 insertion(+), 34 deletions(-)
>
> diff --git a/libavcodec/ac3.c b/libavcodec/ac3.c
> index b54315d..1d4eaa5 100644
> --- a/libavcodec/ac3.c
> +++ b/libavcodec/ac3.c
> @@ -39,8 +39,6 @@ const uint8_t ff_ac3_band_start_tab[AC3_CRITICAL_BANDS+1] =
> {
> 79, 85, 97, 109, 121, 133, 157, 181, 205, 229, 253
> };
>
> -#if CONFIG_HARDCODED_TABLES
> -
> /**
> * Map each frequency coefficient bin to the critical band that contains it.
> */
> @@ -69,10 +67,6 @@ const uint8_t ff_ac3_bin_to_band_tab[253] = {
> 49, 49, 49, 49, 49, 49, 49, 49, 49, 49, 49, 49
> };
>
> -#else /* CONFIG_HARDCODED_TABLES */
> -uint8_t ff_ac3_bin_to_band_tab[253];
> -#endif
> -
> static inline int calc_lowcomp1(int a, int b0, int b1, int c)
> {
> if ((b0 + 256) == b1) {
> @@ -214,21 +208,3 @@ int ff_ac3_bit_alloc_calc_mask(AC3BitAllocParameters *s,
> int16_t *band_psd,
> }
> return 0;
> }
> -
> -/**
> - * Initialize some tables.
> - * note: This function must remain thread safe because it is called by the
> - * AVParser init code.
> - */
> -av_cold void ff_ac3_common_init(void)
> -{
> -#if !CONFIG_HARDCODED_TABLES
> -/* compute ff_ac3_bin_to_band_tab from ff_ac3_band_start_tab */
> -int bin = 0, band;
> -for (band = 0; band < AC3_CRITICAL_BANDS; band++) {
> -int band_end = ff_ac3_band_start_tab[band+1];
> -while (bin < band_end)
> -ff_ac3_bin_to_band_tab[bin++] = band;
> -}
> -#endif /* !CONFIG_HARDCODED_TABLES */
> -}
> diff --git a/libavcodec/ac3dec.c b/libavcodec/ac3dec.c
> index ad91405..efc58e5 100644
> --- a/libavcodec/ac3dec.c
> +++ b/libavcodec/ac3dec.c
> @@ -185,7 +185,6 @@ static av_cold int ac3_decode_init(AVCodecContext *avctx)
>
> s->avctx = avctx;
>
> -ff_ac3_common_init();
> ac3_tables_init();
> ff_mdct_init(&s->imdct_256, 8, 1, 1.0);
> ff_mdct_init(&s->imdct_512, 9, 1, 1.0);
> diff --git a/libavcodec/ac3enc.c b/libavcodec/ac3enc.c
> index c8a0caa..636ca72 100644
> --- a/libavcodec/ac3enc.c
> +++ b/libavcodec/ac3enc.c
> @@ -2431,8 +2431,6 @@ av_cold int ff_ac3_encode_init(AVCodecContext *avctx)
>
> s->eac3 = avctx->codec_id == AV_CODEC_ID_EAC3;
>
> -ff_ac3_common_init();
> -
> ret = validate_options(s);
> if (ret)
> return ret;
> diff --git a/libavcodec/ac3tab.h b/libavcodec/ac3tab.h
> index 74cbd9e..f529fc8 100644
> --- a/libavcodec/ac3tab.h
> +++ b/libavcodec/ac3tab.h
> @@ -27,12 +27,6 @@
> #include "libavutil/internal.h"
> #include "ac3.h"
>
> -#if CONFIG_HARDCODED_TABLES
> -# define HCONST const
> -#else
> -# define HCONST
> -#endif
> -
> extern const uint16_t ff_ac3_frame_size_tab[38][3];
> extern const uint8_t ff_ac3_channels_tab[8];
> extern av_export const uint16_t avpriv_ac3_channel_layout_tab[8];
> @@ -54,7 +48,7 @@ extern const int16_t ff_ac3_floor_tab[8];
> extern const uint16_t ff_ac3_fast_gain_tab[8];
> extern const uint16_t ff_eac3_default_chmap[8];
> extern const uint8_t ff_ac3_band_start_tab[AC3_CRITICAL_BANDS+1];
> -extern HCONST uint8_t ff_ac3_bin_to_band_tab[253];
> +extern const uint8_t ff_ac3_bin_to_band_tab[253];
>
> /** Custom channel map locations bitmask
> * Other channels described in documentation:
Seems fine if it passes FATE. (Note that I'm not AC3 maintainer.)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avcodec/ac3: always use hardcoded tables
The table in question is a 253 byte one. In fact, it turns out that
dynamic generation of the table results in an increased binary size.
Code compiled with GCC 5.2.0, x86-64 (size in bytes), before and after
patch:
old: 62321064 libavcodec/libavcodec.so.57
new: 62320536 libavcodec/libavcodec.so.57
Thus, it always make sense to statically allocate this.
Signed-off-by: Ganesh Ajjanagadde
---
libavcodec/ac3.c| 24
libavcodec/ac3dec.c | 1 -
libavcodec/ac3enc.c | 2 --
libavcodec/ac3tab.h | 8 +---
4 files changed, 1 insertion(+), 34 deletions(-)
diff --git a/libavcodec/ac3.c b/libavcodec/ac3.c
index b54315d..1d4eaa5 100644
--- a/libavcodec/ac3.c
+++ b/libavcodec/ac3.c
@@ -39,8 +39,6 @@ const uint8_t ff_ac3_band_start_tab[AC3_CRITICAL_BANDS+1] = {
79, 85, 97, 109, 121, 133, 157, 181, 205, 229, 253
};
-#if CONFIG_HARDCODED_TABLES
-
/**
* Map each frequency coefficient bin to the critical band that contains it.
*/
@@ -69,10 +67,6 @@ const uint8_t ff_ac3_bin_to_band_tab[253] = {
49, 49, 49, 49, 49, 49, 49, 49, 49, 49, 49, 49
};
-#else /* CONFIG_HARDCODED_TABLES */
-uint8_t ff_ac3_bin_to_band_tab[253];
-#endif
-
static inline int calc_lowcomp1(int a, int b0, int b1, int c)
{
if ((b0 + 256) == b1) {
@@ -214,21 +208,3 @@ int ff_ac3_bit_alloc_calc_mask(AC3BitAllocParameters *s,
int16_t *band_psd,
}
return 0;
}
-
-/**
- * Initialize some tables.
- * note: This function must remain thread safe because it is called by the
- * AVParser init code.
- */
-av_cold void ff_ac3_common_init(void)
-{
-#if !CONFIG_HARDCODED_TABLES
-/* compute ff_ac3_bin_to_band_tab from ff_ac3_band_start_tab */
-int bin = 0, band;
-for (band = 0; band < AC3_CRITICAL_BANDS; band++) {
-int band_end = ff_ac3_band_start_tab[band+1];
-while (bin < band_end)
-ff_ac3_bin_to_band_tab[bin++] = band;
-}
-#endif /* !CONFIG_HARDCODED_TABLES */
-}
diff --git a/libavcodec/ac3dec.c b/libavcodec/ac3dec.c
index ad91405..efc58e5 100644
--- a/libavcodec/ac3dec.c
+++ b/libavcodec/ac3dec.c
@@ -185,7 +185,6 @@ static av_cold int ac3_decode_init(AVCodecContext *avctx)
s->avctx = avctx;
-ff_ac3_common_init();
ac3_tables_init();
ff_mdct_init(&s->imdct_256, 8, 1, 1.0);
ff_mdct_init(&s->imdct_512, 9, 1, 1.0);
diff --git a/libavcodec/ac3enc.c b/libavcodec/ac3enc.c
index c8a0caa..636ca72 100644
--- a/libavcodec/ac3enc.c
+++ b/libavcodec/ac3enc.c
@@ -2431,8 +2431,6 @@ av_cold int ff_ac3_encode_init(AVCodecContext *avctx)
s->eac3 = avctx->codec_id == AV_CODEC_ID_EAC3;
-ff_ac3_common_init();
-
ret = validate_options(s);
if (ret)
return ret;
diff --git a/libavcodec/ac3tab.h b/libavcodec/ac3tab.h
index 74cbd9e..f529fc8 100644
--- a/libavcodec/ac3tab.h
+++ b/libavcodec/ac3tab.h
@@ -27,12 +27,6 @@
#include "libavutil/internal.h"
#include "ac3.h"
-#if CONFIG_HARDCODED_TABLES
-# define HCONST const
-#else
-# define HCONST
-#endif
-
extern const uint16_t ff_ac3_frame_size_tab[38][3];
extern const uint8_t ff_ac3_channels_tab[8];
extern av_export const uint16_t avpriv_ac3_channel_layout_tab[8];
@@ -54,7 +48,7 @@ extern const int16_t ff_ac3_floor_tab[8];
extern const uint16_t ff_ac3_fast_gain_tab[8];
extern const uint16_t ff_eac3_default_chmap[8];
extern const uint8_t ff_ac3_band_start_tab[AC3_CRITICAL_BANDS+1];
-extern HCONST uint8_t ff_ac3_bin_to_band_tab[253];
+extern const uint8_t ff_ac3_bin_to_band_tab[253];
/** Custom channel map locations bitmask
* Other channels described in documentation:
--
2.6.2
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
