Lynne:
> On 05/05/2025 15:52, Andreas Rheinhardt wrote:
>> Lynne:
>>> ---
>>> libavcodec/aac/aacdec_tab.c | 54 ++
>>> libavcodec/aactab.c | 1820 +++++++++++++++++++++++++++++++++++
>>> libavcodec/aactab.h | 20 +
>>> 3 files changed, 1894 insertions(+)
>>>
>>
>> 1. This should only be applied if it is used which this patch does not.
>
> Just posting this for reviews.
>>> diff --git a/libavcodec/aac/aacdec_tab.c b/libavcodec/aac/aacdec_tab.c
>>> index 45a84a9a72..5ba20c0d8a 100644
>>> --- a/libavcodec/aac/aacdec_tab.c
>>> +++ b/libavcodec/aac/aacdec_tab.c
>>> @@ -111,6 +111,16 @@ const AVChannelLayout ff_aac_ch_layout[] = {
>>> VLCElem ff_vlc_scalefactors[352];
>>> const VLCElem *ff_vlc_spectral[11];
>>> +const VLCElem *ff_vlc_cld_lav3_2D[2][2];
>>> +const VLCElem *ff_vlc_cld_lav5_2D[2][2];
>>> +const VLCElem *ff_vlc_cld_lav7_2D[2][2];
>>> +const VLCElem *ff_vlc_cld_lav9_2D[2][2];
>>> +
>>> +const VLCElem *ff_vlc_icc_lav1_2D[2][2];
>>> +const VLCElem *ff_vlc_icc_lav3_2D[2][2];
>>> +const VLCElem *ff_vlc_icc_lav5_2D[2][2];
>>> +const VLCElem *ff_vlc_icc_lav7_2D[2][2];
>>> +
>>> /// Huffman tables for SBR
>>> static const uint8_t sbr_huffman_tab[][2] = {
>>> @@ -279,6 +289,50 @@ static av_cold void aacdec_common_init(void)
>>> 0);
>>> }
>>> +#define LAV_N_PAIR(NAME, NB) \
>>> + ff_vlc_ ## NAME[0][0] = ff_vlc_init_tables(&state, NB, NB, \
>>> + ff_aac_ ## NAME ##
>>> _0_0_bits, \
>>> + sizeof(ff_aac_ ##
>>> NAME ## _0_0_bits), \
>>> + sizeof(ff_aac_ ##
>>> NAME ## _0_0_bits), \
>>> + ff_aac_ ## NAME ##
>>> _0_0_codes, \
>>> + sizeof(ff_aac_ ##
>>> NAME ## _0_0_codes), \
>>> + sizeof(ff_aac_ ##
>>> NAME ## _0_0_codes), \
>>> + 0); \
>>> + ff_vlc_ ## NAME[0][1] = ff_vlc_init_tables(&state, NB, NB, \
>>> + ff_aac_ ## NAME ##
>>> _0_1_bits, \
>>> + sizeof(ff_aac_ ##
>>> NAME ## _0_1_bits), \
>>> + sizeof(ff_aac_ ##
>>> NAME ## _0_1_bits), \
>>> + ff_aac_ ## NAME ##
>>> _0_1_codes, \
>>> + sizeof(ff_aac_ ##
>>> NAME ## _0_1_codes), \
>>> + sizeof(ff_aac_ ##
>>> NAME ## _0_1_codes), \
>>> + 0); \
>>> + ff_vlc_ ## NAME[1][0] = ff_vlc_init_tables(&state, NB, NB, \
>>> + ff_aac_ ## NAME ##
>>> _1_0_bits, \
>>> + sizeof(ff_aac_ ##
>>> NAME ## _1_0_bits), \
>>> + sizeof(ff_aac_ ##
>>> NAME ## _1_0_bits), \
>>> + ff_aac_ ## NAME ##
>>> _1_0_codes, \
>>> + sizeof(ff_aac_ ##
>>> NAME ## _1_0_codes), \
>>> + sizeof(ff_aac_ ##
>>> NAME ## _1_0_codes), \
>>> + 0); \
>>> + ff_vlc_ ## NAME[1][1] = ff_vlc_init_tables(&state, NB, NB, \
>>> + ff_aac_ ## NAME ##
>>> _1_1_bits, \
>>> + sizeof(ff_aac_ ##
>>> NAME ## _1_1_bits), \
>>> + sizeof(ff_aac_ ##
>>> NAME ## _1_1_bits), \
>>> + ff_aac_ ## NAME ##
>>> _1_1_codes, \
>>> + sizeof(ff_aac_ ##
>>> NAME ## _1_1_codes), \
>>> + sizeof(ff_aac_ ##
>>> NAME ## _1_1_codes), \
>>> + 0);
>>
>> 2. I am very much surprised that it works at all (does it?). You use the
>> same state and therefore the same static storage as before; you add new
>> VLCs, yet you do not increase the size of vlc_buf.
>
> The tables are all rather small.
When I decrease the size of vlc_buf by only one element, initialization
fails with an abort, as expected. And so it should be for you, because
there is just no space in vlc_buf left.
>
>> 3. Can't you initialize this in a loop like ff_aac_sbr_vlc? This would
>> remove code duplication as well as relocations.
>
> I didn't know that was possible. vlc.h is poorly documented, with many
> overlapping, wrapped, renamed and macro'd functions that code from
> different decade have gotten adapted to.
Why should this not be possible? You have an example in this very
function. And what exactly is poorly documented?
- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
