Paul B Mahol: > Than how one is supposed to guess correct size to put when this log is gone? > The developer who wants to make VLCs static just has to use a huge array (in order not to run into the abort) and get the value of the final offset parameter (either via av_log or via a debugger); or you just sum the VLC.table_size values before you make the VLCs static. It is no big deal and it is not much different from what it is now.
> On Tue, Nov 10, 2020 at 1:15 PM Andreas Rheinhardt < > andreas.rheinha...@gmail.com> wrote: > >> Paul B Mahol: >>> On Tue, Nov 10, 2020 at 12:31 PM Andreas Rheinhardt < >>> andreas.rheinha...@gmail.com> wrote: >>> >>>> Paul B Mahol: >>>>> Because it is a bad idea. >>>> >>>> And I still like to hear a reason for this. >>>> >>>> >>> Code is there so correct intended size is always allocated. >>> >> >> No. There are two ways to create a VLC: With the INIT_VLC_USE_NEW_STATIC >> flag set and without it. >> If it is unset, ff_init_vlc_sparse() (and ff_init_vlc_from_lengths(), >> too) allocates the needed space for the VLC table (actually, it >> overallocates) and the VLC must in turn be freed via ff_free_vlc(). In >> this case the whole given VLC is treated as uninitialized, i.e. >> vlc->table_allocated is ignored. >> >> If it is set, then vlc->table must point to an array VLC_TYPE[][2] with >> at least vlc->table_allocated elements and vlc->table_allocated must be >> sufficient for the whole VLC: This buffer will never be (re)allocated; >> it actually points to static storage and not to anything that is >> heap-allocated. Given the error message that this commit intends to >> remove there is currently a requirement for table_allocated to be >> exactly equal to the actually required size. And this requires tables of >> the sizes if VLCs are initialized in a loop (alternatively one can of >> course unroll the loop and hardcode the values via one of the INIT_VLC >> macros). >> >>> >>>>> No need to change code that worked for ages. >>>>> >>>> >>>> It allows to remove useless tables and it simplifies making more VLCs >>>> that should be static static. >>>> >>> >>> What is static static? How does it allows to remove useless tables? >>> >> >> And this requirement is just a pointless straitjacket. If one drops it, >> one can remove the useless tables by using the pattern mentioned in the >> commit message. Several commits of this series provide examples for >> this; e.g. #12 >> (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272139.html), >> #40 >> (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272167.html), >> #53 >> (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272179.html), >> #57 >> (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272183.html) or >> #88 (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272215.html). >> >> These are only examples where existing offsets tables were removed; >> making VLCs static like in #68 >> (https://ffmpeg.org/pipermail/ffmpeg-devel/2020-November/272195.html; >> making these VLCs static is especially beneficial for a multithreaded >> decoder like this one so that the VLCs can be shared between all >> threads) would be more complicated if the requirement of always using >> exactly the required size were not dropped. >> >>> >>>> >>>>> On Tue, Nov 10, 2020 at 12:26 PM Andreas Rheinhardt < >>>>> andreas.rheinha...@gmail.com> wrote: >>>>> >>>>>> Paul B Mahol: >>>>>>> I do not think this is a good direction. >>>>>>> >>>>>> >>>>>> Why? >>>>>> >>>>>>> On Tue, Nov 10, 2020 at 11:50 AM Andreas Rheinhardt < >>>>>>> andreas.rheinha...@gmail.com> wrote: >>>>>>> >>>>>>>> Right now the allocated size of the VLC table of a static VLC has to >>>>>>>> exactly match the size actually used for the VLC: If it is not >> enough, >>>>>>>> abort is called; if it is more than enough, an error message is >>>>>>>> emitted. This is no problem when one wants to initialize an >> individual >>>>>>>> VLC via one of the INIT_VLC macros as one just hardcodes the needed >>>>>>>> size. Yet it is an obstacle when one wants to initialize several >> VLCs >>>>>>>> in a loop as one then needs to add an array for the sizes/offsets of >>>>>>>> the VLC tables (unless max_depth of all arrays is one in which case >>>>>>>> the sizes are derivable from the number of bits used). >>>>>>>> >>>>>>>> Yet said size array is not necessary if one removes the warning for >>>> too >>>>>>>> big buffers. The reason is that the amount of entries needed for the >>>>>>>> table is of course generated as a byproduct of initializing the VLC. >>>>>>>> So one can proceed as follows: >>>>>>>> >>>>>>>> static VLC vlcs[NUM]; >>>>>>>> static VLC_TYPE vlc_table[BUF_SIZE][2]; >>>>>>>> >>>>>>>> for (int i = 0, offset = 0; i < NUM; i++) { >>>>>>>> vlcs[i].table = &vlc_table[offset]; >>>>>>>> vlcs[i].table_allocated = BUF_SIZE - offset; >>>>>>>> init_vlc(); >>>>>>>> offset += vlcs[i].table_size; >>>>>>>> } >>>>>>>> >>>>>>>> Of course, BUF_SIZE should be equal to the number of entries >> actually >>>>>>>> needed. >>>>>>>> >>>>>>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com> >>>>>>>> --- >>>>>>>> libavcodec/bitstream.c | 3 --- >>>>>>>> 1 file changed, 3 deletions(-) >>>>>>>> >>>>>>>> diff --git a/libavcodec/bitstream.c b/libavcodec/bitstream.c >>>>>>>> index 03d39ad88c..4ffec7e17a 100644 >>>>>>>> --- a/libavcodec/bitstream.c >>>>>>>> +++ b/libavcodec/bitstream.c >>>>>>>> @@ -281,9 +281,6 @@ static int vlc_common_end(VLC *vlc, int nb_bits, >>>> int >>>>>>>> nb_codes, VLCcode *codes, >>>>>>>> int ret = build_table(vlc, nb_bits, nb_codes, codes, flags); >>>>>>>> >>>>>>>> if (flags & INIT_VLC_USE_NEW_STATIC) { >>>>>>>> - if(vlc->table_size != vlc->table_allocated) >>>>>>>> - av_log(NULL, AV_LOG_ERROR, "needed %d had %d\n", >>>>>>>> vlc->table_size, vlc->table_allocated); >>>>>>>> - >>>>>>>> av_assert0(ret >= 0); >>>>>>>> *vlc_arg = *vlc; >>>>>>>> } else { >>>>>>>> -- >>>>>>>> 2.25.1 >>>>>>>> >>>>>> >>>> _______________________________________________ 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".
