[FFmpeg-devel] [PATCH] aactab: add and initialize 2D VLC tables for USAC Mps212

[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.

> 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xA2FEA5F03F034464.asc
Type: application/pgp-keys
Size: 624 bytes
Desc: OpenPGP public key
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20250505/5ff45842/attachment.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 236 bytes
Desc: OpenPGP digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20250505/5ff45842/attachment.sig>