| Message ID | GV1P250MB07376D404340C2B7ABC096108F1C2@GV1P250MB0737.EURP250.PROD.OUTLOOK.COM |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avcodec/aactab: Provide ff_ltp_coef, ff_tns_tmp2_map unconditionally | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
May 6, 2024, 11:31 by andreas.rheinhardt@outlook.com: > The approach used here has the advantage not to rely > on any DCE. > Also improve certain the checks from > 3390693bfb907765f833766f370e0ba8c7894f44 a bit. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > libavcodec/aac/aacdec.c | 62 ++++++++++++++++++++--------------------- > 1 file changed, 31 insertions(+), 31 deletions(-) > > diff --git a/libavcodec/aac/aacdec.c b/libavcodec/aac/aacdec.c > index c6b93e33a2..6a74b05168 100644 > --- a/libavcodec/aac/aacdec.c > +++ b/libavcodec/aac/aacdec.c > @@ -63,6 +63,20 @@ > #include "libavutil/version.h" > #include "libavutil/thread.h" > > +#if CONFIG_AAC_DECODER && CONFIG_AAC_FIXED_DECODER > +#define IS_FIXED(is_fixed) (is_fixed) > +#define FIXED_OR_FLOAT(is_fixed, func_or_obj, func_args) \ > + ((is_fixed) ? RENAME_FIXED(func_or_obj) func_args : (func_or_obj) func_args) > +#elif CONFIG_AAC_DECODER > +#define IS_FIXED(is_fixed) 0 > +#define FIXED_OR_FLOAT(is_fixed, func_or_obj, func_args) \ > + ((func_or_obj) func_args) > +#else > +#define IS_FIXED(is_fixed) 1 > +#define FIXED_OR_FLOAT(is_fixed, func_or_obj, func_args) \ > + (RENAME_FIXED(func_or_obj) func_args) > +#endif > + > /* > * supported tools > * > @@ -150,11 +164,8 @@ static av_cold int che_configure(AACDecContext *ac, > return AVERROR_INVALIDDATA; > if (che_pos) { > if (!ac->che[type][id]) { > - int ret; > - if (ac->is_fixed) > - ret = ff_aac_sbr_ctx_alloc_init_fixed(ac, &ac->che[type][id], type); > - else > - ret = ff_aac_sbr_ctx_alloc_init(ac, &ac->che[type][id], type); > + int ret = FIXED_OR_FLOAT(ac->is_fixed, ff_aac_sbr_ctx_alloc_init, > + (ac, &ac->che[type][id], type)); > if (ret < 0) > return ret; > } > @@ -171,10 +182,7 @@ static av_cold int che_configure(AACDecContext *ac, > } > } else { > if (ac->che[type][id]) { > - if (ac->is_fixed) > - ff_aac_sbr_ctx_close_fixed(ac->che[type][id]); > - else > - ff_aac_sbr_ctx_close(ac->che[type][id]); > + FIXED_OR_FLOAT(ac->is_fixed, ff_aac_sbr_ctx_close, (ac->che[type][id])); > } > av_freep(&ac->che[type][id]); > } > @@ -1122,8 +1130,7 @@ static av_cold int decode_close(AVCodecContext *avctx) > { > AACDecContext *ac = avctx->priv_data; > int is_fixed = ac->is_fixed; > - void (*sbr_close)(ChannelElement *che) = is_fixed ? ff_aac_sbr_ctx_close_fixed : > - ff_aac_sbr_ctx_close; > + void (*sbr_close)(ChannelElement *che) = FIXED_OR_FLOAT(is_fixed, ff_aac_sbr_ctx_close, ); > > for (int type = 0; type < FF_ARRAY_ELEMS(ac->che); type++) { > for (int i = 0; i < MAX_ELEM_ID; i++) { > @@ -1154,7 +1161,7 @@ static av_cold int decode_close(AVCodecContext *avctx) > static av_cold int init_dsp(AVCodecContext *avctx) > { > AACDecContext *ac = avctx->priv_data; > - int is_fixed = ac->is_fixed, ret; > + int is_fixed = IS_FIXED(ac->is_fixed), ret; > float scale_fixed, scale_float; > const float *const scalep = is_fixed ? &scale_fixed : &scale_float; > enum AVTXType tx_type = is_fixed ? AV_TX_INT32_MDCT : AV_TX_FLOAT_MDCT; > @@ -1188,8 +1195,8 @@ static av_cold int init_dsp(AVCodecContext *avctx) > if (ret < 0) > return ret; > > - ac->dsp = is_fixed ? aac_dsp_fixed : aac_dsp; > - ac->proc = is_fixed ? aac_proc_fixed : aac_proc; > + ac->dsp = FIXED_OR_FLOAT(is_fixed, aac_dsp, ); > + ac->proc = FIXED_OR_FLOAT(is_fixed, aac_proc, ); > > return ac->dsp.init(ac); > } > @@ -1315,9 +1322,9 @@ static void decode_ltp(AACDecContext *ac, LongTermPrediction *ltp, > int sfb; > > ltp->lag = get_bits(gb, 11); > - if (CONFIG_AAC_FIXED_DECODER && ac->is_fixed) > + if (IS_FIXED(ac->is_fixed)) > ltp->coef_fixed = Q30(ff_ltp_coef[get_bits(gb, 3)]); > - else if (CONFIG_AAC_DECODER) > + else > ltp->coef = ff_ltp_coef[get_bits(gb, 3)]; > > for (sfb = 0; sfb < FFMIN(max_sfb, MAX_LTP_LONG_SFB); sfb++) > @@ -1626,9 +1633,9 @@ static int decode_tns(AACDecContext *ac, TemporalNoiseShaping *tns, > tmp2_idx = 2 * coef_compress + coef_res; > > for (i = 0; i < tns->order[w][filt]; i++) { > - if (CONFIG_AAC_FIXED_DECODER && ac->is_fixed) > + if (IS_FIXED(ac->is_fixed)) > tns->coef_fixed[w][filt][i] = Q31(ff_tns_tmp2_map[tmp2_idx][get_bits(gb, coef_len)]); > - else if (CONFIG_AAC_DECODER) > + else > tns->coef[w][filt][i] = ff_tns_tmp2_map[tmp2_idx][get_bits(gb, coef_len)]; > } > } > @@ -1977,11 +1984,8 @@ static int decode_extension_payload(AACDecContext *ac, GetBitContext *gb, int cn > ac->avctx->profile = AV_PROFILE_AAC_HE; > } > > - if (CONFIG_AAC_FIXED_DECODER && ac->is_fixed) > - res = ff_aac_sbr_decode_extension_fixed(ac, che, gb, crc_flag, cnt, elem_type); > - else if (CONFIG_AAC_DECODER) > - res = ff_aac_sbr_decode_extension(ac, che, gb, crc_flag, cnt, elem_type); > - > + res = FIXED_OR_FLOAT(ac->is_fixed, ff_aac_sbr_decode_extension, > + (ac, che, gb, crc_flag, cnt, elem_type)); > > if (ac->oc[1].m4ac.ps == 1 && !ac->warned_he_aac_mono) { > av_log(ac->avctx, AV_LOG_VERBOSE, "Treating HE-AAC mono as stereo.\n"); > @@ -2090,14 +2094,10 @@ static void spectral_to_sample(AACDecContext *ac, int samples) > ac->dsp.update_ltp(ac, &che->ch[1]); > } > if (ac->oc[1].m4ac.sbr > 0) { > - if (CONFIG_AAC_FIXED_DECODER && ac->is_fixed) > - ff_aac_sbr_apply_fixed(ac, che, type, > - (void *)che->ch[0].output, > - (void *)che->ch[1].output); > - else if (CONFIG_AAC_DECODER) > - ff_aac_sbr_apply(ac, che, type, > - (void *)che->ch[0].output, > - (void *)che->ch[1].output); > + FIXED_OR_FLOAT(ac->is_fixed,ff_aac_sbr_apply, > + (ac, che, type, > + (void *)che->ch[0].output, > + (void *)che->ch[1].output)); > I'm not particularly a fan of FIXED_OR_FLOAT, with the way that function arguments are given to the macro. We already rely on DCE globally, and the code only has a few different paths for fixed vs float. Would it be possible to fix the linking errors without adding more abstractions?
Lynne: > May 6, 2024, 11:31 by andreas.rheinhardt@outlook.com: > >> The approach used here has the advantage not to rely >> on any DCE. >> Also improve certain the checks from >> 3390693bfb907765f833766f370e0ba8c7894f44 a bit. >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> >> --- >> libavcodec/aac/aacdec.c | 62 ++++++++++++++++++++--------------------- >> 1 file changed, 31 insertions(+), 31 deletions(-) >> >> diff --git a/libavcodec/aac/aacdec.c b/libavcodec/aac/aacdec.c >> index c6b93e33a2..6a74b05168 100644 >> --- a/libavcodec/aac/aacdec.c >> +++ b/libavcodec/aac/aacdec.c >> @@ -63,6 +63,20 @@ >> #include "libavutil/version.h" >> #include "libavutil/thread.h" >> >> +#if CONFIG_AAC_DECODER && CONFIG_AAC_FIXED_DECODER >> +#define IS_FIXED(is_fixed) (is_fixed) >> +#define FIXED_OR_FLOAT(is_fixed, func_or_obj, func_args) \ >> + ((is_fixed) ? RENAME_FIXED(func_or_obj) func_args : (func_or_obj) func_args) >> +#elif CONFIG_AAC_DECODER >> +#define IS_FIXED(is_fixed) 0 >> +#define FIXED_OR_FLOAT(is_fixed, func_or_obj, func_args) \ >> + ((func_or_obj) func_args) >> +#else >> +#define IS_FIXED(is_fixed) 1 >> +#define FIXED_OR_FLOAT(is_fixed, func_or_obj, func_args) \ >> + (RENAME_FIXED(func_or_obj) func_args) >> +#endif >> + >> /* >> * supported tools >> * >> @@ -150,11 +164,8 @@ static av_cold int che_configure(AACDecContext *ac, >> return AVERROR_INVALIDDATA; >> if (che_pos) { >> if (!ac->che[type][id]) { >> - int ret; >> - if (ac->is_fixed) >> - ret = ff_aac_sbr_ctx_alloc_init_fixed(ac, &ac->che[type][id], type); >> - else >> - ret = ff_aac_sbr_ctx_alloc_init(ac, &ac->che[type][id], type); >> + int ret = FIXED_OR_FLOAT(ac->is_fixed, ff_aac_sbr_ctx_alloc_init, >> + (ac, &ac->che[type][id], type)); >> if (ret < 0) >> return ret; >> } >> @@ -171,10 +182,7 @@ static av_cold int che_configure(AACDecContext *ac, >> } >> } else { >> if (ac->che[type][id]) { >> - if (ac->is_fixed) >> - ff_aac_sbr_ctx_close_fixed(ac->che[type][id]); >> - else >> - ff_aac_sbr_ctx_close(ac->che[type][id]); >> + FIXED_OR_FLOAT(ac->is_fixed, ff_aac_sbr_ctx_close, (ac->che[type][id])); >> } >> av_freep(&ac->che[type][id]); >> } >> @@ -1122,8 +1130,7 @@ static av_cold int decode_close(AVCodecContext *avctx) >> { >> AACDecContext *ac = avctx->priv_data; >> int is_fixed = ac->is_fixed; >> - void (*sbr_close)(ChannelElement *che) = is_fixed ? ff_aac_sbr_ctx_close_fixed : >> - ff_aac_sbr_ctx_close; >> + void (*sbr_close)(ChannelElement *che) = FIXED_OR_FLOAT(is_fixed, ff_aac_sbr_ctx_close, ); >> >> for (int type = 0; type < FF_ARRAY_ELEMS(ac->che); type++) { >> for (int i = 0; i < MAX_ELEM_ID; i++) { >> @@ -1154,7 +1161,7 @@ static av_cold int decode_close(AVCodecContext *avctx) >> static av_cold int init_dsp(AVCodecContext *avctx) >> { >> AACDecContext *ac = avctx->priv_data; >> - int is_fixed = ac->is_fixed, ret; >> + int is_fixed = IS_FIXED(ac->is_fixed), ret; >> float scale_fixed, scale_float; >> const float *const scalep = is_fixed ? &scale_fixed : &scale_float; >> enum AVTXType tx_type = is_fixed ? AV_TX_INT32_MDCT : AV_TX_FLOAT_MDCT; >> @@ -1188,8 +1195,8 @@ static av_cold int init_dsp(AVCodecContext *avctx) >> if (ret < 0) >> return ret; >> >> - ac->dsp = is_fixed ? aac_dsp_fixed : aac_dsp; >> - ac->proc = is_fixed ? aac_proc_fixed : aac_proc; >> + ac->dsp = FIXED_OR_FLOAT(is_fixed, aac_dsp, ); >> + ac->proc = FIXED_OR_FLOAT(is_fixed, aac_proc, ); >> >> return ac->dsp.init(ac); >> } >> @@ -1315,9 +1322,9 @@ static void decode_ltp(AACDecContext *ac, LongTermPrediction *ltp, >> int sfb; >> >> ltp->lag = get_bits(gb, 11); >> - if (CONFIG_AAC_FIXED_DECODER && ac->is_fixed) >> + if (IS_FIXED(ac->is_fixed)) >> ltp->coef_fixed = Q30(ff_ltp_coef[get_bits(gb, 3)]); >> - else if (CONFIG_AAC_DECODER) >> + else >> ltp->coef = ff_ltp_coef[get_bits(gb, 3)]; >> >> for (sfb = 0; sfb < FFMIN(max_sfb, MAX_LTP_LONG_SFB); sfb++) >> @@ -1626,9 +1633,9 @@ static int decode_tns(AACDecContext *ac, TemporalNoiseShaping *tns, >> tmp2_idx = 2 * coef_compress + coef_res; >> >> for (i = 0; i < tns->order[w][filt]; i++) { >> - if (CONFIG_AAC_FIXED_DECODER && ac->is_fixed) >> + if (IS_FIXED(ac->is_fixed)) >> tns->coef_fixed[w][filt][i] = Q31(ff_tns_tmp2_map[tmp2_idx][get_bits(gb, coef_len)]); >> - else if (CONFIG_AAC_DECODER) >> + else >> tns->coef[w][filt][i] = ff_tns_tmp2_map[tmp2_idx][get_bits(gb, coef_len)]; >> } >> } >> @@ -1977,11 +1984,8 @@ static int decode_extension_payload(AACDecContext *ac, GetBitContext *gb, int cn >> ac->avctx->profile = AV_PROFILE_AAC_HE; >> } >> >> - if (CONFIG_AAC_FIXED_DECODER && ac->is_fixed) >> - res = ff_aac_sbr_decode_extension_fixed(ac, che, gb, crc_flag, cnt, elem_type); >> - else if (CONFIG_AAC_DECODER) >> - res = ff_aac_sbr_decode_extension(ac, che, gb, crc_flag, cnt, elem_type); >> - >> + res = FIXED_OR_FLOAT(ac->is_fixed, ff_aac_sbr_decode_extension, >> + (ac, che, gb, crc_flag, cnt, elem_type)); >> >> if (ac->oc[1].m4ac.ps == 1 && !ac->warned_he_aac_mono) { >> av_log(ac->avctx, AV_LOG_VERBOSE, "Treating HE-AAC mono as stereo.\n"); >> @@ -2090,14 +2094,10 @@ static void spectral_to_sample(AACDecContext *ac, int samples) >> ac->dsp.update_ltp(ac, &che->ch[1]); >> } >> if (ac->oc[1].m4ac.sbr > 0) { >> - if (CONFIG_AAC_FIXED_DECODER && ac->is_fixed) >> - ff_aac_sbr_apply_fixed(ac, che, type, >> - (void *)che->ch[0].output, >> - (void *)che->ch[1].output); >> - else if (CONFIG_AAC_DECODER) >> - ff_aac_sbr_apply(ac, che, type, >> - (void *)che->ch[0].output, >> - (void *)che->ch[1].output); >> + FIXED_OR_FLOAT(ac->is_fixed,ff_aac_sbr_apply, >> + (ac, che, type, >> + (void *)che->ch[0].output, >> + (void *)che->ch[1].output)); >> > > I'm not particularly a fan of FIXED_OR_FLOAT, with the > way that function arguments are given to the macro. > We already rely on DCE globally, and the code only > has a few different paths for fixed vs float. > Would it be possible to fix the linking errors without > adding more abstractions? If you don't like the way function arguments are given to the macro, then you can take a look at v2. It avoids this (except for the one special case where the current code uses casts...). And we should actually not rely on DCE. It is not mandated by ISO C. - Andreas
May 6, 2024, 21:39 by andreas.rheinhardt@outlook.com: > Lynne: > >> May 6, 2024, 11:31 by andreas.rheinhardt@outlook.com: >> >>> The approach used here has the advantage not to rely >>> on any DCE. >>> Also improve certain the checks from >>> 3390693bfb907765f833766f370e0ba8c7894f44 a bit. >>> >>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> >>> --- >>> libavcodec/aac/aacdec.c | 62 ++++++++++++++++++++--------------------- >>> 1 file changed, 31 insertions(+), 31 deletions(-) >>> >>> diff --git a/libavcodec/aac/aacdec.c b/libavcodec/aac/aacdec.c >>> index c6b93e33a2..6a74b05168 100644 >>> --- a/libavcodec/aac/aacdec.c >>> +++ b/libavcodec/aac/aacdec.c >>> @@ -63,6 +63,20 @@ >>> #include "libavutil/version.h" >>> #include "libavutil/thread.h" >>> >>> +#if CONFIG_AAC_DECODER && CONFIG_AAC_FIXED_DECODER >>> +#define IS_FIXED(is_fixed) (is_fixed) >>> +#define FIXED_OR_FLOAT(is_fixed, func_or_obj, func_args) \ >>> + ((is_fixed) ? RENAME_FIXED(func_or_obj) func_args : (func_or_obj) func_args) >>> +#elif CONFIG_AAC_DECODER >>> +#define IS_FIXED(is_fixed) 0 >>> +#define FIXED_OR_FLOAT(is_fixed, func_or_obj, func_args) \ >>> + ((func_or_obj) func_args) >>> +#else >>> +#define IS_FIXED(is_fixed) 1 >>> +#define FIXED_OR_FLOAT(is_fixed, func_or_obj, func_args) \ >>> + (RENAME_FIXED(func_or_obj) func_args) >>> +#endif >>> + >>> /* >>> * supported tools >>> * >>> @@ -150,11 +164,8 @@ static av_cold int che_configure(AACDecContext *ac, >>> return AVERROR_INVALIDDATA; >>> if (che_pos) { >>> if (!ac->che[type][id]) { >>> - int ret; >>> - if (ac->is_fixed) >>> - ret = ff_aac_sbr_ctx_alloc_init_fixed(ac, &ac->che[type][id], type); >>> - else >>> - ret = ff_aac_sbr_ctx_alloc_init(ac, &ac->che[type][id], type); >>> + int ret = FIXED_OR_FLOAT(ac->is_fixed, ff_aac_sbr_ctx_alloc_init, >>> + (ac, &ac->che[type][id], type)); >>> if (ret < 0) >>> return ret; >>> } >>> @@ -171,10 +182,7 @@ static av_cold int che_configure(AACDecContext *ac, >>> } >>> } else { >>> if (ac->che[type][id]) { >>> - if (ac->is_fixed) >>> - ff_aac_sbr_ctx_close_fixed(ac->che[type][id]); >>> - else >>> - ff_aac_sbr_ctx_close(ac->che[type][id]); >>> + FIXED_OR_FLOAT(ac->is_fixed, ff_aac_sbr_ctx_close, (ac->che[type][id])); >>> } >>> av_freep(&ac->che[type][id]); >>> } >>> @@ -1122,8 +1130,7 @@ static av_cold int decode_close(AVCodecContext *avctx) >>> { >>> AACDecContext *ac = avctx->priv_data; >>> int is_fixed = ac->is_fixed; >>> - void (*sbr_close)(ChannelElement *che) = is_fixed ? ff_aac_sbr_ctx_close_fixed : >>> - ff_aac_sbr_ctx_close; >>> + void (*sbr_close)(ChannelElement *che) = FIXED_OR_FLOAT(is_fixed, ff_aac_sbr_ctx_close, ); >>> >>> for (int type = 0; type < FF_ARRAY_ELEMS(ac->che); type++) { >>> for (int i = 0; i < MAX_ELEM_ID; i++) { >>> @@ -1154,7 +1161,7 @@ static av_cold int decode_close(AVCodecContext *avctx) >>> static av_cold int init_dsp(AVCodecContext *avctx) >>> { >>> AACDecContext *ac = avctx->priv_data; >>> - int is_fixed = ac->is_fixed, ret; >>> + int is_fixed = IS_FIXED(ac->is_fixed), ret; >>> float scale_fixed, scale_float; >>> const float *const scalep = is_fixed ? &scale_fixed : &scale_float; >>> enum AVTXType tx_type = is_fixed ? AV_TX_INT32_MDCT : AV_TX_FLOAT_MDCT; >>> @@ -1188,8 +1195,8 @@ static av_cold int init_dsp(AVCodecContext *avctx) >>> if (ret < 0) >>> return ret; >>> >>> - ac->dsp = is_fixed ? aac_dsp_fixed : aac_dsp; >>> - ac->proc = is_fixed ? aac_proc_fixed : aac_proc; >>> + ac->dsp = FIXED_OR_FLOAT(is_fixed, aac_dsp, ); >>> + ac->proc = FIXED_OR_FLOAT(is_fixed, aac_proc, ); >>> >>> return ac->dsp.init(ac); >>> } >>> @@ -1315,9 +1322,9 @@ static void decode_ltp(AACDecContext *ac, LongTermPrediction *ltp, >>> int sfb; >>> >>> ltp->lag = get_bits(gb, 11); >>> - if (CONFIG_AAC_FIXED_DECODER && ac->is_fixed) >>> + if (IS_FIXED(ac->is_fixed)) >>> ltp->coef_fixed = Q30(ff_ltp_coef[get_bits(gb, 3)]); >>> - else if (CONFIG_AAC_DECODER) >>> + else >>> ltp->coef = ff_ltp_coef[get_bits(gb, 3)]; >>> >>> for (sfb = 0; sfb < FFMIN(max_sfb, MAX_LTP_LONG_SFB); sfb++) >>> @@ -1626,9 +1633,9 @@ static int decode_tns(AACDecContext *ac, TemporalNoiseShaping *tns, >>> tmp2_idx = 2 * coef_compress + coef_res; >>> >>> for (i = 0; i < tns->order[w][filt]; i++) { >>> - if (CONFIG_AAC_FIXED_DECODER && ac->is_fixed) >>> + if (IS_FIXED(ac->is_fixed)) >>> tns->coef_fixed[w][filt][i] = Q31(ff_tns_tmp2_map[tmp2_idx][get_bits(gb, coef_len)]); >>> - else if (CONFIG_AAC_DECODER) >>> + else >>> tns->coef[w][filt][i] = ff_tns_tmp2_map[tmp2_idx][get_bits(gb, coef_len)]; >>> } >>> } >>> @@ -1977,11 +1984,8 @@ static int decode_extension_payload(AACDecContext *ac, GetBitContext *gb, int cn >>> ac->avctx->profile = AV_PROFILE_AAC_HE; >>> } >>> >>> - if (CONFIG_AAC_FIXED_DECODER && ac->is_fixed) >>> - res = ff_aac_sbr_decode_extension_fixed(ac, che, gb, crc_flag, cnt, elem_type); >>> - else if (CONFIG_AAC_DECODER) >>> - res = ff_aac_sbr_decode_extension(ac, che, gb, crc_flag, cnt, elem_type); >>> - >>> + res = FIXED_OR_FLOAT(ac->is_fixed, ff_aac_sbr_decode_extension, >>> + (ac, che, gb, crc_flag, cnt, elem_type)); >>> >>> if (ac->oc[1].m4ac.ps == 1 && !ac->warned_he_aac_mono) { >>> av_log(ac->avctx, AV_LOG_VERBOSE, "Treating HE-AAC mono as stereo.\n"); >>> @@ -2090,14 +2094,10 @@ static void spectral_to_sample(AACDecContext *ac, int samples) >>> ac->dsp.update_ltp(ac, &che->ch[1]); >>> } >>> if (ac->oc[1].m4ac.sbr > 0) { >>> - if (CONFIG_AAC_FIXED_DECODER && ac->is_fixed) >>> - ff_aac_sbr_apply_fixed(ac, che, type, >>> - (void *)che->ch[0].output, >>> - (void *)che->ch[1].output); >>> - else if (CONFIG_AAC_DECODER) >>> - ff_aac_sbr_apply(ac, che, type, >>> - (void *)che->ch[0].output, >>> - (void *)che->ch[1].output); >>> + FIXED_OR_FLOAT(ac->is_fixed,ff_aac_sbr_apply, >>> + (ac, che, type, >>> + (void *)che->ch[0].output, >>> + (void *)che->ch[1].output)); >>> >> >> I'm not particularly a fan of FIXED_OR_FLOAT, with the >> way that function arguments are given to the macro. >> We already rely on DCE globally, and the code only >> has a few different paths for fixed vs float. >> Would it be possible to fix the linking errors without >> adding more abstractions? >> > > If you don't like the way function arguments are given to the macro, > then you can take a look at v2. It avoids this (except for the one > special case where the current code uses casts...). > I'm still not a fan of adding as many lines of macros to deal with it when that would be basically the same amount of lines as ifdefs. > And we should actually not rely on DCE. It is not mandated by ISO C. > As long as there's no effort done to reduce it, I don't think it will be a problem to introduce more of it. Its everywhere so much that adding more doesn't increase the work significantly.
Lynne: > May 6, 2024, 21:39 by andreas.rheinhardt@outlook.com: > >> Lynne: >> >>> May 6, 2024, 11:31 by andreas.rheinhardt@outlook.com: >>> >>>> The approach used here has the advantage not to rely >>>> on any DCE. >>>> Also improve certain the checks from >>>> 3390693bfb907765f833766f370e0ba8c7894f44 a bit. >>>> >>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> >>>> --- >>>> libavcodec/aac/aacdec.c | 62 ++++++++++++++++++++--------------------- >>>> 1 file changed, 31 insertions(+), 31 deletions(-) >>>> >>>> diff --git a/libavcodec/aac/aacdec.c b/libavcodec/aac/aacdec.c >>>> index c6b93e33a2..6a74b05168 100644 >>>> --- a/libavcodec/aac/aacdec.c >>>> +++ b/libavcodec/aac/aacdec.c >>>> @@ -63,6 +63,20 @@ >>>> #include "libavutil/version.h" >>>> #include "libavutil/thread.h" >>>> >>>> +#if CONFIG_AAC_DECODER && CONFIG_AAC_FIXED_DECODER >>>> +#define IS_FIXED(is_fixed) (is_fixed) >>>> +#define FIXED_OR_FLOAT(is_fixed, func_or_obj, func_args) \ >>>> + ((is_fixed) ? RENAME_FIXED(func_or_obj) func_args : (func_or_obj) func_args) >>>> +#elif CONFIG_AAC_DECODER >>>> +#define IS_FIXED(is_fixed) 0 >>>> +#define FIXED_OR_FLOAT(is_fixed, func_or_obj, func_args) \ >>>> + ((func_or_obj) func_args) >>>> +#else >>>> +#define IS_FIXED(is_fixed) 1 >>>> +#define FIXED_OR_FLOAT(is_fixed, func_or_obj, func_args) \ >>>> + (RENAME_FIXED(func_or_obj) func_args) >>>> +#endif >>>> + >>>> /* >>>> * supported tools >>>> * >>>> @@ -150,11 +164,8 @@ static av_cold int che_configure(AACDecContext *ac, >>>> return AVERROR_INVALIDDATA; >>>> if (che_pos) { >>>> if (!ac->che[type][id]) { >>>> - int ret; >>>> - if (ac->is_fixed) >>>> - ret = ff_aac_sbr_ctx_alloc_init_fixed(ac, &ac->che[type][id], type); >>>> - else >>>> - ret = ff_aac_sbr_ctx_alloc_init(ac, &ac->che[type][id], type); >>>> + int ret = FIXED_OR_FLOAT(ac->is_fixed, ff_aac_sbr_ctx_alloc_init, >>>> + (ac, &ac->che[type][id], type)); >>>> if (ret < 0) >>>> return ret; >>>> } >>>> @@ -171,10 +182,7 @@ static av_cold int che_configure(AACDecContext *ac, >>>> } >>>> } else { >>>> if (ac->che[type][id]) { >>>> - if (ac->is_fixed) >>>> - ff_aac_sbr_ctx_close_fixed(ac->che[type][id]); >>>> - else >>>> - ff_aac_sbr_ctx_close(ac->che[type][id]); >>>> + FIXED_OR_FLOAT(ac->is_fixed, ff_aac_sbr_ctx_close, (ac->che[type][id])); >>>> } >>>> av_freep(&ac->che[type][id]); >>>> } >>>> @@ -1122,8 +1130,7 @@ static av_cold int decode_close(AVCodecContext *avctx) >>>> { >>>> AACDecContext *ac = avctx->priv_data; >>>> int is_fixed = ac->is_fixed; >>>> - void (*sbr_close)(ChannelElement *che) = is_fixed ? ff_aac_sbr_ctx_close_fixed : >>>> - ff_aac_sbr_ctx_close; >>>> + void (*sbr_close)(ChannelElement *che) = FIXED_OR_FLOAT(is_fixed, ff_aac_sbr_ctx_close, ); >>>> >>>> for (int type = 0; type < FF_ARRAY_ELEMS(ac->che); type++) { >>>> for (int i = 0; i < MAX_ELEM_ID; i++) { >>>> @@ -1154,7 +1161,7 @@ static av_cold int decode_close(AVCodecContext *avctx) >>>> static av_cold int init_dsp(AVCodecContext *avctx) >>>> { >>>> AACDecContext *ac = avctx->priv_data; >>>> - int is_fixed = ac->is_fixed, ret; >>>> + int is_fixed = IS_FIXED(ac->is_fixed), ret; >>>> float scale_fixed, scale_float; >>>> const float *const scalep = is_fixed ? &scale_fixed : &scale_float; >>>> enum AVTXType tx_type = is_fixed ? AV_TX_INT32_MDCT : AV_TX_FLOAT_MDCT; >>>> @@ -1188,8 +1195,8 @@ static av_cold int init_dsp(AVCodecContext *avctx) >>>> if (ret < 0) >>>> return ret; >>>> >>>> - ac->dsp = is_fixed ? aac_dsp_fixed : aac_dsp; >>>> - ac->proc = is_fixed ? aac_proc_fixed : aac_proc; >>>> + ac->dsp = FIXED_OR_FLOAT(is_fixed, aac_dsp, ); >>>> + ac->proc = FIXED_OR_FLOAT(is_fixed, aac_proc, ); >>>> >>>> return ac->dsp.init(ac); >>>> } >>>> @@ -1315,9 +1322,9 @@ static void decode_ltp(AACDecContext *ac, LongTermPrediction *ltp, >>>> int sfb; >>>> >>>> ltp->lag = get_bits(gb, 11); >>>> - if (CONFIG_AAC_FIXED_DECODER && ac->is_fixed) >>>> + if (IS_FIXED(ac->is_fixed)) >>>> ltp->coef_fixed = Q30(ff_ltp_coef[get_bits(gb, 3)]); >>>> - else if (CONFIG_AAC_DECODER) >>>> + else >>>> ltp->coef = ff_ltp_coef[get_bits(gb, 3)]; >>>> >>>> for (sfb = 0; sfb < FFMIN(max_sfb, MAX_LTP_LONG_SFB); sfb++) >>>> @@ -1626,9 +1633,9 @@ static int decode_tns(AACDecContext *ac, TemporalNoiseShaping *tns, >>>> tmp2_idx = 2 * coef_compress + coef_res; >>>> >>>> for (i = 0; i < tns->order[w][filt]; i++) { >>>> - if (CONFIG_AAC_FIXED_DECODER && ac->is_fixed) >>>> + if (IS_FIXED(ac->is_fixed)) >>>> tns->coef_fixed[w][filt][i] = Q31(ff_tns_tmp2_map[tmp2_idx][get_bits(gb, coef_len)]); >>>> - else if (CONFIG_AAC_DECODER) >>>> + else >>>> tns->coef[w][filt][i] = ff_tns_tmp2_map[tmp2_idx][get_bits(gb, coef_len)]; >>>> } >>>> } >>>> @@ -1977,11 +1984,8 @@ static int decode_extension_payload(AACDecContext *ac, GetBitContext *gb, int cn >>>> ac->avctx->profile = AV_PROFILE_AAC_HE; >>>> } >>>> >>>> - if (CONFIG_AAC_FIXED_DECODER && ac->is_fixed) >>>> - res = ff_aac_sbr_decode_extension_fixed(ac, che, gb, crc_flag, cnt, elem_type); >>>> - else if (CONFIG_AAC_DECODER) >>>> - res = ff_aac_sbr_decode_extension(ac, che, gb, crc_flag, cnt, elem_type); >>>> - >>>> + res = FIXED_OR_FLOAT(ac->is_fixed, ff_aac_sbr_decode_extension, >>>> + (ac, che, gb, crc_flag, cnt, elem_type)); >>>> >>>> if (ac->oc[1].m4ac.ps == 1 && !ac->warned_he_aac_mono) { >>>> av_log(ac->avctx, AV_LOG_VERBOSE, "Treating HE-AAC mono as stereo.\n"); >>>> @@ -2090,14 +2094,10 @@ static void spectral_to_sample(AACDecContext *ac, int samples) >>>> ac->dsp.update_ltp(ac, &che->ch[1]); >>>> } >>>> if (ac->oc[1].m4ac.sbr > 0) { >>>> - if (CONFIG_AAC_FIXED_DECODER && ac->is_fixed) >>>> - ff_aac_sbr_apply_fixed(ac, che, type, >>>> - (void *)che->ch[0].output, >>>> - (void *)che->ch[1].output); >>>> - else if (CONFIG_AAC_DECODER) >>>> - ff_aac_sbr_apply(ac, che, type, >>>> - (void *)che->ch[0].output, >>>> - (void *)che->ch[1].output); >>>> + FIXED_OR_FLOAT(ac->is_fixed,ff_aac_sbr_apply, >>>> + (ac, che, type, >>>> + (void *)che->ch[0].output, >>>> + (void *)che->ch[1].output)); >>>> >>> >>> I'm not particularly a fan of FIXED_OR_FLOAT, with the >>> way that function arguments are given to the macro. >>> We already rely on DCE globally, and the code only >>> has a few different paths for fixed vs float. >>> Would it be possible to fix the linking errors without >>> adding more abstractions? >>> >> >> If you don't like the way function arguments are given to the macro, >> then you can take a look at v2. It avoids this (except for the one >> special case where the current code uses casts...). >> > > I'm still not a fan of adding as many lines of macros to deal with it > when that would be basically the same amount of lines as ifdefs. > The same amount of lines as ifdefs? V1 adds as many lines as it removes, v2 adds one line. Fixing this via ifdefs (or even ordinary CONFIG checks with dce) will certainly amount to more. > >> And we should actually not rely on DCE. It is not mandated by ISO C. >> > > As long as there's no effort done to reduce it, I don't think > it will be a problem to introduce more of it. Its everywhere so > much that adding more doesn't increase the work significantly.
diff --git a/libavcodec/aac/aacdec.c b/libavcodec/aac/aacdec.c index c6b93e33a2..6a74b05168 100644 --- a/libavcodec/aac/aacdec.c +++ b/libavcodec/aac/aacdec.c @@ -63,6 +63,20 @@ #include "libavutil/version.h" #include "libavutil/thread.h" +#if CONFIG_AAC_DECODER && CONFIG_AAC_FIXED_DECODER +#define IS_FIXED(is_fixed) (is_fixed) +#define FIXED_OR_FLOAT(is_fixed, func_or_obj, func_args) \ + ((is_fixed) ? RENAME_FIXED(func_or_obj) func_args : (func_or_obj) func_args) +#elif CONFIG_AAC_DECODER +#define IS_FIXED(is_fixed) 0 +#define FIXED_OR_FLOAT(is_fixed, func_or_obj, func_args) \ + ((func_or_obj) func_args) +#else +#define IS_FIXED(is_fixed) 1 +#define FIXED_OR_FLOAT(is_fixed, func_or_obj, func_args) \ + (RENAME_FIXED(func_or_obj) func_args) +#endif + /* * supported tools * @@ -150,11 +164,8 @@ static av_cold int che_configure(AACDecContext *ac, return AVERROR_INVALIDDATA; if (che_pos) { if (!ac->che[type][id]) { - int ret; - if (ac->is_fixed) - ret = ff_aac_sbr_ctx_alloc_init_fixed(ac, &ac->che[type][id], type); - else - ret = ff_aac_sbr_ctx_alloc_init(ac, &ac->che[type][id], type); + int ret = FIXED_OR_FLOAT(ac->is_fixed, ff_aac_sbr_ctx_alloc_init, + (ac, &ac->che[type][id], type)); if (ret < 0) return ret; } @@ -171,10 +182,7 @@ static av_cold int che_configure(AACDecContext *ac, } } else { if (ac->che[type][id]) { - if (ac->is_fixed) - ff_aac_sbr_ctx_close_fixed(ac->che[type][id]); - else - ff_aac_sbr_ctx_close(ac->che[type][id]); + FIXED_OR_FLOAT(ac->is_fixed, ff_aac_sbr_ctx_close, (ac->che[type][id])); } av_freep(&ac->che[type][id]); } @@ -1122,8 +1130,7 @@ static av_cold int decode_close(AVCodecContext *avctx) { AACDecContext *ac = avctx->priv_data; int is_fixed = ac->is_fixed; - void (*sbr_close)(ChannelElement *che) = is_fixed ? ff_aac_sbr_ctx_close_fixed : - ff_aac_sbr_ctx_close; + void (*sbr_close)(ChannelElement *che) = FIXED_OR_FLOAT(is_fixed, ff_aac_sbr_ctx_close, ); for (int type = 0; type < FF_ARRAY_ELEMS(ac->che); type++) { for (int i = 0; i < MAX_ELEM_ID; i++) { @@ -1154,7 +1161,7 @@ static av_cold int decode_close(AVCodecContext *avctx) static av_cold int init_dsp(AVCodecContext *avctx) { AACDecContext *ac = avctx->priv_data; - int is_fixed = ac->is_fixed, ret; + int is_fixed = IS_FIXED(ac->is_fixed), ret; float scale_fixed, scale_float; const float *const scalep = is_fixed ? &scale_fixed : &scale_float; enum AVTXType tx_type = is_fixed ? AV_TX_INT32_MDCT : AV_TX_FLOAT_MDCT; @@ -1188,8 +1195,8 @@ static av_cold int init_dsp(AVCodecContext *avctx) if (ret < 0) return ret; - ac->dsp = is_fixed ? aac_dsp_fixed : aac_dsp; - ac->proc = is_fixed ? aac_proc_fixed : aac_proc; + ac->dsp = FIXED_OR_FLOAT(is_fixed, aac_dsp, ); + ac->proc = FIXED_OR_FLOAT(is_fixed, aac_proc, ); return ac->dsp.init(ac); } @@ -1315,9 +1322,9 @@ static void decode_ltp(AACDecContext *ac, LongTermPrediction *ltp, int sfb; ltp->lag = get_bits(gb, 11); - if (CONFIG_AAC_FIXED_DECODER && ac->is_fixed) + if (IS_FIXED(ac->is_fixed)) ltp->coef_fixed = Q30(ff_ltp_coef[get_bits(gb, 3)]); - else if (CONFIG_AAC_DECODER) + else ltp->coef = ff_ltp_coef[get_bits(gb, 3)]; for (sfb = 0; sfb < FFMIN(max_sfb, MAX_LTP_LONG_SFB); sfb++) @@ -1626,9 +1633,9 @@ static int decode_tns(AACDecContext *ac, TemporalNoiseShaping *tns, tmp2_idx = 2 * coef_compress + coef_res; for (i = 0; i < tns->order[w][filt]; i++) { - if (CONFIG_AAC_FIXED_DECODER && ac->is_fixed) + if (IS_FIXED(ac->is_fixed)) tns->coef_fixed[w][filt][i] = Q31(ff_tns_tmp2_map[tmp2_idx][get_bits(gb, coef_len)]); - else if (CONFIG_AAC_DECODER) + else tns->coef[w][filt][i] = ff_tns_tmp2_map[tmp2_idx][get_bits(gb, coef_len)]; } } @@ -1977,11 +1984,8 @@ static int decode_extension_payload(AACDecContext *ac, GetBitContext *gb, int cn ac->avctx->profile = AV_PROFILE_AAC_HE; } - if (CONFIG_AAC_FIXED_DECODER && ac->is_fixed) - res = ff_aac_sbr_decode_extension_fixed(ac, che, gb, crc_flag, cnt, elem_type); - else if (CONFIG_AAC_DECODER) - res = ff_aac_sbr_decode_extension(ac, che, gb, crc_flag, cnt, elem_type); - + res = FIXED_OR_FLOAT(ac->is_fixed, ff_aac_sbr_decode_extension, + (ac, che, gb, crc_flag, cnt, elem_type)); if (ac->oc[1].m4ac.ps == 1 && !ac->warned_he_aac_mono) { av_log(ac->avctx, AV_LOG_VERBOSE, "Treating HE-AAC mono as stereo.\n"); @@ -2090,14 +2094,10 @@ static void spectral_to_sample(AACDecContext *ac, int samples) ac->dsp.update_ltp(ac, &che->ch[1]); } if (ac->oc[1].m4ac.sbr > 0) { - if (CONFIG_AAC_FIXED_DECODER && ac->is_fixed) - ff_aac_sbr_apply_fixed(ac, che, type, - (void *)che->ch[0].output, - (void *)che->ch[1].output); - else if (CONFIG_AAC_DECODER) - ff_aac_sbr_apply(ac, che, type, - (void *)che->ch[0].output, - (void *)che->ch[1].output); + FIXED_OR_FLOAT(ac->is_fixed,ff_aac_sbr_apply, + (ac, che, type, + (void *)che->ch[0].output, + (void *)che->ch[1].output)); } } if (type <= TYPE_CCE)
The approach used here has the advantage not to rely on any DCE. Also improve certain the checks from 3390693bfb907765f833766f370e0ba8c7894f44 a bit. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavcodec/aac/aacdec.c | 62 ++++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 31 deletions(-)