On 05/05/2025 16:57, Andreas Rheinhardt wrote:
I start with the tables, not the function that initializes them. All I knew is that VLCs are required to be in separate codes+bits tables. There are multiple variants of VLC init, so I have no idea whether one variant can be adapted to another.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?
Again, I'm just posting this for reviews.
OpenPGP_0xA2FEA5F03F034464.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
_______________________________________________ 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".
